Skip to content

Commit 4b1790e

Browse files
lukekarryslukekarrys
committed
eperm stuff
eperm stuff make sure other test still run guard against eperm during windows readdir retry more idk coverage log less less test assertions for CI output add stack traces add stack traces retry all FS during moveRemove fix auto import remove stack traces normal ci
1 parent d7b3cd4 commit 4b1790e

15 files changed

+456
-196
lines changed

‎.github/workflows/ci.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ jobs:
3636
run: npm install
3737

3838
- name: Run Tests
39-
run: npm test -- -t0 -c
39+
run: npm test
4040
env:
4141
RIMRAF_TEST_START_CHAR: a
4242
RIMRAF_TEST_END_CHAR: f

‎.github/workflows/eperm.yml

+31
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
name: CI
2+
3+
on:
4+
workflow_dispatch:
5+
inputs:
6+
iterations:
7+
default: '20000'
8+
9+
jobs:
10+
test:
11+
runs-on: windows-latest
12+
defaults:
13+
run:
14+
shell: bash
15+
16+
steps:
17+
- name: Checkout Repository
18+
uses: actions/checkout@v4
19+
20+
- name: Use Nodejs 22.x
21+
uses: actions/setup-node@v4
22+
with:
23+
node-version: 22.x
24+
25+
- name: Install dependencies
26+
run: npm install
27+
28+
- name: Run Tests
29+
run: npm test -- test/integration/eperm.ts
30+
env:
31+
RIMRAF_TEST_EPERM_ITERATIONS: ${{ inputs.iterations }}

‎package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,7 @@
9393
],
9494
"module": "./dist/esm/index.js",
9595
"tap": {
96-
"coverage-map": "map.js"
96+
"coverage-map": "map.js",
97+
"timeout": 0
9798
}
9899
}

‎src/fs.ts

+6-26
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,14 @@
11
// promisify ourselves, because older nodes don't have fs.promises
22

33
import fs, { Dirent } from 'fs'
4-
import { readdirSync as rdSync } from 'fs'
54

65
// sync ones just take the sync version from node
7-
export {
8-
chmodSync,
9-
mkdirSync,
10-
renameSync,
11-
rmdirSync,
12-
rmSync,
13-
statSync,
14-
lstatSync,
15-
unlinkSync,
16-
} from 'fs'
6+
export { chmodSync, renameSync, rmdirSync, rmSync, unlinkSync } from 'fs'
177

18-
export const readdirSync = (path: fs.PathLike): Dirent[] =>
19-
rdSync(path, { withFileTypes: true })
8+
export const statSync = (path: fs.PathLike) => fs.statSync(path)
9+
export const lstatSync = (path: fs.PathLike) => fs.lstatSync(path)
10+
export const readdirSync = (path: fs.PathLike) =>
11+
fs.readdirSync(path, { withFileTypes: true })
2012

2113
// unrolled for better inlining, this seems to get better performance
2214
// than something like:
@@ -26,19 +18,8 @@ export const readdirSync = (path: fs.PathLike): Dirent[] =>
2618
const chmod = (path: fs.PathLike, mode: fs.Mode): Promise<void> =>
2719
new Promise((res, rej) => fs.chmod(path, mode, er => (er ? rej(er) : res())))
2820

29-
const mkdir = (
30-
path: fs.PathLike,
31-
options?:
32-
| fs.Mode
33-
| (fs.MakeDirectoryOptions & { recursive?: boolean | null })
34-
| null,
35-
): Promise<string | undefined> =>
21+
const readdir = async (path: fs.PathLike): Promise<Dirent[]> =>
3622
new Promise((res, rej) =>
37-
fs.mkdir(path, options, (er, made) => (er ? rej(er) : res(made))),
38-
)
39-
40-
const readdir = (path: fs.PathLike): Promise<Dirent[]> =>
41-
new Promise<Dirent[]>((res, rej) =>
4223
fs.readdir(path, { withFileTypes: true }, (er, data) =>
4324
er ? rej(er) : res(data),
4425
),
@@ -70,7 +51,6 @@ const unlink = (path: fs.PathLike): Promise<void> =>
7051

7152
export const promises = {
7253
chmod,
73-
mkdir,
7454
readdir,
7555
rename,
7656
rm,

‎src/retry-busy.ts

+15-14
Original file line numberDiff line numberDiff line change
@@ -1,30 +1,28 @@
11
// note: max backoff is the maximum that any *single* backoff will do
22

33
import { setTimeout } from 'timers/promises'
4-
import { RimrafAsyncOptions, RimrafOptions } from './index.js'
4+
import { RimrafAsyncOptions, RimrafSyncOptions } from './index.js'
55
import { isFsError } from './error.js'
66

77
export const MAXBACKOFF = 200
88
export const RATE = 1.2
99
export const MAXRETRIES = 10
1010
export const codes = new Set(['EMFILE', 'ENFILE', 'EBUSY'])
1111

12-
export const retryBusy = <T>(fn: (path: string) => Promise<T>) => {
13-
const method = async (
14-
path: string,
15-
opt: RimrafAsyncOptions,
16-
backoff = 1,
17-
total = 0,
18-
) => {
12+
export const retryBusy = <T, U extends RimrafAsyncOptions>(
13+
fn: (path: string, opt: U) => Promise<T>,
14+
retryCodes: Set<string> = codes,
15+
) => {
16+
const method = async (path: string, opt: U, backoff = 1, total = 0) => {
1917
const mbo = opt.maxBackoff || MAXBACKOFF
2018
const rate = opt.backoff || RATE
2119
const max = opt.maxRetries || MAXRETRIES
2220
let retries = 0
2321
while (true) {
2422
try {
25-
return await fn(path)
23+
return await fn(path, opt)
2624
} catch (er) {
27-
if (isFsError(er) && er.path === path && codes.has(er.code)) {
25+
if (isFsError(er) && er.path === path && retryCodes.has(er.code)) {
2826
backoff = Math.ceil(backoff * rate)
2927
total = backoff + total
3028
if (total < mbo) {
@@ -45,18 +43,21 @@ export const retryBusy = <T>(fn: (path: string) => Promise<T>) => {
4543
}
4644

4745
// just retries, no async so no backoff
48-
export const retryBusySync = <T>(fn: (path: string) => T) => {
49-
const method = (path: string, opt: RimrafOptions) => {
46+
export const retryBusySync = <T, U extends RimrafSyncOptions>(
47+
fn: (path: string, opt: U) => T,
48+
retryCodes: Set<string> = codes,
49+
) => {
50+
const method = (path: string, opt: U) => {
5051
const max = opt.maxRetries || MAXRETRIES
5152
let retries = 0
5253
while (true) {
5354
try {
54-
return fn(path)
55+
return fn(path, opt)
5556
} catch (er) {
5657
if (
5758
isFsError(er) &&
5859
er.path === path &&
59-
codes.has(er.code) &&
60+
retryCodes.has(er.code) &&
6061
retries < max
6162
) {
6263
retries++

‎src/rimraf-move-remove.ts

+84-66
Original file line numberDiff line numberDiff line change
@@ -14,50 +14,109 @@
1414
import { basename, parse, resolve } from 'path'
1515
import { defaultTmp, defaultTmpSync } from './default-tmp.js'
1616
import { ignoreENOENT, ignoreENOENTSync } from './ignore-enoent.js'
17-
import { lstatSync, promises, renameSync, rmdirSync, unlinkSync } from './fs.js'
17+
import * as FS from './fs.js'
1818
import { Dirent, Stats } from 'fs'
1919
import { RimrafAsyncOptions, RimrafSyncOptions } from './index.js'
2020
import { readdirOrError, readdirOrErrorSync } from './readdir-or-error.js'
2121
import { fixEPERM, fixEPERMSync } from './fix-eperm.js'
2222
import { errorCode } from './error.js'
23-
const { lstat, rename, unlink, rmdir } = promises
23+
import { retryBusy, retryBusySync, codes } from './retry-busy.js'
24+
25+
type Tmp<T extends RimrafAsyncOptions | RimrafSyncOptions> = T & {
26+
tmp: string
27+
}
2428

2529
// crypto.randomBytes is much slower, and Math.random() is enough here
26-
const uniqueFilename = (path: string) => `.${basename(path)}.${Math.random()}`
30+
const uniqueName = (path: string, tmp: string) =>
31+
resolve(tmp, `.${basename(path)}.${Math.random()}`)
32+
33+
// moveRemove is the fallback on Windows and due to flaky EPERM errors
34+
// if we are actually on Windows, then we add EPERM to the list of
35+
// error codes that we treat as busy, as well as retrying all fs
36+
// operations for EPERM only.
37+
const isWin = process.platform === 'win32'
2738

28-
const unlinkFixEPERM = fixEPERM(unlink)
29-
const unlinkFixEPERMSync = fixEPERMSync(unlinkSync)
39+
// all fs functions are only retried for EPERM and only on windows
40+
const retryFsCodes = isWin ? new Set(['EPERM']) : undefined
41+
const maybeRetry = <T, U extends RimrafAsyncOptions>(
42+
fn: (path: string, opt: U) => Promise<T>,
43+
) => (retryFsCodes ? retryBusy(fn, retryFsCodes) : fn)
44+
const maybeRetrySync = <T, U extends RimrafSyncOptions>(
45+
fn: (path: string, opt: U) => T,
46+
) => (retryFsCodes ? retryBusySync(fn, retryFsCodes) : fn)
47+
const rename = maybeRetry(
48+
async (path: string, opt: Tmp<RimrafAsyncOptions>) => {
49+
const newPath = uniqueName(path, opt.tmp)
50+
await FS.promises.rename(path, newPath)
51+
return newPath
52+
},
53+
)
54+
const renameSync = maybeRetrySync(
55+
(path: string, opt: Tmp<RimrafSyncOptions>) => {
56+
const newPath = uniqueName(path, opt.tmp)
57+
FS.renameSync(path, newPath)
58+
return newPath
59+
},
60+
)
61+
const readdir = maybeRetry(readdirOrError)
62+
const readdirSync = maybeRetrySync(readdirOrErrorSync)
63+
const lstat = maybeRetry(FS.promises.lstat)
64+
const lstatSync = maybeRetrySync(FS.lstatSync)
65+
66+
// unlink and rmdir and always retryable regardless of platform
67+
// but we add the EPERM error code as a busy signal on Windows only
68+
const retryCodes = new Set([...codes, ...(retryFsCodes || [])])
69+
const unlink = retryBusy(fixEPERM(FS.promises.unlink), retryCodes)
70+
const unlinkSync = retryBusySync(fixEPERMSync(FS.unlinkSync), retryCodes)
71+
const rmdir = retryBusy(fixEPERM(FS.promises.rmdir), retryCodes)
72+
const rmdirSync = retryBusySync(fixEPERMSync(FS.rmdirSync), retryCodes)
3073

3174
export const rimrafMoveRemove = async (
3275
path: string,
33-
opt: RimrafAsyncOptions,
76+
{ tmp, ...opt }: RimrafAsyncOptions,
3477
) => {
3578
opt?.signal?.throwIfAborted()
79+
80+
tmp ??= await defaultTmp(path)
81+
if (path === tmp && parse(path).root !== path) {
82+
throw new Error('cannot delete temp directory used for deletion')
83+
}
84+
3685
return (
3786
(await ignoreENOENT(
38-
lstat(path).then(stat => rimrafMoveRemoveDir(path, opt, stat)),
87+
lstat(path, opt).then(stat =>
88+
rimrafMoveRemoveDir(path, { ...opt, tmp }, stat),
89+
),
3990
)) ?? true
4091
)
4192
}
4293

94+
export const rimrafMoveRemoveSync = (
95+
path: string,
96+
{ tmp, ...opt }: RimrafSyncOptions,
97+
) => {
98+
opt?.signal?.throwIfAborted()
99+
100+
tmp ??= defaultTmpSync(path)
101+
if (path === tmp && parse(path).root !== path) {
102+
throw new Error('cannot delete temp directory used for deletion')
103+
}
104+
105+
return (
106+
ignoreENOENTSync(() =>
107+
rimrafMoveRemoveDirSync(path, { ...opt, tmp }, lstatSync(path, opt)),
108+
) ?? true
109+
)
110+
}
111+
43112
const rimrafMoveRemoveDir = async (
44113
path: string,
45-
opt: RimrafAsyncOptions,
114+
opt: Tmp<RimrafAsyncOptions>,
46115
ent: Dirent | Stats,
47116
): Promise<boolean> => {
48117
opt?.signal?.throwIfAborted()
49-
if (!opt.tmp) {
50-
return rimrafMoveRemoveDir(
51-
path,
52-
{ ...opt, tmp: await defaultTmp(path) },
53-
ent,
54-
)
55-
}
56-
if (path === opt.tmp && parse(path).root !== path) {
57-
throw new Error('cannot delete temp directory used for deletion')
58-
}
59118

60-
const entries = ent.isDirectory() ? await readdirOrError(path) : null
119+
const entries = ent.isDirectory() ? await readdir(path, opt) : null
61120
if (!Array.isArray(entries)) {
62121
// this can only happen if lstat/readdir lied, or if the dir was
63122
// swapped out with a file at just the right moment.
@@ -74,7 +133,7 @@ const rimrafMoveRemoveDir = async (
74133
if (opt.filter && !(await opt.filter(path, ent))) {
75134
return false
76135
}
77-
await ignoreENOENT(tmpUnlink(path, opt.tmp, unlinkFixEPERM))
136+
await ignoreENOENT(rename(path, opt).then(p => unlink(p, opt)))
78137
return true
79138
}
80139

@@ -98,49 +157,18 @@ const rimrafMoveRemoveDir = async (
98157
if (opt.filter && !(await opt.filter(path, ent))) {
99158
return false
100159
}
101-
await ignoreENOENT(tmpUnlink(path, opt.tmp, rmdir))
160+
await ignoreENOENT(rename(path, opt).then(p => rmdir(p, opt)))
102161
return true
103162
}
104163

105-
const tmpUnlink = async <T>(
106-
path: string,
107-
tmp: string,
108-
rm: (p: string) => Promise<T>,
109-
) => {
110-
const tmpFile = resolve(tmp, uniqueFilename(path))
111-
await rename(path, tmpFile)
112-
return await rm(tmpFile)
113-
}
114-
115-
export const rimrafMoveRemoveSync = (path: string, opt: RimrafSyncOptions) => {
116-
opt?.signal?.throwIfAborted()
117-
return (
118-
ignoreENOENTSync(() =>
119-
rimrafMoveRemoveDirSync(path, opt, lstatSync(path)),
120-
) ?? true
121-
)
122-
}
123-
124164
const rimrafMoveRemoveDirSync = (
125165
path: string,
126-
opt: RimrafSyncOptions,
166+
opt: Tmp<RimrafSyncOptions>,
127167
ent: Dirent | Stats,
128168
): boolean => {
129169
opt?.signal?.throwIfAborted()
130-
if (!opt.tmp) {
131-
return rimrafMoveRemoveDirSync(
132-
path,
133-
{ ...opt, tmp: defaultTmpSync(path) },
134-
ent,
135-
)
136-
}
137-
const tmp: string = opt.tmp
138170

139-
if (path === opt.tmp && parse(path).root !== path) {
140-
throw new Error('cannot delete temp directory used for deletion')
141-
}
142-
143-
const entries = ent.isDirectory() ? readdirOrErrorSync(path) : null
171+
const entries = ent.isDirectory() ? readdirSync(path, opt) : null
144172
if (!Array.isArray(entries)) {
145173
// this can only happen if lstat/readdir lied, or if the dir was
146174
// swapped out with a file at just the right moment.
@@ -157,7 +185,7 @@ const rimrafMoveRemoveDirSync = (
157185
if (opt.filter && !opt.filter(path, ent)) {
158186
return false
159187
}
160-
ignoreENOENTSync(() => tmpUnlinkSync(path, tmp, unlinkFixEPERMSync))
188+
ignoreENOENTSync(() => unlinkSync(renameSync(path, opt), opt))
161189
return true
162190
}
163191

@@ -175,16 +203,6 @@ const rimrafMoveRemoveDirSync = (
175203
if (opt.filter && !opt.filter(path, ent)) {
176204
return false
177205
}
178-
ignoreENOENTSync(() => tmpUnlinkSync(path, tmp, rmdirSync))
206+
ignoreENOENTSync(() => rmdirSync(renameSync(path, opt), opt))
179207
return true
180208
}
181-
182-
const tmpUnlinkSync = (
183-
path: string,
184-
tmp: string,
185-
rmSync: (p: string) => void,
186-
) => {
187-
const tmpFile = resolve(tmp, uniqueFilename(path))
188-
renameSync(path, tmpFile)
189-
return rmSync(tmpFile)
190-
}

0 commit comments

Comments
 (0)