Latest qunit with phantomjs swallows exceptions from other scripts #403

Closed
ashwinr opened this Issue Jan 31, 2013 · 23 comments

Projects

None yet

5 participants

@ashwinr

When running qunit (1.11.0) under phantomjs (1.8.1), exceptions in scripts that are not qunit test files are swallowed. They do not show up in phantomjs' console output.

@JamesMGreene
jQuery Foundation member

Do you know if this is a new issue? (Did the same thing occur with QUnit 1.10?)

@ashwinr

Attached code for testing the above issue: phantomjs and qunit files.

@ashwinr

Just tested it against 1.10.0. Same issue :(

@ashwinr

One of the reasons this is important is because a lot of us use qunit with requirejs for unit testing, and so failures in requirejs (timeout/remote script errors) do not show up at all (makes debugging hard).

@JamesMGreene
jQuery Foundation member

Understood.

Dealing with these globally unhandled errors (with window.onerror) in multiple libraries (QUnit, RequireJS, PhantomJS) is tough because most libraries will want to make the error as handled (to prevent it from screwing over the browser) but this prevents any other registered handlers for window.onerror from knowing about it. The libraries also need to hookup their window.onerrror handler such that it doesn't blow away previously registered handlers.

In actuality, QUnit is doing the right thing on both accounts: it runs any previous registered window.onerror handlers first, only runs ours if the error wasn't handled by those previous handlers, and then our handler will only mark the error as handled (i.e. suppress it) if there is a QUnit.throws assertion being executed at the time:
https://github.com/jquery/qunit/blob/58cef74b5d5f4993e7dc53e8ada7c97651d02cff/qunit/qunit.js#L1186-L1216

So, in reality, I think this may be an issue with PhantomJS (to which I am also a contributor) or RequireJS.

@ashwinr

Interesting. I just tested this again, and like you mention, QUnit does the right thing by returning false (allowing the default browser handler to run). As you can see, I'm not even using requirejs, so I believe the error might be in phantomjs' default onError handler.

@JamesMGreene
jQuery Foundation member

Oh right, your example didn't have RequireJS involved.

So yeah, the likelihood that this is a problem with PhantomJS seems high but that is also troubling as we are currently just consuming the error signal that we receive from [Qt]WebKit. This may imply that QtWebKit is actually setting a window.onerror handler at some point that overrides QUnit's existing one... this seems unlikely given the timing of the page load, though, as I'm sure WebKit would want to get that handler on the page ASAP rather than waiting until after a bunch of scripts load.

@ariya, any insights?

@JamesMGreene
jQuery Foundation member

@jonleighton: Could this be related to any of your PhantomJS commits for stack traces (e.g. Breakpad, etc.)?

e.g.

@jonleighton

Breakpad is a crash reporter, so not related.

But yeah, it might be related to my stack traces work - I don't know.

I think WebKit 2.3 introduces error.stack natively though, so we might be able to do something with then when it gets merged.

@ashwinr

@jonleighton since I'm not sure when all your changes went in, if you can indicate a version of QUnit before your changes, I can test it on phantomjs (if it helps at all).

@jonleighton

@ashwinr the changes are in phantomjs, not qunit

@ashwinr

ah, sorry, my mistake. JamesMGreene's commit hashes comment seemed to point into qunit's repo, so I assumed qunit.

I can take a look around your commits, and see if I find something. Do you guys know when 2.3 WebKit will get merged?

@JamesMGreene
jQuery Foundation member

Oh, whoops, I forgot that the SHAs would auto-link to the QUnit repo. I've fixed them now.

@jzaefferer
jQuery Foundation member

@JamesMGreene @ashwinr @jonleighton could one of you revisit this?

@JamesMGreene
jQuery Foundation member

We have an unstable but mostly functional Phantom 2.0 branch [with an updated WebKit] that I can try it out with soon.

@JamesMGreene
jQuery Foundation member

I've been looking into this with both Phantom 1.9.7 and 2.0.0-beta... definitely some oddities but they seem to all be on the Phantom side as far as I can tell right now. Phantom 2.0.0-beta regresses even further on error handling right now, unfortunately.

Unless anyone opposes, I think we should create a new Phantom bug and close this one out.

@ashwinr
@JamesMGreene
jQuery Foundation member

Sure. With my current Phantom 2.0.0-beta version, the phantom.onError and WebPage#onError handlers aren't being triggered at all, even if there isn't a registered window.onerror handler on the actual web page.

cc: @Vitallium

@jzaefferer
jQuery Foundation member

@JamesMGreene @Vitallium any updates on this?

@jzaefferer
jQuery Foundation member

Need to test with Phantom 2.0. @leobalter could you do that some time?

@leobalter leobalter self-assigned this May 19, 2015
@jzaefferer
jQuery Foundation member

Phantomjs 2.0 is still not ready. PhantomJS 1.x is still bad. Whatever.

@jzaefferer jzaefferer closed this Oct 16, 2015
@JamesMGreene
jQuery Foundation member

Phantomjs 2.0 is still not ready.

Huh?

@jzaefferer
jQuery Foundation member

See Medium/phantomjs#288 which lead me eventually to ariya/phantomjs#12948

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment