diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 73c0e451..a675374a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -3,6 +3,46 @@ name: CI on: [push, pull_request] jobs: + eperm: + runs-on: windows-latest + defaults: + run: + shell: bash + + steps: + - name: Checkout Repository + uses: actions/checkout@v4 + + - name: Use Nodejs 22.x + uses: actions/setup-node@v4 + with: + node-version: 22.x + + - name: Install dependencies + run: npm install + + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + - run: npm test -- -t0 test/integration/eperm.ts --disable-coverage --grep=. --grep=async + build: strategy: matrix: diff --git a/src/fs.ts b/src/fs.ts index 45abbcb2..f31d605e 100644 --- a/src/fs.ts +++ b/src/fs.ts @@ -79,3 +79,35 @@ export const promises = { lstat, unlink, } + +// import fs, { Dirent } from 'fs' +// import fsPromises from 'fs/promises' + +// export { +// chmodSync, +// mkdirSync, +// renameSync, +// rmdirSync, +// rmSync, +// statSync, +// lstatSync, +// unlinkSync, +// } from 'fs' + +// export const readdirSync = (path: fs.PathLike): Dirent[] => +// fs.readdirSync(path, { withFileTypes: true }) + +// const readdir = (path: fs.PathLike): Promise => +// fsPromises.readdir(path, { withFileTypes: true }) + +// export const promises = { +// chmod: fsPromises.chmod, +// mkdir: fsPromises.mkdir, +// readdir, +// rename: fsPromises.rename, +// rm: fsPromises.rm, +// rmdir: fsPromises.rmdir, +// stat: fsPromises.stat, +// lstat: fsPromises.lstat, +// unlink: fsPromises.unlink, +// } diff --git a/src/retry-busy.ts b/src/retry-busy.ts index 40ceb31d..b0d099bc 100644 --- a/src/retry-busy.ts +++ b/src/retry-busy.ts @@ -9,7 +9,10 @@ export const RATE = 1.2 export const MAXRETRIES = 10 export const codes = new Set(['EMFILE', 'ENFILE', 'EBUSY']) -export const retryBusy = (fn: (path: string) => Promise) => { +export const retryBusy = ( + fn: (path: string) => Promise, + extraCodes?: Set, +) => { const method = async ( path: string, opt: RimrafAsyncOptions, @@ -24,9 +27,22 @@ export const retryBusy = (fn: (path: string) => Promise) => { try { return await fn(path) } catch (er) { - if (isFsError(er) && er.path === path && codes.has(er.code)) { + if ( + isFsError(er) && + er.path === path && + (codes.has(er.code) || extraCodes?.has(er.code)) + ) { backoff = Math.ceil(backoff * rate) total = backoff + total + if (er.code === 'EPERM') { + console.trace('EPERM retry busy', { + error: er, + total, + mbo, + retries, + backoff, + }) + } if (total < mbo) { await setTimeout(backoff) return method(path, opt, backoff, total) @@ -45,7 +61,10 @@ export const retryBusy = (fn: (path: string) => Promise) => { } // just retries, no async so no backoff -export const retryBusySync = (fn: (path: string) => T) => { +export const retryBusySync = ( + fn: (path: string) => T, + extraCodes?: Set, +) => { const method = (path: string, opt: RimrafOptions) => { const max = opt.maxRetries || MAXRETRIES let retries = 0 @@ -56,7 +75,7 @@ export const retryBusySync = (fn: (path: string) => T) => { if ( isFsError(er) && er.path === path && - codes.has(er.code) && + (codes.has(er.code) || extraCodes?.has(er.code)) && retries < max ) { retries++ diff --git a/src/rimraf-move-remove.ts b/src/rimraf-move-remove.ts index f6a3ef1b..1adb44f7 100644 --- a/src/rimraf-move-remove.ts +++ b/src/rimraf-move-remove.ts @@ -20,42 +20,43 @@ import { RimrafAsyncOptions, RimrafSyncOptions } from './index.js' import { readdirOrError, readdirOrErrorSync } from './readdir-or-error.js' import { fixEPERM, fixEPERMSync } from './fix-eperm.js' import { errorCode } from './error.js' +import { retryBusy, retryBusySync } from './retry-busy.js' const { lstat, rename, unlink, rmdir } = promises // crypto.randomBytes is much slower, and Math.random() is enough here const uniqueFilename = (path: string) => `.${basename(path)}.${Math.random()}` -const unlinkFixEPERM = fixEPERM(unlink) -const unlinkFixEPERMSync = fixEPERMSync(unlinkSync) +const retryCodes = new Set(['EPERM']) +const unlinkFixEPERM = retryBusy(fixEPERM(unlink), retryCodes) +const unlinkFixEPERMSync = retryBusySync(fixEPERMSync(unlinkSync), retryCodes) +const rmdirFixEPERM = retryBusy(fixEPERM(rmdir), retryCodes) +const rmdirFixEPERMSync = retryBusySync(fixEPERMSync(rmdirSync), retryCodes) export const rimrafMoveRemove = async ( path: string, - opt: RimrafAsyncOptions, + { tmp, ...opt }: RimrafAsyncOptions, ) => { opt?.signal?.throwIfAborted() + + tmp ??= await defaultTmp(path) + if (path === tmp && parse(path).root !== path) { + throw new Error('cannot delete temp directory used for deletion') + } + return ( (await ignoreENOENT( - lstat(path).then(stat => rimrafMoveRemoveDir(path, opt, stat)), + lstat(path).then(stat => rimrafMoveRemoveDir(path, tmp, opt, stat)), )) ?? true ) } const rimrafMoveRemoveDir = async ( path: string, - opt: RimrafAsyncOptions, + tmp: string, + opt: Omit, ent: Dirent | Stats, ): Promise => { opt?.signal?.throwIfAborted() - if (!opt.tmp) { - return rimrafMoveRemoveDir( - path, - { ...opt, tmp: await defaultTmp(path) }, - ent, - ) - } - if (path === opt.tmp && parse(path).root !== path) { - throw new Error('cannot delete temp directory used for deletion') - } const entries = ent.isDirectory() ? await readdirOrError(path) : null if (!Array.isArray(entries)) { @@ -66,6 +67,13 @@ const rimrafMoveRemoveDir = async ( if (errorCode(entries) === 'ENOENT') { return true } + if (errorCode(entries) === 'EPERM') { + // TODO: what to do here?? + console.trace('EPERM reading dir uring move remove', { + error: entries, + }) + throw entries + } if (errorCode(entries) !== 'ENOTDIR') { throw entries } @@ -74,14 +82,14 @@ const rimrafMoveRemoveDir = async ( if (opt.filter && !(await opt.filter(path, ent))) { return false } - await ignoreENOENT(tmpUnlink(path, opt.tmp, unlinkFixEPERM)) + await ignoreENOENT(tmpUnlink(path, tmp, opt, unlinkFixEPERM)) return true } const removedAll = ( await Promise.all( entries.map(ent => - rimrafMoveRemoveDir(resolve(path, ent.name), opt, ent), + rimrafMoveRemoveDir(resolve(path, ent.name), tmp, opt, ent), ), ) ).every(v => v === true) @@ -98,47 +106,46 @@ const rimrafMoveRemoveDir = async ( if (opt.filter && !(await opt.filter(path, ent))) { return false } - await ignoreENOENT(tmpUnlink(path, opt.tmp, rmdir)) + await ignoreENOENT(tmpUnlink(path, tmp, opt, rmdirFixEPERM)) return true } -const tmpUnlink = async ( +const tmpUnlink = async ( path: string, tmp: string, - rm: (p: string) => Promise, + opt: RimrafAsyncOptions, + rm: (p: string, opt: RimrafAsyncOptions) => Promise, ) => { const tmpFile = resolve(tmp, uniqueFilename(path)) await rename(path, tmpFile) - return await rm(tmpFile) + await rm(tmpFile, opt) } -export const rimrafMoveRemoveSync = (path: string, opt: RimrafSyncOptions) => { +export const rimrafMoveRemoveSync = ( + path: string, + { tmp, ...opt }: RimrafSyncOptions, +) => { opt?.signal?.throwIfAborted() + + tmp ??= defaultTmpSync(path) + if (path === tmp && parse(path).root !== path) { + throw new Error('cannot delete temp directory used for deletion') + } + return ( ignoreENOENTSync(() => - rimrafMoveRemoveDirSync(path, opt, lstatSync(path)), + rimrafMoveRemoveDirSync(path, tmp, opt, lstatSync(path)), ) ?? true ) } const rimrafMoveRemoveDirSync = ( path: string, - opt: RimrafSyncOptions, + tmp: string, + opt: Omit, ent: Dirent | Stats, ): boolean => { opt?.signal?.throwIfAborted() - if (!opt.tmp) { - return rimrafMoveRemoveDirSync( - path, - { ...opt, tmp: defaultTmpSync(path) }, - ent, - ) - } - const tmp: string = opt.tmp - - if (path === opt.tmp && parse(path).root !== path) { - throw new Error('cannot delete temp directory used for deletion') - } const entries = ent.isDirectory() ? readdirOrErrorSync(path) : null if (!Array.isArray(entries)) { @@ -149,6 +156,13 @@ const rimrafMoveRemoveDirSync = ( if (errorCode(entries) === 'ENOENT') { return true } + if (errorCode(entries) === 'EPERM') { + // TODO: what to do here?? + console.trace('EPERM reading dir uring move remove', { + error: entries, + }) + throw entries + } if (errorCode(entries) !== 'ENOTDIR') { throw entries } @@ -157,14 +171,14 @@ const rimrafMoveRemoveDirSync = ( if (opt.filter && !opt.filter(path, ent)) { return false } - ignoreENOENTSync(() => tmpUnlinkSync(path, tmp, unlinkFixEPERMSync)) + ignoreENOENTSync(() => tmpUnlinkSync(path, tmp, opt, unlinkFixEPERMSync)) return true } let removedAll = true for (const ent of entries) { const p = resolve(path, ent.name) - removedAll = rimrafMoveRemoveDirSync(p, opt, ent) && removedAll + removedAll = rimrafMoveRemoveDirSync(p, tmp, opt, ent) && removedAll } if (!removedAll) { return false @@ -175,16 +189,17 @@ const rimrafMoveRemoveDirSync = ( if (opt.filter && !opt.filter(path, ent)) { return false } - ignoreENOENTSync(() => tmpUnlinkSync(path, tmp, rmdirSync)) + ignoreENOENTSync(() => tmpUnlinkSync(path, tmp, opt, rmdirFixEPERMSync)) return true } const tmpUnlinkSync = ( path: string, tmp: string, - rmSync: (p: string) => void, + opt: RimrafSyncOptions, + rmSync: (p: string, opt: RimrafSyncOptions) => unknown, ) => { const tmpFile = resolve(tmp, uniqueFilename(path)) renameSync(path, tmpFile) - return rmSync(tmpFile) + rmSync(tmpFile, opt) } diff --git a/src/rimraf-windows.ts b/src/rimraf-windows.ts index 567e7143..29bb1813 100644 --- a/src/rimraf-windows.ts +++ b/src/rimraf-windows.ts @@ -37,7 +37,7 @@ const rimrafWindowsDirMoveRemoveFallback = async ( await rimrafWindowsDirRetry(path, opt) return true } catch (er) { - if (errorCode(er) === 'ENOTEMPTY') { + if (errorCode(er) === 'ENOTEMPTY' || errorCode(er) === 'EPERM') { return rimrafMoveRemove(path, opt) } throw er @@ -55,7 +55,7 @@ const rimrafWindowsDirMoveRemoveFallbackSync = ( rimrafWindowsDirRetrySync(path, opt) return true } catch (er) { - if (errorCode(er) === 'ENOTEMPTY') { + if (errorCode(er) === 'ENOTEMPTY' || errorCode(er) === 'EPERM') { return rimrafMoveRemoveSync(path, opt) } throw er diff --git a/test/integration/eperm.ts b/test/integration/eperm.ts new file mode 100644 index 00000000..419b6909 --- /dev/null +++ b/test/integration/eperm.ts @@ -0,0 +1,193 @@ +import t, { Test } from 'tap' +import { mkdirSync, readdirSync, writeFileSync } from 'fs' +import { sep, join } from 'path' +import { globSync } from 'glob' +import { windows, windowsSync } from '../../src/index.js' +import { randomBytes } from 'crypto' +import assert from 'assert' + +const arrSame = (arr1: string[], arr2: string[]) => { + const s = (a: string[]) => [...a].sort().join(',') + return s(arr1) === s(arr2) +} + +const setup = ( + t: Test, + { + iterations, + depth, + files: fileCount, + fileKb, + }: { + iterations: number + depth: number + files: number + fileKb: number + }, +) => { + const dir = t.testdir() + + const letters = (length: number) => + Array.from({ length }).map((_, i) => (10 + i).toString(36)) + const files = letters(fileCount).map(f => `file_${f}`) + const dirs = join(...letters(depth)) + .split(sep) + .reduce((acc, d) => acc.concat(join(acc.at(-1) ?? '', d)), []) + const entries = dirs + .flatMap(d => [d, ...files.map(f => join(d, f))]) + .map(d => join(dir, d)) + + let iteration = 0 + return function* () { + while (iteration !== iterations) { + // use custom error to throw instead of using tap assertions to cut down + // on output when running many iterations + class RunError extends Error { + constructor(message: string, c?: Record) { + super(message, { + cause: { + testName: t.name, + iteration, + ...c, + }, + }) + } + } + + const actual = readdirSync(dir) + assert( + !actual.length, + new RunError(`dir is not empty`, { + found: actual, + }), + ) + + mkdirSync(join(dir, dirs.at(-1)!), { recursive: true }) + for (const d of dirs) { + for (const f of files) { + writeFileSync(join(dir, d, f), randomBytes(1024 * fileKb)) + } + } + + // randomize results from glob so that when running Promise.all(rimraf) + // on the result it will potentially delete parent directories before + // child directories and their files. This seems to make EPERM errors + // more likely on Windows. + const matches = globSync('**/*', { cwd: dir }) + .sort(() => 0.5 - Math.random()) + .map(m => join(dir, m)) + + assert( + arrSame(matches, entries), + new RunError(`glob result does not match expected`, { + found: matches, + wanted: entries, + }), + ) + + iteration += 1 + + yield [matches, RunError] as const + } + + return { + contents: readdirSync(dir), + iteration, + iterations, + } + } +} + +// Copied from sindresorhus/del since it was reported in +// https://github.com/isaacs/rimraf/pull/314 that this test would throw EPERM +// errors consistently in Windows CI environments. +// https://github.com/sindresorhus/del/blob/chore/update-deps/test.js#L116 +t.test('windows does not throw EPERM', t => { + const options = + process.env.CI ? + { + iterations: 1000, + depth: 15, + files: 7, + fileKb: 100, + } + : { + iterations: 200, + depth: 8, + files: 3, + fileKb: 10, + } + + t.test('sync', t => { + let i + const r = setup(t, options)() + while ((i = r.next())) { + if (i.done) { + i = i.value + break + } + + const [matches, RunError] = i.value + const result = matches + .map(path => { + try { + return { + path, + deleted: windowsSync(path), + } + } catch (error) { + throw new RunError('rimraf error', { error, path }) + } + }) + .filter(({ deleted }) => deleted !== true) + assert( + !result.length, + new RunError(`some entries were not deleted`, { + found: result, + }), + ) + } + + t.strictSame(i.contents, []) + t.equal(i.iteration, i.iterations, `ran all ${i.iteration} iterations`) + t.end() + }) + + t.test('async', async t => { + let i + const r = setup(t, options)() + while ((i = r.next())) { + if (i.done) { + i = i.value + break + } + + const [matches, RunError] = i.value + const result = ( + await Promise.all( + matches.map(async path => { + try { + return { + path, + deleted: await windows(path), + } + } catch (error) { + throw new RunError('rimraf error', { error, path }) + } + }), + ) + ).filter(({ deleted }) => deleted !== true) + assert( + !result.length, + new RunError(`some entries were not deleted`, { + found: result, + }), + ) + } + + t.strictSame(i.contents, []) + t.equal(i.iteration, i.iterations, `ran all ${i.iteration} iterations`) + }) + + t.end() +})