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

fs.rmdir() on file (not directory) has "ENOTDIR" on Linux/macOS vs "ENOENT" on Windows #8797

Closed
jokeyrhyme opened this issue Sep 27, 2016 · 11 comments
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.

Comments

@jokeyrhyme
Copy link

jokeyrhyme commented Sep 27, 2016

  • Version: 6.6.0
  • Platform: Darwin fulmen.local 16.1.0 Darwin Kernel Version 16.1.0: Mon Sep 12 19:36:59 PDT 2016; root:xnu-3789.20.43~78/RELEASE_X86_64 x86_64
  • Subsystem: "fs" module

Just noticed an inconsistency between Windows and everything else (why, Windows, why?!) for the fs.rmdir() and fs.rmdirSync() built-in functions.

const fs = require('fs')
const path = require('path')

fs.rmdirSync(__filename)

Assuming you save this to a file and node it, this file deletes itself on Windows instead of throwing an error as on Linux and macOS.

Note that fs.unlink() and fs.unlinkSync() consistently result in Errors if used on a directory's path.

Update: the snippet results in an "ENOENT" error on Windows, instead of an "ENOTDIR" error as on Linux / macOS

@mscdex mscdex added the fs Issues and PRs related to the fs subsystem / file system. label Sep 27, 2016
@silverwind
Copy link
Contributor

What version of Windows? I cannot reproduce on Windows 7 (getting ENOENT, while the docs would suggest ENOTEMPTY to be thrown).

@jokeyrhyme
Copy link
Author

Well, I was writing this library: https://github.com/jokeyrhyme/idempotent-fs.js

And the AppVeyor CI tests fail for related assertions: https://ci.appveyor.com/project/jokeyrhyme/idempotent-fs-js/build/1.0.4/job/sd2snlj4iv5ya0oi

I'll run this against versions of Windows I can control today. It's possible that it's file-system related. Maybe there's a difference between FAT32 and NTFS?

@silverwind
Copy link
Contributor

Maybe there's a difference between FAT32 and NTFS?

I'd say it's unlikely. Probably more likely that the error is swallowed for files on network drivers. One thing I'd watch out while testing is to try removing a different file than the current script because there might be a active handle (resulting in EACCES).

@jokeyrhyme
Copy link
Author

This particular test is deliberately trying to get an error, and I was lazily avoiding the need to create temporary directories. :P But yes, not a great idea generally.

@jokeyrhyme jokeyrhyme changed the title fs.rmdir() on file (not directory) has Error result on Linux/macOS, no Error on Windows fs.rmdir() on file (not directory) has "ENOTDIR" on Linux/macOS vs "ENOENT" on Windows Sep 28, 2016
@jokeyrhyme
Copy link
Author

@silverwind yeah, after some testing on Windows 10 on an NTFS partition in VirtualBox, I can see that fs.rmdir(__filename) throws "ENOENT", which is different to the "ENOTDIR" error I was getting on Linux / macOS. There's still something fishy here, but it isn't exactly what I initially reported.

@silverwind
Copy link
Contributor

Do I understand correctly that the error was always there, just with a different code?

@jokeyrhyme
Copy link
Author

@silverwind yep. I've updated the Issue title and description. I have found that the error codes are inconsistent. We just have to determine now if this is worth fixing, or if it is minor enough to just ignore.

@silverwind
Copy link
Contributor

silverwind commented Sep 30, 2016

@nodejs/platform-windows should we attempt to normalize error codes in this case? The issue here is that Unix has a ENOTDIR while the Windows API does not know this code.

sebn added a commit to cozy-labs/cozy-desktop that referenced this issue Jul 6, 2017
@Trott
Copy link
Member

Trott commented Jul 10, 2017

Re-pinging @nodejs/platform-windows. Also: @nodejs/fs.

Keep this open?

@cjihrig
Copy link
Contributor

cjihrig commented Jul 10, 2017

I think this is something we could document. ENOTDIR seems like the more correct error code, but normalizing that on Windows would probably not be worth it.

@refack refack added doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. labels Jul 11, 2017
@refack
Copy link
Contributor

refack commented Jul 11, 2017

Note for those coming after...
This idiosyncrasy should be documented in https://github.com/nodejs/node/blob/master/doc/api/fs.md#fsrmdirpath-callback

al-k21 pushed a commit to al-k21/node that referenced this issue Jul 18, 2017
fs.rmdir() on the file (not directory) results in different errors on
Windows to everything else

fixes: nodejs#8797
@refack refack added the wip Issues and PRs that are still a work in progress. label Jul 19, 2017
@refack refack removed the wip Issues and PRs that are still a work in progress. label Jul 19, 2017
addaleax pushed a commit that referenced this issue Jul 22, 2017
fs.rmdir() on the file (not directory) results in different errors on
Windows to everything else

Fixes: #8797
PR-URL: #14323
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Fishrock123 pushed a commit that referenced this issue Jul 24, 2017
fs.rmdir() on the file (not directory) results in different errors on
Windows to everything else

Fixes: #8797
PR-URL: #14323
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this issue Aug 16, 2017
fs.rmdir() on the file (not directory) results in different errors on
Windows to everything else

Fixes: #8797
PR-URL: #14323
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this issue Aug 16, 2017
fs.rmdir() on the file (not directory) results in different errors on
Windows to everything else

Fixes: #8797
PR-URL: #14323
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 3, 2017
fs.rmdir() on the file (not directory) results in different errors on
Windows to everything else

Fixes: #8797
PR-URL: #14323
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
MylesBorins pushed a commit that referenced this issue Sep 5, 2017
fs.rmdir() on the file (not directory) results in different errors on
Windows to everything else

Fixes: #8797
PR-URL: #14323
Reviewed-By: Vse Mozhet Byt <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Gibson Fahnestock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. fs Issues and PRs related to the fs subsystem / file system. good first issue Issues that are suitable for first-time contributors.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants