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

Option to let exceptions propagate #553

Closed
joliss opened this issue Aug 23, 2012 · 20 comments
Closed

Option to let exceptions propagate #553

joliss opened this issue Aug 23, 2012 · 20 comments
Labels
type: feature enhancement proposal

Comments

@joliss
Copy link
Contributor

joliss commented Aug 23, 2012

Mocha's browser runner always catches exceptions. I wish it could let them propagate so I can see the stack trace in the JS console.

The line numbers Mocha displays are useful when only (1) you have the file open in the editor, and (2) it's not compiled from CoffeeScript. When either of these conditions isn't met, they turn pretty useless, and you'd really prefer to have a linked stack trace like the console in Chrome or Firebug gives you.

See e.g. http://i.imgur.com/CQV37.png - that trace is not very usable, because now I have to go hunting for the offending line.

@joliss
Copy link
Contributor Author

joliss commented Aug 23, 2012

To clarify: I wish there was an option to let them propagate.

I believe QUnit's notrycatch does that, for instance. http://i.imgur.com/nZC3t.png

@tj
Copy link
Contributor

tj commented Aug 24, 2012

I dont get it, that is the same stack trace you would see in the console

@joliss
Copy link
Contributor Author

joliss commented Aug 24, 2012

In the console, I'd have the lines linked. In other words, if I click /assets/local-ember/ember.js?body=1:10585, it would open the source inspector and show me the exact line on which the error is happening.

This makes a huge difference in usability, because now I don't have to go hunting for the line.

@rauchg
Copy link
Contributor

rauchg commented Aug 24, 2012

Agreed. I wonder if there's some console API we can hook into to avoid throwing and having options

@wagenet
Copy link

wagenet commented Mar 19, 2013

I'm strongly in agreement with @joliss on this. Being able to interactively debug is a huge win. The ability to turn off exception catching is one of my favorite features of QUnit. That said, I'm not sure how easily this could be done with Mocha since it uses exceptions for assertions.

@joliss
Copy link
Contributor Author

joliss commented Mar 19, 2013

Related discussion: https://twitter.com/wagenet/status/313867942502744064

@tj
Copy link
Contributor

tj commented Mar 19, 2013

I suppose it's a tradeoff, I've never really been too turned off by this but if you have a massive lib like ember I suppose it can definitely be a bit annoying, I'd just jump into the debugger at that point

@vendethiel
Copy link

agreed

@wagenet
Copy link

wagenet commented Mar 19, 2013

@visionmedia This is what I do. But if you have libraries like jQuery which intentionally use exceptions to check functionality it can be quite a pain to hop through each one of those errors. Anyway, it doesn't sound like something easy to fix so I'm not expecting much to change here.

@tj
Copy link
Contributor

tj commented Mar 19, 2013

most assertion libs have some sort of AssertionError constructor but yeah you're right as far as that being leaky since we accept anything really, or maybe do the inverse and check for TypeErrors etc.

@joliss
Copy link
Contributor Author

joliss commented Mar 19, 2013

I think maybe this is not too hard to implement if we let all exceptions (and assertions failures) propagate unconditionally. Part of what's holding me back is the way we're implementing the HTML runner: It makes it very hard to manage state. The ?grep= URLs are not very clean already, and if we add something like QUnit's ?notrycatch=true, it'll get messy.

@jimmycuadra
Copy link
Contributor

I have a test failing that I wanted to use the Chrome debugger to investigate, but in order for the debugger to intercept the exception, I had to remove the try/catch at the end of Runnable#run. If there was just a boolean I could set when calling Mocha#setup to make the try/catch optional, that would be sufficient. If it's an option to turn it on, why do we care about distinguishing assertions from other exceptions?

@esamattis
Copy link

I agree it would be very useful to have an option to disable exception capturing for the browser runner when using the "Pause on uncaught exceptions" feature from Chrome (and maybe others).

Now it is bit pain to manually find the correct line for the debugger. Especially when using source maps which is always the case with Browserify for example. The stack trace displayed in the runner does not know nothing about them.

@dsawardekar
Copy link

This is an old issue but perhaps a simpler solution is to console.log the error's stack trace before the done() in Runnable.

console.log(err.stack);
done(err);

This puts the stack trace in the chrome's console, so line numbers are clickable. And takes you to right place if source maps are setup correctly. All without affecting the Mocha test runner in any manner.

Additionally this could be done behind a flag like --log-exceptions so the default case is not affected. And logs could be hidden if not inside a browser env, etc.

This idea is working fine for me currently within a browserify bundle with source maps. Happy to make a PR if there is interest in having this in master.

@Diokuz
Copy link

Diokuz commented Sep 23, 2014

+1

@brianpeiris
Copy link

+1 Being able to toggle a flag and have the exception propagate to the browser's debugger would make things a lot easier.

@boneskull boneskull added type: feature enhancement proposal status: accepting prs Mocha can use your help with this one! labels Oct 20, 2014
@NiGhTTraX
Copy link

+1

1 similar comment
@abenhamdine
Copy link

+1

amsul added a commit to pumpupapp/mocha that referenced this issue Apr 18, 2015
amsul added a commit to pumpupapp/mocha that referenced this issue Apr 18, 2015
amsul added a commit to pumpupapp/mocha that referenced this issue Apr 18, 2015
added tests for allowUncaught option

error handler prints to dom with allowUncaught
amsul added a commit to pumpupapp/mocha that referenced this issue Apr 18, 2015
fixed merge issues

fixed merge issues

fixed merge issues
amsul added a commit to pumpupapp/mocha that referenced this issue Apr 18, 2015
allows unhandled exceptions to propagate in the browser
added tests for allowUncaught option
global error handler prints to dom with allowUncaught
amsul added a commit to pumpupapp/mocha that referenced this issue Apr 19, 2015
allows unhandled exceptions to propagate in the browser
added tests for allowUncaught option
global error handler prints to dom with allowUncaught
boneskull pushed a commit to boneskull/mocha that referenced this issue Aug 31, 2015
allows unhandled exceptions to propagate in the browser
added tests for allowUncaught option
global error handler prints to dom with allowUncaught
@berkerpeksag
Copy link
Contributor

This is already fixed in 23939a9 (see #1662) and can now be closed.

@danielstjules
Copy link
Contributor

Good call, thanks! :)

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

No branches or pull requests