-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Async Express.js function with invalid reference hangs Mocha without an error or stack trace #4632
Comments
What is the result if you replace |
With or without
Thanks for the lead, I was unaware and will check that out! |
@juergba I checked into ladjs/supertest#697 . It may be related, but I'm not sure that exactly the same thing is going on here, for two reasons:
Also, in their case, they don't talk about what's actually happening in the back end, whereas in my example, taking |
I can't reproduce this one. There is no difference in outcome with or without the
The |
@juergba Please note my point in 1. following the other issue has no I had read that So...is my original (reproducible, on my end) example a problem with Mocha, or Supertest, or the interaction between them, or something else? As a mere user of these libraries, I can only tell that the stack isn't working (in what I would consider a common debugging scenario), not which part(s) may need changes for it to work. |
I'm not going to reproduce this case. Trying to handle a promise the synchronous way just doesn't make sense. That's javascript basics. And yes, I did clone your repo and can reproduce your results.
Your
|
Fair enough (re the sync case)—makes sense, I was rather blindly just following the other issue. Thanks for confirming that you can reproduce my original issue. ✔ Ah, sorry, I misspoke about Thank you, I really appreciate your explanation and especially your tip to use As for Supertest and Thanks again! =) |
As for a Supertest-only example, here's what I have:
Result of
So...is there something Mocha could help with after all, then, given that the option you recommended is for Node.js, not Mocha? Or should it just be documented as something of a best practice, along the lines of "BTW, Node.js has deprecated unhandled promises, so if you async/await throughout your stack, you'll want to use their |
In first place it's your code which produces an unhandled rejection. So you should emphasis on fixing this issue, probably that has to be done in the express part. It's not a good practice to produce uncaught exceptions or unhandled rejections and expect Mocha to fix this for you. Mocha is not documenting Node's option, this is a bottomless task we are not going to start with. And there is nothing Mocha can do, if you instruct: "wait for this promise to be resolved", but your code (or your dependencies) prints a soft warning only. Now you have to give me a good reason not to close this issue. |
Absolutely, it's my code where the problem lies. However, the situation affects the developer UX of working with the Mocha/Supertest stack, making fixing such an issue (which can be brought about by stale code, or something as little as a typo) take quite a bit longer than it needs to, but now I'm just repeating my opening report. Wrapping every function in try/catch seems like a lot of boilerplate just to get a normal stack trace during testing—something your Node.js option suggestion more or less takes care of without all that hassle, so again +1 for that solution. :) I appreciate you confirming there's nothing Mocha can do (and for taking the time to clarify things above). Even if I might wish for best practices / workable developer environments to be part of documentation in general, I understand not "going down the rabbit hole" of documenting Node.js' option, and I respect the boundary you just set. Thanks again for your insights and for Mocha! 🙌 |
Prerequisites
faq
labelnode node_modules/.bin/mocha --version
(Local) andmocha --version
(Global). We recommend that you not install Mocha globally. ← confirmed Mocha is not installed globally; local is at latest 8.3.2Description
If an
async
function is serving up an Express.js endpoint, and that function causes a runtimeTypeError
, Mocha times out, then hangs, and never displays the real error nor a stack trace.Without
async
in the same situation, Mocha displays the error and a stack trace, and exits properly.Steps to Reproduce
I've created a minimal repo at https://github.com/codingthat/mocha-supertest-async-hang-repro-repo
Expected behavior:
Actual behavior: [What actually happens]
Reproduces how often: 100%
Versions
mocha --version
andnode node_modules/.bin/mocha --version
: (apt "not installed" message) and8.3.2
node --version
:v14.16.1
Additional Information
I think this is important, because Express.js controller functions often need to be
async
to be able toawait
database calls. As it currently is, a mere typo in such a function can mean a lengthy process of recreating an E2E test by hand just to get at the real error that's occurring. Non-async
controllers behave much better with Mocha, letting you already debug from Mocha's output without any additional (and tedious) steps. But avoidingasync
/await
in controllers just because of this would be a step backward.I admit I'm not sure if this is a Mocha bug or perhaps a bug with Supertest or another dependency. Please let me know if I should report it elsewhere—this was as minimal an example as I could manage.
The text was updated successfully, but these errors were encountered: