Uploaded image for project: 'ClojureScript'
  1. CLJS-2758

Improving tracability of cljs.spec.test instrumented function call conform errors

    Details

    • Type: Improvement
    • Status: Open
    • Priority: Major
    • Resolution: Unresolved
    • Affects versions: 1.10.238
    • Fix versions: None
    • Labels:
    • Patch:
      Code

      Description

      Context: Use `cljs.spec.test.alpha/instrument` to instrument `fdef`-ed functions. Call one of said functions with data that fails the argument spec. The thrown exception currently does not provide a lot of context:

      1. The exception message contains "missing filename" and "missing line number" - ideally those should contain the file and line number of the offending call
      2. The exception data is coded to include information about the caller, but the current implementation always yields `nil` here.

      The result is that failure to conform to an fdef argument spec leaves no trace back to the call - it only tells you which function's spec was not satiesfied.

      I've started to investigate, and found a few glitches, but I do not have a complete suggestion. I'm offering my patches so far, hoping to get some input on whether or not it would be desirable for me to continue investigating.

      Here's what I found so far:

      1. `target` was assumed to be `"browser"`, while it is documented to be `"default"` in browsers. Fixed in first patch
      2. When attempting to parse the stack trace to find the caller, `host` included the port number. Adressed in the second patch
      3. When attempting to parse the stack trace, `goog.userAgent.product` is used to determine the browser, but it is not reliable - for example, `product/IPHONE` is true when Chrome is in responsive design mode. This is both incorrect, and helpeless in the case of `cljs.spec.test.alpha`, which does not recognize iphones at all. Because the user agent is only used to parse stack traces, I fixed this by inferring the user agent from the stack trace instead.
      4. The `find-caller` function used the wrong package name to find the `spec_checking_fn`. Additionally, it hard-coded the dots in the package name, while some browsers use `$`. I fixed this in the forth patch. I also moved creating the error outside of the `conform!` anonymous function, as placing it here removes `spec-checking-fn` from the stack trace.

      With these patches, the caller is set reliably, although it doesn't seem to reflect the originating call-site. Also, the thrown exception still lacks file name and line number. I'd be happy to continue investigating, and add some tests if you are interested in eventually accepting these patches.

      As a side-note: It seems to me that the function that infers user agent from stack trace more appropriately belongs in stacktrace.cljs, and I feel strongly that this should be the default behavior when no `user-agent-product` is provided (instead of the current: returning the stack trace string unmodified). I'll be happy to fix this as well, and add some tests, if the interest is there. Note that the current approach allows Safari to go down the Firefox route, which works like a charm.

      Lastly: sorry if I put too many things in one place, I'll be happy to split up as instructed.

        Attachments

          Activity

            People

            • Assignee:
              Unassigned
              Reporter:
              alex+import import
            • Votes:
              0 Vote for this issue
              Watchers:
              0 Start watching this issue

              Dates

              • Created:
                Updated: