bensu / doo

doo is a library and lein plugin to run cljs.test on different js environments.
Eclipse Public License 1.0
324 stars 63 forks source link

Stack traces in PhantomJS #102

Open cvermilion opened 8 years ago

cvermilion commented 8 years ago

It would be great for the Phantom error handler to spit out stack traces. It looks like this would be fairly straightforward: https://github.com/figly/doo/pull/1 (that works in Phantom but I haven't checked Slimer).

Obviously source-mapped traces would be more useful but this seems trickier -- I got something working here: https://github.com/figly/doo/pull/2, but it's currently a giant mess, basically just inlining code from https://github.com/janekp/mapstrace and https://github.com/mozilla/source-map directly into the Phantom-launching code because packaging is such a pain with Phantom I couldn't get it to work easily.

I could probably clean up the source-map PR (perhaps by repackaging mapstrace and source-map into something Phantom can import) if that would be useful/desirable, but I thought I'd check before doing the extra work.

bensu commented 8 years ago

Hi @cvermilion

Thanks for your interest and preliminar work. To add proper stack traces is in the roadmap, I just haven't found the time to do it myself. If you want to submit a PR, even as a draft we can review it together and see what is missing.

bensu commented 8 years ago

It looks like the pr-str is the culprit. In JavaScript doing error.toString() prints "Error: the error's message" but doesn't show the stack trace. We need to override cljs.test's formatter function from doo or reach into cljs.test to call some other function when an error is to be formatted.

bensu commented 8 years ago

Also related is CLJS-1255

timgilbert commented 7 years ago

Even a very dumb and ugly version of this would be super helpful; right now if there's an exception thrown in any of my files at load-time I get a cryptic one-line error with absolutely no indication of where it is coming from, which makes diagnosis very difficult.

MatthewDarling commented 6 years ago

I'm not sure how portable this is, but (println (.-stack e)) seems to get at least some results for me:

file:///Users/matthewdarling/code/budb/out/budb/cljs/register_test.js:9:18
cljs$test$run_block@file:///Users/matthewdarling/code/budb/out/cljs/test.js:378:17
file:///Users/matthewdarling/code/budb/out/running/doo_runner.js:3770:32
doo$runner$run_BANG_@file:///Users/matthewdarling/code/budb/out/doo/runner.js:55:50

global code
evaluateJavaScript@[native code]
evaluate@phantomjs://platform/webpage.js:390:39
phantomjs://code/phantom764181956308983033.js:124:19
_onPageOpenFinished@phantomjs://platform/webpage.js:286:27

(with thanks to #134 for providing an example repo)

It's not pretty. And it's got two random blank lines in the middle. The global code line plus the @[native code] are totally useless. And it's at the JS level rather than CLJS. But at least it gets the CLJS namespaces right, which I've found helpful as a starting point.

Here's how Lumo handles stacktraces, in case that's relevant. .-stack is used there as well. I tried to copy their approach by doing this:

(println
 (stacktrace/mapped-stacktrace-str
  (stacktrace/parse-stacktrace {}
                               (.-stack e)
                               {:ua-product :chrome}
                               {:output-dir "phantomjs://code/phantom"})
  {}
  nil))

But unfortunately that produced worse output:

     global (NO_SOURCE_FILE)
     evaluateJavaScript@[native (NO_SOURCE_FILE)
miikka commented 6 years ago

@MatthewDarling, did you try with {:ua-product :safari}? That should be the right parser for PhantomJS, which uses Webkit.

MatthewDarling commented 6 years ago

You're right, that is better:

     cljs$test$run_block (file:///Users/matthewdarling/code/budb/out/cljs/test.cljs:378:17)
     doo$runner$run_BANG_ (file:///Users/matthewdarling/code/budb/out/doo/runner.cljs:56:50)
     evaluateJavaScript (NO_SOURCE_FILE)
     evaluate (phantomjs://platform/webpage.cljs:390:39)
     _onPageOpenFinished (phantomjs://platform/webpage.cljs:286:27)

It's got one glaring error though: it drops the first line of the unparsed stacktrace. In the original, that shows the original test file, which is the most important line since it's the actual error location. There's probably some combination of arguments to parse-stacktrace which would fix that, though. I'll try to experiment with it a little bit.

And I guess this would need to be environment specific... Thankfully doo will usually know the environment, I think?

MatthewDarling commented 6 years ago

Seems like the Safari parser drops any line that doesn't have @ in it. I'm not sure why the register_test line doesn't have that though. But it doesn't seem like any of the various arguments to parse-stacktrace will avoid dropping that line.

Possibly doo could include a custom stacktrace parser for PhantomJS, and since PhantomJS is deprecated, the format probably won't change too much :)

Speaking of deprecation, I thought I would check out the Chrome headless stacktraces. Result is a bit nicer than PhantomJS:

LOG: 'TypeError: Cannot read property 'Error' of undefined
    at http://localhost:9876/base/out/budb/cljs/register_test.js:9:19
    at cljs$test$run_block (http://localhost:9876/base/out/cljs/test.js:378:13)
    at http://localhost:9876/base/out/running/doo_runner.js:10:28
    at doo$runner$run_BANG_ (http://localhost:9876/base/out/doo/runner.js:71:46)
    at ContextKarma.start (http://localhost:9876/absolute/usr/local/lib/node_modules/karma-cljs-test/adapter.js?65c8dc9efb30a365e93505c5a0badc776600fef6:6:30)
    at ContextKarma.loaded (http://localhost:9876/context.js:162:12)
    at http://localhost:9876/context.html:535:22'

Except the parsed result is worse (using :chrome parser of course):

     TypeError: (NO_SOURCE_FILE)
     cljs$test$run_block (http://localhost:9876/base/out/cljs/test.cljs:378:13)
     doo$runner$run_BANG_ (http://localhost:9876/base/out/doo/runner.cljs:71:46)
     ContextKarma.start (http://localhost:9876/absolute/usr/local/lib/node_modules/karma-cljs-test/adapter.js?65c8dc9efb30a365e93505c5a0badc776600fef6:6:30)
     ContextKarma.loaded (http://localhost:9876/context.cljs:162:12)

The register_test line is still dropped, and now the original error message is gone too.