Skip to content
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

more useable fs/promises backtraces on error #50160

Closed
axkibe opened this issue Oct 12, 2023 · 1 comment · Fixed by #49849
Closed

more useable fs/promises backtraces on error #50160

axkibe opened this issue Oct 12, 2023 · 1 comment · Fixed by #49849
Labels
feature request Issues that request new features to be added to Node.js.

Comments

@axkibe
Copy link
Contributor

axkibe commented Oct 12, 2023

What is the problem this feature will solve?

Take following minimal test case:

import fs from 'node:fs/promises';
async function b( ) { return await fs.readFile( 'inexistent' ); }
async function a( ) { return await b( ); }
await a( );

the output will be:

      internalBinding('errors').triggerUncaughtException(
                                ^

[Error: ENOENT: no such file or directory, open 'inexistent'] {
  errno: -2,
  code: 'ENOENT',
  syscall: 'open',
  path: 'inexistent'
}

Node.js v20.8.0

so no hint where.

What is the feature you are proposing to solve the problem?

I used to wrap all fs call to something like this:

import fs from 'node:fs/promises';
async function myread( filename )
{
    try { return await fs.readFile( filename ); }
    catch( e ) { throw new Error( 'cannot read "' + filename + '"' ); }
}
async function b( ) { return await myread( 'inexistent' ); }
async function a( ) { return await b( ); }
await a( );

So the error stack becomes:

file:///home/axel/xtest2/x2.mjs:6
	catch( e ) { throw new Error( 'cannot read "' + filename + '"' ); }
	                   ^

Error: cannot read "inexistent"
    at myread (file:///home/axel/xtest2/x2.mjs:6:21)
    at async b (file:///home/axel/xtest2/x2.mjs:8:30)
    at async a (file:///home/axel/xtest2/x2.mjs:9:30)
    at async file:///home/axel/xtest2/x2.mjs:10:1

Suggestion: IMO it would be beneficial for node to make these more userfriendly/useful backtraces by default without having to wrap all fs calls by the user?

What alternatives have you considered?

explained above, wrap the fs calls with try/catch blocks all myself, it's very possible, likely someone wrote a npm package anyway, likely several, but IMO this is one of the things the core should come with making the entry easier for new people.

@axkibe axkibe added the feature request Issues that request new features to be added to Node.js. label Oct 12, 2023
@sapphi-red
Copy link
Contributor

sapphi-red commented Oct 12, 2023

I guess my PR (#49849) implements this.

nodejs-github-bot pushed a commit that referenced this issue Oct 26, 2023
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: #34817
PR-URL: #49849
Fixes: #50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
alexfernandez pushed a commit to alexfernandez/node that referenced this issue Nov 1, 2023
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: nodejs#34817
PR-URL: nodejs#49849
Fixes: nodejs#50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
targos pushed a commit that referenced this issue Nov 11, 2023
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: #34817
PR-URL: #49849
Fixes: #50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
sapphi-red added a commit to sapphi-red/node that referenced this issue Dec 12, 2023
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: nodejs#34817
PR-URL: nodejs#49849
Fixes: nodejs#50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
sapphi-red added a commit to sapphi-red/node that referenced this issue Dec 12, 2023
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: nodejs#34817
PR-URL: nodejs#49849
Backport-PR-URL: nodejs#51127
Fixes: nodejs#50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
sapphi-red added a commit to sapphi-red/node that referenced this issue Dec 12, 2023
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: nodejs#34817
PR-URL: nodejs#49849
Backport-PR-URL: nodejs#51127
Fixes: nodejs#50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
sapphi-red added a commit to sapphi-red/node that referenced this issue Mar 26, 2024
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: nodejs#34817
PR-URL: nodejs#49849
Backport-PR-URL: nodejs#51127
Fixes: nodejs#50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
sapphi-red added a commit to sapphi-red/node that referenced this issue Mar 26, 2024
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: nodejs#34817
PR-URL: nodejs#49849
Backport-PR-URL: nodejs#51127
Fixes: nodejs#50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
marco-ippolito pushed a commit that referenced this issue Apr 29, 2024
Sync functions in fs throwed an error with a stacktrace which is helpful
for debugging. But functions in fs/promises throwed an error without
a stacktrace. This commit adds stacktraces by calling
Error.captureStacktrace and re-throwing the error.

Refs: #34817
PR-URL: #49849
Backport-PR-URL: #51127
Fixes: #50160
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
Reviewed-By: Jiawen Geng <technicalcute@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issues that request new features to be added to Node.js.
Projects
Status: Pending Triage
Development

Successfully merging a pull request may close this issue.

2 participants