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

rmdirsync doesn't throw error when symlinked path is passed as argument in windows node 12 #31231

Open
SparshithNR opened this issue Jan 6, 2020 · 3 comments
Labels
fs Issues and PRs related to the fs subsystem / file system.

Comments

@SparshithNR
Copy link

I have written a simple code to pin down the issue.
Env: Windows
Node version: 12

const fs = require('fs');

fs.mkdirSync("tmp");
fs.symlinkSync(".", "tmp/out");
console.log(`fs.existSync(tmp/out) : ${fs.existsSync('tmp/out')}`);
fs.rmdirSync('tmp/out');

Execution in CI:
Node 12

Build started
git clone -q --branch=rmdirsync https://github.com/SparshithNR/node-symlink.git C:\projects\node-symlink
git checkout -qf a6cb35fc2b3d2df328d4aeae3db65d732243142c
Restoring build cache
Cache 'C:\Users\appveyor\AppData\Local\Yarn' - Restored
Running Install scripts
Install-Product node $env:nodejs_version
Uninstalling node 8.16.2 (x86)...
Installing node 12.13.1 (x86)...
git rev-parse HEAD
a6cb35fc2b3d2df328d4aeae3db65d732243142c
yarn run test
yarn run v1.16.0
$ node index.js
fs.existSync(tmp/out) : true
Done in 0.27s.
Updating build cache...
Cache 'C:\Users\appveyor\AppData\Local\Yarn' - Up to date
Build success

Node 10:

Build started
git clone -q --branch=rmdirsync https://github.com/SparshithNR/node-symlink.git C:\projects\node-symlink
git checkout -qf a6cb35fc2b3d2df328d4aeae3db65d732243142c
Restoring build cache
Cache 'C:\Users\appveyor\AppData\Local\Yarn' - Restored
Running Install scripts
Install-Product node $env:nodejs_version
Uninstalling node 8.16.2 (x86)...
Installing node 10.17.0 (x86)...
git rev-parse HEAD
a6cb35fc2b3d2df328d4aeae3db65d732243142c
yarn run test
yarn run v1.16.0
$ node index.js
fs.existSync(tmp/out) : true
fs.js:114
    throw err;
    ^
Error: ENOENT: no such file or directory, rmdir 'tmp/out'
    at Object.rmdirSync (fs.js:684:3)
    at Object.<anonymous> (C:\projects\node-symlink\index.js:6:4)
    at Module._compile (internal/modules/cjs/loader.js:778:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:831:12)
    at startup (internal/bootstrap/node.js:283:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:623:3)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
Command exited with code 1

Code can be found here: https://github.com/SparshithNR/node-symlink/tree/rmdirsync
Windows Execution: https://ci.appveyor.com/project/SparshithNR/node-symlink/builds/29933274
Linux Execution: https://github.com/SparshithNR/node-symlink/runs/376483962

@Hakerh400
Copy link
Contributor

I think the bahavior in Node.js v12.x is correct. It should not throw an error. The error you see in v10.x was a bug (or a feature), which was fixed in #23724. Function fs.symlinkSync required third parameter, which can be either 'file' or 'dir' (default was 'file'), so fs.rmdirSync fails because the symlink points to an inaccessible file.

@SparshithNR
Copy link
Author

SparshithNR commented Jan 7, 2020

@Hakerh400fs.rmdirSync is failing in node 10 not in node 12. So is it expected not to do anything and silently return in node 12?

@SparshithNR
Copy link
Author

SparshithNR commented Jan 7, 2020

One more question is why the behavior is different in the different platforms?
Linux: Throws error
Mac: Throws Error
Windows: Doesn't throw an error.

Please have look at the below terminal output from Linux CI.

$ node --version
v12.14.0
$ npm --version
6.13.4
$ nvm --version
0.35.2
install.npm
0.57s$ npm install 
0.20s$ npm test
> node-symlink@1.0.0 test /home/travis/build/SparshithNR/node-symlink
> node index.js
fs.existSync(tmp/out) : true
internal/fs/utils.js:220
    throw err;
    ^
Error: ENOTDIR: not a directory, rmdir 'tmp/out'
    at Object.rmdirSync (fs.js:752:3)
    at Object.<anonymous> (/home/travis/build/SparshithNR/node-symlink/index.js:6:4)
    at Module._compile (internal/modules/cjs/loader.js:959:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:995:10)
    at Module.load (internal/modules/cjs/loader.js:815:32)
    at Function.Module._load (internal/modules/cjs/loader.js:727:14)
    at Function.Module.runMain (internal/modules/cjs/loader.js:1047:10)
    at internal/main/run_main_module.js:17:11 {
  errno: -20,
  syscall: 'rmdir',
  code: 'ENOTDIR',
  path: 'tmp/out'
}

https://travis-ci.com/SparshithNR/node-symlink/jobs/272882479

fs.rmdirSync throwing error in node 12 Linux and not in node 12 windows forces us to code with a branching.

@targos targos added the fs Issues and PRs related to the fs subsystem / file system. label Dec 26, 2020
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.
Projects
None yet
Development

No branches or pull requests

3 participants