-
Notifications
You must be signed in to change notification settings - Fork 37
No reporter prints uncaughtException
details in case of browser tests
#429
Comments
Wouldn't the correct approach be to fix the object emitted by json-proxy? |
I don't see, what the issue has to do with json-proxy. Isn't that just another reporter? "suite:error": function (msg) {
this.emit("suite:error", msg.data);
} the Error "no contexts" produces a useful error message:
I am still not happy about the "Unknown context", but this can for sure also be fixed. |
Seems we also need a handler for "uncaughtException": "uncaughtException": function (msg) {
this.emit("uncaughtException", msg.data);
}
|
Where are you adding these handlers? I might have missed something... but my understanding is that normally, when the error happens in node, the |
I added the handlers in buster-test-cli/lib/runners/browser/remote-runner.js. The json-proxy isn't used in case of browser tests. I think you can use it via this.emit(event, {
event: event,
client: this.getClient(msg.slaveId),
data: msg.data,
runtime: msg.data.runtime
}); That's why the exception is inside a |
Huh? Isn't json-proxy used by the browser runner to send data back to the server? Ofc the fix in remote runner might be even better - I really don't know buster architecture well enough :) (hint hint, some diagrams would be useful) |
Hmmm. @cjohansen's comment: #327 (comment) |
@dominykas, you are right. After a long search i found the point in the code, where json-proxy is configured to be used for browser tests, var reporter = B.reporters.jsonProxy.create(emitter);
reporter.listen(runner); |
OK, so we're still left with the decision of where to fix this - json-proxy or remote-runner... |
I think the fix should be applied in the JSON proxy. It doesn't change any of the other event objects does it? |
After some more explanation from @dwittner, it seems his suggested solution in remote runner is the correct fix. |
@dominykas, do you want to fix it? |
I'm not sure when I'll have time to get to it. I've never looked at remote runner code either... And when I do find time - I'll probably start with fixes for other issues I've got in progress - so not sure which one of us will get around to this one sooner :) but if it's still there - sure. |
Fixed in version 0.8.7 of buster-test-cli. |
<3 |
Briefly mentioned this on busterjs/buster-test#27. Seems that in nodejs cases, the param for
uncaughtException
handler contains the instance of theError
object itself, but if it comes back from the browser - the details are in the serialized object in thedata
property of thee
param.I don't think updating all the reporters to check for
e.data
is the correct approach... What's the route the exception takes once it's emitted by thejson-proxy
?The text was updated successfully, but these errors were encountered: