async_hooks: clear context frame for thrown microtasks - #64147
Conversation
|
cc @nodejs/diagnostics |
There was a problem hiding this comment.
Don't we generally want the context to be available in uncaughtException? We did a bunch of work at one point to ensure it would be there so uncaughtException handlers could attribute the errors to spans they came from in tracers. It'd probably be more correct to cut off the context after those handlers run. 🤔
I thought as well. The key assertion here is this one: https://github.com/nodejs/node/pull/64147/changes#diff-6c14bb576b8a1eaf12aeb421aec5efd35b58d92c8a02d668a83a2f74f9c6474fR18. |
|
Note that if we think the current behavior is correct, then I'll add a test for it to match. |
|
@mcollina The commit needs to be signed. |
Signed-off-by: Matteo Collina <hello@matteocollina.com>
f091a7a to
f2da5c5
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #64147 +/- ##
==========================================
- Coverage 90.32% 90.31% -0.02%
==========================================
Files 759 760 +1
Lines 248342 248535 +193
Branches 46866 46887 +21
==========================================
+ Hits 224320 224461 +141
- Misses 15461 15510 +49
- Partials 8561 8564 +3
🚀 New features to boost your workflow:
|
This clears the AsyncContextFrame before rethrowing exceptions from
queueMicrotask()callbacks.V8 restores continuation-preserved embedder data for microtasks. If a microtask callback throws, exception reporting can re-enter JavaScript while the microtask's context frame is still current. Clearing the frame before rethrowing keeps subsequent exception formatting and microtasks from observing the throwing callback's AsyncLocalStorage state.
Includes a regression test covering a non-Error object thrown from a microtask whose string conversion queues another microtask.