Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upfs: inconsistent options treatment between rm() and rmdir() #35689
Comments
|
@iansu @bcoe @cjihrig @nodejs/fs I think the right thing to do here is for both methods to honor |
|
I agree that they should both treat In your example it also seems strange that |
It produces an error, but my callback swallows it. |
For non-recursive calls, I wonder if retries still make sense when dealing with NFS, for example. |
I think it makes sense, not having a file inside is not the only reason for an unsuccessful deletion, it would be nice to have maxRetries in this case too |
I think I don't hold this opinion particularly strongly, I just can't see retries being as useful outside the context of Edit: it's called maxBusyTries in rimraf, but I think it does the same thing. |
What steps will reproduce the bug?
Use
/dev/nullas the path like below, or use some other path that will not be removed and force a retry.This tries, doesn't remove
/dev/null, and exits with "done" very quickly.$ node -e 'fs.rmdir("/dev/null", { maxRetries: 42 }, () => { console.log("done"); })'So does this:
$ node -e 'fs.rmdir("/dev/null", { maxRetries: 420 }, () => { console.log("done"); })'This does the same:
$ node -e 'fs.rm("/dev/null", { maxRetries: 1 }, () => { console.log("done"); })'But this takes about 90 seconds to finish, presumably because it is respecting the
maxRetriesoption (with a backoff, I imagine) whereasfs.rmdir()ignores it ifrecursiveis not set:$ node -e 'fs.rm("/dev/null", { maxRetries: 42 }, () => { console.log("done"); })'How often does it reproduce? Is there a required condition?
Reproduces every time.
What is the expected behavior?
I would expect
fs.rm()andfs.rmdir()to treatmaxRetriesthe same.What do you see instead?
fs.rm()honors it with or withoutrecursivebeing set, whereasfs.rmdir()ignores it unlessrecursiveis set.Additional information