Skip to content

Conversation

@carlos-menezes
Copy link

@carlos-menezes carlos-menezes commented May 16, 2025

Closes #57045.

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels May 16, 2025
@juanarbol
Copy link
Member

juanarbol commented May 16, 2025

Hi, Carlos

Thank you so much for taking the time to open this pull request. I can see the effort and thought you’ve put into it.

Although this PR handles a extremely rare case, which is likely not gonna happen and, the current ENOENT error is informative enough. To keep things consistent for existing users, we try to avoid changing core behavior unless there's a strong, well-discussed reason to do so.

Also, as a note a-side, may be worth reading the contributing guidelines and/or take a low-hanging fruit.

@carlos-menezes
Copy link
Author

@juanarbol thanks for the guidance. Feel free to close this PR in that case.

I assumed the issue linked in the PR was considered a low-hanging fruit contribution given it has the good first issue tag. What should I look for next time I decide to pick an issue up, i.e. what's doable or not?

size_t cwd_len = sizeof(buf);
int err = uv_cwd(buf, &cwd_len);
if (err) {
if (err == UV_ENOENT) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error that's thrown already has an "ENOENT" error code. This shouldn't be necessary.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does, but from the linked issue it is often non-obvious where the error actually came from. By applying this change it makes it clearer why this ENOENT error happened.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

process.cwd() fails in a not usefully descriptive way

5 participants