-
Notifications
You must be signed in to change notification settings - Fork 30.7k
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
process: improve error message for process.cwd() when directory is deleted #57053
base: main
Are you sure you want to change the base?
Conversation
…leted If the current working directory is deleted, process.cwd() throws a generic 'uv_cwd' error, which can be confusing. This commit enhances the error message by modifying the original error instead of throwing a new one. PR-URL: nodejs#57053
Review requested:
|
Thanks for taking this one on! ❤️ A few things though: Why would mutating the error be preferable to using Line 75 in d384151
error.cause would preserve it, which may be helpful.
This also needs a test. I think the change to the native function may make the javascript change unreachable due to no longer having the syscall info. 🤔 And lastly, a more minor detail: a couple lint checks fail as the commit message is too long and the message lines are too long. We limit lines to 80 columns. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #57053 +/- ##
=======================================
Coverage 89.10% 89.11%
=======================================
Files 665 665
Lines 193203 193213 +10
Branches 37216 37216
=======================================
+ Hits 172158 172181 +23
- Misses 13768 13778 +10
+ Partials 7277 7254 -23
|
what should further with this pr |
I agree that I believe originally there was a new error thrown without |
I think @jasnell has the best advice here--add the error as a |
Please check the pr |
Code change LGTM but this needs a test. The test might be a bit tricky but it could create a temporary directory with a script in it, run that script, delete that directory, then call the api from the script that was in that directory, etc.... anyway, once a test is added I'm happy to approve this. |
This PR improves the error message thrown by
process.cwd()
when the current working directory is deleted. Instead of throwing a new error, it enhances the original error message, making it clearer that the directory was deleted while the process was still inside it.Changes Made:
wrappedCwd()
to enhance the existing error message forENOENT: uv_cwd
instead of creating a newError
instance.process.chdir()
to switch directories).Before (Old Behavior):
When the working directory was deleted,
process.cwd()
threw a generic error with little context:After (New Behavior):
Now, the error provides a more meaningful message:
Relevant Issue:
(If this PR fixes an issue, add the issue number here)
Example: Fixes #XXXX
PR-URL:
#57053
Reviewer Notes: