Skip to content

process: expose enhanced stack trace to uncaughtException handlers - #65580

Open
santusht06 wants to merge 1 commit into
nodejs:mainfrom
santusht06:fix-uncaught-exception-enhanced-stack
Open

process: expose enhanced stack trace to uncaughtException handlers#65580
santusht06 wants to merge 1 commit into
nodejs:mainfrom
santusht06:fix-uncaught-exception-enhanced-stack

Conversation

@santusht06

Copy link
Copy Markdown

When an EventEmitter instance emits an unhandled 'error' event, Node attaches an internal stack enhancer (kEnhanceStackBeforeInspector) capturing the call site where emit('error', ...) was invoked.

However, createOnGlobalUncaughtException() in lib/internal/process/execution.js was dispatching the error to uncaughtExceptionMonitor and uncaughtException listeners before executing fatalExceptionStackEnhancers.beforeInspector(er).

As a result, custom uncaught exception handlers and APM monitoring libraries were unable to access the enhanced call site on err.stack.

Changes

  • Invoked fatalExceptionStackEnhancers.beforeInspector(er) in createOnGlobalUncaughtException() prior to emitting uncaughtExceptionMonitor and uncaughtException.
  • Added test/parallel/test-process-uncaught-exception-enhanced-stack.js to verify that both uncaughtExceptionMonitor and uncaughtException receive the enhanced err.stack containing the emit('error', ...) call site for EventEmitter and subclass instances.

Fixes: #55838

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders

@codecov

codecov Bot commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.19%. Comparing base (8af7545) to head (9e69cf3).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65580      +/-   ##
==========================================
+ Coverage   90.18%   90.19%   +0.01%     
==========================================
  Files         771      771              
  Lines      264619   264640      +21     
  Branches    50231    50238       +7     
==========================================
+ Hits       238653   238701      +48     
+ Misses      16961    16947      -14     
+ Partials     9005     8992      -13     
Files with missing lines Coverage Δ
lib/internal/process/execution.js 100.00% <100.00%> (ø)

... and 24 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@MikeMcC399

This comment was marked as resolved.

@santusht06

Copy link
Copy Markdown
Author

Thank you @MikeMcC399. I have closed #65531 and #65543 to focus solely on this PR. I will fix the CI failures and run the full test suite before pushing any further updates. Apologies for not following the one-PR-at-a-time guideline.

@santusht06

Copy link
Copy Markdown
Author

Rebased onto the latest upstream/main. The CI build failures (shared libraries on aarch64/x86_64) are pre-existing infrastructure issues unrelated to this change — this PR touches only two JavaScript files (lib/internal/process/execution.js and the new test). All process and EventEmitter tests pass locally.

@santusht06
santusht06 marked this pull request as draft September 6, 2026 17:57
@santusht06

Copy link
Copy Markdown
Author

Converting to draft. After deeper analysis I found a critical bug in the current approach:

fatalExceptionStackEnhancers.beforeInspector() mutates er.stack in-place but does not remove the kEnhanceStackBeforeInspector symbol from the error object. In the no-handler path (when the process exits), the C++ layer (ReportFatalException in src/node_errors.cc) independently calls enhance_fatal_stack_before_inspector — which resolves to the same beforeInspector function. This causes enhanceStackTrace() to run twice, doubling the 'Emitted error event at:'\ frame in the crash output for every unhandled EventEmitter error. This would be a visible regression for all Node.js users not using uncaughtException` handlers.

Need to design this more carefully before proceeding.

When an EventEmitter instance emits an unhandled 'error' event, Node
attaches an internal stack enhancer (`kEnhanceStackBeforeInspector`)
capturing the call site of the `emit('error', ...)` invocation.

Previously, `createOnGlobalUncaughtException()` dispatched the error
to `uncaughtExceptionMonitor` and `uncaughtException` listeners before
applying this enhancement. Consequently, user handlers and monitoring
libraries did not see the emitter call site on `err.stack`.

This commit enhances the stack trace directly via the internal
`kEnhanceStackBeforeInspector` symbol before invoking user handlers,
and removes the symbol so the C++ fatal exception exit path does not
double-apply the frame if the exception remains unhandled.

Also update `test-events-uncaught-exception-stack.js` to assert the
enhanced frame is present, and add a comprehensive test suite covering
monitor listeners, subclass emitters, and fatal double-call safety.

Fixes: nodejs#55838
Signed-off-by: Santusht kotai <115890693+santusht06@users.noreply.github.com>
Assisted-by: Antigravity
@santusht06
santusht06 force-pushed the fix-uncaught-exception-enhanced-stack branch from ed6bd63 to 9e69cf3 Compare September 6, 2026 20:51
@santusht06
santusht06 marked this pull request as ready for review September 6, 2026 20:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Expose the "enhanced" stack trace from uncaught exceptions to the uncaughtException and uncaughtExceptionMonitor handlers on process

3 participants