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

[v20.x backport] fs: add stacktrace to fs/promises #51127

Closed

Conversation

sapphi-red
Copy link
Contributor

This PR is a backport of #49849.


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

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch. labels Dec 12, 2023
sapphi-red added a commit to sapphi-red/node that referenced this pull request 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
Copy link
Contributor Author

The lint error is not caused by this PR.

@UlisesGascon
Copy link
Member

The lint error is not caused by this PR.

The lint issue was solved in c519e80 🙌

sapphi-red added a commit to sapphi-red/node that referenced this pull request 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
Copy link
Contributor Author

Rebased on top of it 👍

@richardlau
Copy link
Member

This needs to be rebased.

sapphi-red added a commit to sapphi-red/node that referenced this pull request 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>
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>
@richardlau richardlau added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 26, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 26, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

marco-ippolito pushed a commit that referenced this pull request 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>
@marco-ippolito
Copy link
Member

Landed in c530520

@sapphi-red sapphi-red deleted the backport-49849-to-v20.x branch May 15, 2024 13:51
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. v20.x v20.x Issues that can be reproduced on v20.x or PRs targeting the v20.x-staging branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants