Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use original Error object for uncaught errors when provided #2403

Closed
wants to merge 1 commit into from

Conversation

mgiuffrida
Copy link

Include stack trace of async errors by not throwing away the Error passed by global.onerror.

Fixes #958, the same issue referenced at #815 (comment).

@mgiuffrida
Copy link
Author

@freshp86 this helps with stack traces in Chrome's WebUI tests

@mgiuffrida mgiuffrida mentioned this pull request Jul 30, 2016
@boneskull
Copy link
Contributor

@mgiuffrida mocha.js is generated by make; you'll need to apply this change in lib/ somewhere (or maybe browser-entry.js)

going to try to release v3.0.0 tonight, so you might want to wait until that happens before retrying.

Uses the existing error instead of creating a new stack trace from the current context.
@mgiuffrida
Copy link
Author

@boneskull thanks, that was silly of me.

From the actual mocha.js log it seems I shouldn't try to commit a built version; i've moved the change to browse-entry.js instead and rebased.

@@ -51,8 +51,11 @@ process.removeListener = function(e, fn){

process.on = function(e, fn){
if ('uncaughtException' == e) {
global.onerror = function(err, url, line){
fn(new Error(err + ' (' + url + ':' + line + ')'));
global.onerror = function(err, url, line, col, errObj){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should col be leveraged?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It isn't currently, and it normally isn't in JS, right?

@boneskull
Copy link
Contributor

Closing for inactivity; please reopen if continuing. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: browser browser-specific type: feature enhancement proposal
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants