Skip to content

node: warn if cwd is inaccessible during bootstrap#12022

Closed
Fishrock123 wants to merge 1 commit into
nodejs:masterfrom
Fishrock123:warn-on-inaccesible-cwd
Closed

node: warn if cwd is inaccessible during bootstrap#12022
Fishrock123 wants to merge 1 commit into
nodejs:masterfrom
Fishrock123:warn-on-inaccesible-cwd

Conversation

@Fishrock123
Copy link
Copy Markdown
Contributor

@Fishrock123 Fishrock123 commented Mar 24, 2017

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines
Affected core subsystem(s)

bootstrap

This was added/fixed in 2c6f79c and seems like it could be quite confusing, so... maybe we should warn? ¯\_(ツ)_/¯

I don't really know how to trigger this though, so no test... for now?

cc @bnoordhuis, maybe this isn't a good idea, idk.
@nodejs-github-bot nodejs-github-bot added the lib / src Issues and PRs related to general changes in the lib or src directory. label Mar 24, 2017
@Fishrock123 Fishrock123 force-pushed the warn-on-inaccesible-cwd branch from cee2c9b to f381a70 Compare March 24, 2017 15:54
@addaleax
Copy link
Copy Markdown
Member

addaleax commented Mar 24, 2017

I don't really know how to trigger this though, so no test... for now?

mkdir /tmp/a; cd /tmp/a; rmdir /tmp/a; node -p 'module.paths'
@Fishrock123
Copy link
Copy Markdown
Contributor Author

Fishrock123 commented Mar 24, 2017

Ah ok, will write a test in a bit.
cjihrig
cjihrig requested changes Mar 24, 2017
View reviewed changes
Comment thread lib/internal/bootstrap_node.js Outdated
Copy link
Copy Markdown
Contributor

@cjihrig cjihrig Mar 24, 2017

Choose a reason for hiding this comment

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

Can you lowercase Current Working Directory.
Comment thread lib/internal/bootstrap_node.js Outdated
Copy link
Copy Markdown
Contributor

@cjihrig cjihrig Mar 24, 2017

Choose a reason for hiding this comment

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

Can you drop the last two dots.
jasnell
jasnell requested changes Mar 24, 2017
View reviewed changes
Comment thread lib/internal/bootstrap_node.js Outdated
Copy link
Copy Markdown
Member

@jasnell jasnell Mar 24, 2017

Choose a reason for hiding this comment

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

comma at the end of this line... did you mean to put a + here instead?
@Fishrock123
Copy link
Copy Markdown
Contributor Author

Fishrock123 commented Mar 27, 2017

ok, looks like there's more to it than this. Something loads path during startup which explodes regardless:

path.js:1163
          cwd = process.cwd();
                        ^

Error: ENOENT: no such file or directory, uv_cwd
    at Object.resolve (path.js:1163:25)
    at Function.Module._initPaths (module.js:642:22)
    at module.js:683:8
    at NativeModule.compile (bootstrap_node.js:548:7)
    at Function.NativeModule.require (bootstrap_node.js:489:18)
    at evalScript (bootstrap_node.js:390:33)
    at run (bootstrap_node.js:121:11)
    at run (bootstrap_node.js:445:7)
    at startup (bootstrap_node.js:120:9)
    at bootstrap_node.js:560:3
@Fishrock123
Copy link
Copy Markdown
Contributor Author

Fishrock123 commented Mar 27, 2017

I think the solution there would probably be to overload process.cwd() entirely on load?
@Fishrock123 Fishrock123 force-pushed the warn-on-inaccesible-cwd branch from f381a70 to f14c96f Compare March 27, 2017 16:43
@cjihrig
Copy link
Copy Markdown
Contributor

cjihrig commented Apr 11, 2017

The cwd being removed is only supported for evaluating code. Not running "normally." I added a known issue test in #12343. We could add a better message for other scenarios, or set the cwd like we do for eval script.
cjihrig added a commit to cjihrig/node that referenced this pull request Apr 18, 2017
@cjihrig
test: add cwd ENOENT known issue test
4fc1199 
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Refs: nodejs#12022
PR-URL: nodejs#12343
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@BridgeAR BridgeAR added the stalled Issues and PRs that are stalled. label Aug 26, 2017
@BridgeAR
Copy link
Copy Markdown
Member

BridgeAR commented Aug 26, 2017

Ping @Fishrock123
@BridgeAR
Copy link
Copy Markdown
Member

BridgeAR commented Sep 12, 2017

Closing this due to a long inactivity. Please feel free to reopen if you want to follow up on this @Fishrock123
@BridgeAR BridgeAR closed this Sep 12, 2017
Trott pushed a commit to Trott/io.js that referenced this pull request Sep 29, 2017
@cjihrig @Trott
test: add cwd ENOENT known issue test
1f4b70b 
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Refs: nodejs#12022
PR-URL: nodejs#12343
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 10, 2017
@cjihrig @MylesBorins
test: add cwd ENOENT known issue test
0597d3f 
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Backport-PR-URL: #15479
Refs: #12022
PR-URL: #12343
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit that referenced this pull request Oct 25, 2017
@cjihrig @MylesBorins
test: add cwd ENOENT known issue test
d8f5637 
If the current working directory is removed, Node cannot
start normally because the module system calls uv_cwd().

Backport-PR-URL: #15479
Refs: #12022
PR-URL: #12343
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lib / src Issues and PRs related to general changes in the lib or src directory. stalled Issues and PRs that are stalled.

6 participants

@Fishrock123 @addaleax @cjihrig @BridgeAR @jasnell @nodejs-github-bot