fs: add stack traces to async callback errors#61720
Open
DukeDeSouth wants to merge 1 commit intonodejs:mainfrom
Open
fs: add stack traces to async callback errors#61720DukeDeSouth wants to merge 1 commit intonodejs:mainfrom
DukeDeSouth wants to merge 1 commit intonodejs:mainfrom
Conversation
Async fs callback errors (created via UVException in C++ when the
JavaScript call stack is empty) previously had no stack frames β only
the error message. This made debugging fs operations very difficult,
as users could not determine where in their code a failed operation
was initiated.
This change enriches async fs callback errors with stack traces via
a two-tier approach:
Tier 1 (always-on, zero overhead on success):
- `enrichFsError()` is called in `makeCallback()` when an error
is received from the native binding
- Captures the stack at callback invocation time, providing at
minimum the internal Node.js frames
- Only runs on the error path (~1us), no overhead on success
Tier 2 (opt-in via NODE_FS_ASYNC_STACK_TRACES=1):
- When enabled, captures the JavaScript call-site stack at
operation initiation time (in makeCallback/readFile)
- Provides full stack traces showing WHERE in user code the
operation was called, similar to sync fs error stacks
- ~1us overhead per async fs call when enabled
This is consistent with how `handleErrorFromBinding` enriches errors
in the sync path (lib/internal/fs/utils.js) and how the promise-based
path enriches errors (lib/internal/fs/promises.js:handleErrorFromBinding).
Before:
Error: ENOENT: no such file or directory, open 'config.json'
(no stack frames)
After (default):
Error: ENOENT: no such file or directory, open 'config.json'
at FSReqCallback.readFileAfterOpen [as oncomplete] (node:fs:292:13)
After (NODE_FS_ASYNC_STACK_TRACES=1):
Error: ENOENT: no such file or directory, open 'config.json'
at loadConfig (/app/server.js:42:6)
at main (/app/server.js:98:3)
Fixes: nodejs#30944
Co-authored-by: Cursor <cursoragent@cursor.com>
Member
|
This sort of thing has existed since ~2010 (see longjohn and stuff like bluebird async stack traces) and in fact you get this mostly by connecting a debugger (which also does non-zero-cost stack traces). The eventual design was to support this only for async/await since it was zero cost and new and easy to reason about - there are exceptions (e.g. event emitter errors). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Human View
Problem
Async fs callback errors have no JavaScript stack traces β only the error message (#30944, open since 2019, 29 reactions). This makes debugging fs operations very difficult:
The root cause: errors are created in C++ (
FSReqAfterScope::RejectβUVException) when the JavaScript call stack is empty. The sync API and promises API don't have this problem β sync errors are created in JS context, and the promises API already callsErrorCaptureStackTraceinhandleErrorFromBinding.Solution
Two-tier approach that enriches async callback errors with stack traces:
Tier 1 (always-on, default):
enrichFsError()is called inmakeCallback()on the error path. Zero overhead on success (~2-5nsinstanceofcheck), captures internal Node.js frames on error.Tier 2 (opt-in via
NODE_FS_ASYNC_STACK_TRACES=1): Captures the JavaScript call-site stack at operation initiation time, providing full stack traces showing WHERE in user code the operation was called. ~1us overhead per async fs call when enabled.Before:
After (default):
After (
NODE_FS_ASYNC_STACK_TRACES=1):Changes
lib/internal/fs/utils.js: AddenrichFsError()andshouldCaptureCallSiteStack()β shared error enrichment utilitieslib/fs.js: ModifymakeCallback()andmakeStatsCallback()to enrich errors; add call-site capture toreadFile()lib/internal/fs/read/context.js: Add error enrichment toReadFileContextpathtest/parallel/test-fs-error-stack-trace.js: Comprehensive tests for all async fs operationsDesign Rationale
makeCallbackand not C++? Pure JavaScript change, same pattern as existinghandleErrorFromBinding(sync) and promises APIError.captureStackTracecosts ~1us per call. Forfs.stat(~2.5us), this adds 40% overhead. Opt-in avoids surprising perf regressions.NODE_FS_ASYNC_STACK_TRACES=1 node app.js).Fixes: #30944
AI View (DCCE Protocol v1.0)
Metadata
AI Contribution Summary
Verification Steps Performed
Human Review Guidance
lib/internal/fs/utils.js,lib/fs.js,lib/internal/fs/read/context.jsMade with M7 Cursor