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.

Sign up for GitHub

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

Jump to bottom

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

Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

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 sapphi-red force-pushed the backport-49849-to-v20.x branch from 119b5ee to 3a1ee22 Compare December 12, 2023 05:25
sapphi-red added a commit to sapphi-red/node that referenced this pull request Dec 12, 2023
@sapphi-red
fs: add stacktrace to fs/promises
3a1ee22 
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 sapphi-red mentioned this pull request Dec 12, 2023
Merged
@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 sapphi-red force-pushed the backport-49849-to-v20.x branch from 3a1ee22 to 61766a7 Compare December 12, 2023 13:17
sapphi-red added a commit to sapphi-red/node that referenced this pull request Dec 12, 2023
@sapphi-red
fs: add stacktrace to fs/promises
61766a7 
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 👍

@UlisesGascon UlisesGascon force-pushed the v20.x-staging branch 2 times, most recently from 7ee5261 to 4c9099f Compare December 15, 2023 18:47
@richardlau
Copy link
Member

This needs to be rebased.

@sapphi-red sapphi-red force-pushed the backport-49849-to-v20.x branch from 61766a7 to c4d58e7 Compare March 26, 2024 04:54
sapphi-red added a commit to sapphi-red/node that referenced this pull request Mar 26, 2024
@sapphi-red
fs: add stacktrace to fs/promises
c4d58e7 
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
fs: add stacktrace to fs/promises
518d819 
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 sapphi-red force-pushed the backport-49849-to-v20.x branch from c4d58e7 to 518d819 Compare March 26, 2024 04:54
@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

CI: https://ci.nodejs.org/job/node-test-pull-request/57957/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57989/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/57995/

marco-ippolito pushed a commit that referenced this pull request Apr 29, 2024
@sapphi-red @marco-ippolito
fs: add stacktrace to fs/promises
c530520 
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marco-ippolito marco-ippolito marco-ippolito approved these changes

@RafaelGSS RafaelGSS RafaelGSS approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sapphi-red @UlisesGascon @richardlau @nodejs-github-bot @marco-ippolito @RafaelGSS