-
Notifications
You must be signed in to change notification settings - Fork 25
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
Idea: Switching Mammoth to use native async
/await
might yield better stack traces.
#205
Comments
Small demo of the problem: https://gist.github.com/cakoose/5271c469142205d37c600000cc22bb3e |
Mammoth automatically executes your query when you await it. This is done by adding a There isn't a function which we can change to async to make sure native async / await is triggered. How do you think we can fix this?
|
Re 1: Using an explicit Re 2: That style of stack trace preservation isn't as efficient, and thus is not recommended in production. Having something only work in dev is still definitely useful, but I'm hoping we can get V8 native async stack traces working, which will work in both. Tangent: Is an explicit Zapatos has an explicit When I first started using Mammoth, I preferred the idea of Zapatos' explicit There's also a small practical advantage to Mammoth's approach: I can use the ESLint no-floating-promises rule to detect when I forget to |
I do not see any performance differences in the happy path when enabling |
I was just going off of the warning in Knex's docs ("This has small performance overhead, so it is advised to use only for development.") and the general warnings around "async stack trace" libraries in general, e.g. I assume the cost is mainly in generating the stack trace, but it probably adds some GC pressure too But maybe that overhead is not actually that much in this context because a remote DB query is already relatively expensive? Where's the benchmark? I wonder if it's measuring a case where generating the stack is cheap for whatever reason, e.g. shallow stack. |
The benchmark is here https://github.com/Ff00ff/mammoth/tree/master/workspaces/sql-benchmarks. You are right, the stack isn't too deep so maybe that's why there is no significant overhead. |
Historically, JavaScript stack traces get truncated once you yield back to the event loop, e.g. for I/O. In the last few years, V8 added async stack traces, which fix that problem if you use native
async
/await
instead of raw promises. This makes it way easier to track down errors.It looks like Mammoth uses raw promises, so the stack traces I get are truncated. I think if it used native
async
/await
, the stack traces would be better. I'm not familiar enough with Mammoth to understand the difficulties and downsides of doing that, though.A workaround: wrap each function with code that catches exceptions and fixes the stack trace: GitHub gist. That works if I'm only using a few functions or there are only a few call sites. But Mammoth has a pretty big API and I use Mammoth everywhere in my app, so that would be too tedious.
The text was updated successfully, but these errors were encountered: