eperm stuff

lukekarrys · Oct 10, 2024 · cfee0e1 · cfee0e1
1 parent d1a59d7
commit cfee0e1
Show file tree

Hide file tree

Showing 11 changed files with 349 additions and 70 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -1,21 +1,21 @@
 name: CI
 
-on: [push, pull_request]
+on: [pull_request]
 
 jobs:
   build:
     strategy:
       matrix:
-        node-version: [20.x, 22.x]
+        node-version: [22.x]
         platform:
-        - os: ubuntu-latest
-          shell: bash
-        - os: macos-latest
-          shell: bash
+        # - os: ubuntu-latest
+        #   shell: bash
+        # - os: macos-latest
+        #   shell: bash
         - os: windows-latest
           shell: bash
-        - os: windows-latest
-          shell: powershell
+        # - os: windows-latest
+        #   shell: powershell
       fail-fast: false
 
     runs-on: ${{ matrix.platform.os }}
@@ -36,8 +36,21 @@ jobs:
         run: npm install
 
       - name: Run Tests
-        run: npm test -- -t0 -c
-        env:
-          RIMRAF_TEST_START_CHAR: a
-          RIMRAF_TEST_END_CHAR: f
-          RIMRAF_TEST_DEPTH: 5
+        run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
+      - run: npm test -- test/integration/eperm.ts --disable-coverage --grep=. --grep=async
diff --git a/package.json b/package.json
@@ -93,6 +93,7 @@
   ],
   "module": "./dist/esm/index.js",
   "tap": {
-    "coverage-map": "map.js"
+    "coverage-map": "map.js",
+    "timeout": 600
   }
 }
diff --git a/src/error.ts b/src/error.ts
@@ -1,17 +1,28 @@
+export const Codes = {
+  EPERM: 'EPERM',
+  ENOENT: 'ENOENT',
+  EMFILE: 'EMFILE',
+  ENFILE: 'ENFILE',
+  EBUSY: 'EBUSY',
+  ENOTDIR: 'ENOTDIR',
+  ENOTEMPTY: 'ENOTEMPTY',
+} as const
+
+export type Code = (typeof Codes)[keyof typeof Codes]
+export type CodeSet = Set<Code>
+const codes: CodeSet = new Set(Object.values(Codes))
+
 const isRecord = (o: unknown): o is Record<string, unknown> =>
   !!o && typeof o === 'object'
 
-const hasString = (o: Record<string, unknown>, key: string) =>
-  key in o && typeof o[key] === 'string'
+const isString = (v: unknown): v is string => typeof v === 'string'
+
+const isCode = (v: unknown): v is Code => isString(v) && codes.has(v as Code)
 
-export const isFsError = (
-  o: unknown,
-): o is NodeJS.ErrnoException & {
-  code: string
-  path: string
-} => isRecord(o) && hasString(o, 'code') && hasString(o, 'path')
+export const isError = (o: unknown): o is { code: Code; path: string } =>
+  isRecord(o) && isCode(o.code) && isString(o.path)
 
-export const errorCode = (er: unknown) =>
-  isRecord(er) && hasString(er, 'code') ? er.code : null
+export const errorCode = (er: unknown): Code | null =>
+  isRecord(er) && isCode(er.code) ? er.code : null
 
 export const asFsError = (er: unknown) => er as NodeJS.ErrnoException
diff --git a/src/fix-eperm.ts b/src/fix-eperm.ts
@@ -1,4 +1,4 @@
-import { errorCode } from './error.js'
+import { Codes, errorCode } from './error.js'
 import { chmodSync, promises } from './fs.js'
 import { ignoreENOENT, ignoreENOENTSync } from './ignore-enoent.js'
 const { chmod } = promises
@@ -9,7 +9,7 @@ export const fixEPERM =
     try {
       return void (await ignoreENOENT(fn(path)))
     } catch (er) {
-      if (errorCode(er) === 'EPERM') {
+      if (errorCode(er) === Codes.EPERM) {
         if (
           !(await ignoreENOENT(
             chmod(path, 0o666).then(() => true),
@@ -30,7 +30,7 @@ export const fixEPERMSync =
     try {
       return void ignoreENOENTSync(() => fn(path))
     } catch (er) {
-      if (errorCode(er) === 'EPERM') {
+      if (errorCode(er) === Codes.EPERM) {
         if (!ignoreENOENTSync(() => (chmodSync(path, 0o666), true), er)) {
           return
         }

diff --git 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<Dirent[]> =>
+//   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/ignore-enoent.ts b/src/ignore-enoent.ts
@@ -1,8 +1,8 @@
-import { errorCode } from './error.js'
+import { Codes, errorCode } from './error.js'
 
 export const ignoreENOENT = async <T>(p: Promise<T>, rethrow?: unknown) =>
   p.catch(er => {
-    if (errorCode(er) === 'ENOENT') {
+    if (errorCode(er) === Codes.ENOENT) {
       return
     }
     throw rethrow ?? er
@@ -12,7 +12,7 @@ export const ignoreENOENTSync = <T>(fn: () => T, rethrow?: unknown) => {
   try {
     return fn()
   } catch (er) {
-    if (errorCode(er) === 'ENOENT') {
+    if (errorCode(er) === Codes.ENOENT) {
       return
     }
     throw rethrow ?? er

diff --git a/src/retry-busy.ts b/src/retry-busy.ts
@@ -2,36 +2,58 @@
 
 import { setTimeout } from 'timers/promises'
 import { RimrafAsyncOptions, RimrafOptions } from './index.js'
-import { isFsError } from './error.js'
+import { CodeSet, Codes, isError } from './error.js'
 
 export const MAXBACKOFF = 200
 export const RATE = 1.2
 export const MAXRETRIES = 10
-export const codes = new Set(['EMFILE', 'ENFILE', 'EBUSY'])
+export const codes: CodeSet = new Set([Codes.EMFILE, Codes.ENFILE, Codes.EBUSY])
 
-export const retryBusy = <T>(fn: (path: string) => Promise<T>) => {
+export const retryBusy = <T>(
+  fn: (path: string) => Promise<T>,
+  extraCodes?: CodeSet,
+) => {
   const method = async (
     path: string,
-    opt: RimrafAsyncOptions,
-    backoff = 1,
+    {
+      maxBackoff = MAXBACKOFF,
+      backoff = RATE,
+      maxRetries = MAXRETRIES,
+    }: RimrafAsyncOptions = {},
+    nextBackoff = 1,
     total = 0,
   ) => {
-    const mbo = opt.maxBackoff || MAXBACKOFF
-    const rate = opt.backoff || RATE
-    const max = opt.maxRetries || MAXRETRIES
     let retries = 0
     while (true) {
       try {
         return await fn(path)
       } catch (er) {
-        if (isFsError(er) && er.path === path && codes.has(er.code)) {
-          backoff = Math.ceil(backoff * rate)
-          total = backoff + total
-          if (total < mbo) {
-            await setTimeout(backoff)
-            return method(path, opt, backoff, total)
+        if (
+          isError(er) &&
+          er.path === path &&
+          (codes.has(er.code) || extraCodes?.has(er.code))
+        ) {
+          nextBackoff = Math.ceil(nextBackoff * backoff)
+          total = nextBackoff + total
+          if (er.code === Codes.EPERM) {
+            console.trace({
+              total: total,
+              mbo: maxBackoff,
+              retries,
+              backoff: nextBackoff,
+              er,
+            })
+          }
+          if (total < maxBackoff) {
+            await setTimeout(nextBackoff)
+            return method(
+              path,
+              { maxBackoff, backoff, maxRetries },
+              nextBackoff,
+              total,
+            )
           }
-          if (retries < max) {
+          if (retries < maxRetries) {
             retries++
             continue
           }
@@ -40,24 +62,23 @@ export const retryBusy = <T>(fn: (path: string) => Promise<T>) => {
       }
     }
   }
-
   return method
 }
 
 // just retries, no async so no backoff
-export const retryBusySync = <T>(fn: (path: string) => T) => {
-  const method = (path: string, opt: RimrafOptions) => {
-    const max = opt.maxRetries || MAXRETRIES
+export const retryBusySync =
+  <T>(fn: (path: string) => T, extraCodes?: CodeSet) =>
+  (path: string, { maxRetries = MAXRETRIES }: RimrafOptions = {}) => {
     let retries = 0
     while (true) {
       try {
         return fn(path)
       } catch (er) {
         if (
-          isFsError(er) &&
+          isError(er) &&
           er.path === path &&
-          codes.has(er.code) &&
-          retries < max
+          (codes.has(er.code) || extraCodes?.has(er.code)) &&
+          retries < maxRetries
         ) {
           retries++
           continue
@@ -66,5 +87,3 @@ export const retryBusySync = <T>(fn: (path: string) => T) => {
       }
     }
   }
-  return method
-}
diff --git a/src/rimraf-move-remove.ts b/src/rimraf-move-remove.ts
@@ -19,13 +19,18 @@ import { Dirent, Stats } from 'fs'
 import { RimrafAsyncOptions, RimrafSyncOptions } from './index.js'
 import { readdirOrError, readdirOrErrorSync } from './readdir-or-error.js'
 import { fixEPERM, fixEPERMSync } from './fix-eperm.js'
+import { Codes, 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([Codes.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,
@@ -62,18 +67,18 @@ const rimrafMoveRemoveDir = async (
     // swapped out with a file at just the right moment.
     /* c8 ignore start */
     if (entries) {
-      if (entries.code === 'ENOENT') {
+      if (errorCode(entries) === Codes.ENOENT) {
         return true
       }
-      if (entries.code !== 'ENOTDIR') {
+      if (errorCode(entries) !== Codes.ENOTDIR) {
         throw entries
       }
     }
     /* c8 ignore stop */
     if (opt.filter && !(await opt.filter(path, ent))) {
       return false
     }
-    await ignoreENOENT(tmpUnlink(path, opt.tmp, unlinkFixEPERM))
+    await ignoreENOENT(tmpUnlink(path, opt.tmp, p => unlinkFixEPERM(p, opt)))
     return true
   }
 
@@ -97,7 +102,7 @@ const rimrafMoveRemoveDir = async (
   if (opt.filter && !(await opt.filter(path, ent))) {
     return false
   }
-  await ignoreENOENT(tmpUnlink(path, opt.tmp, rmdir))
+  await ignoreENOENT(tmpUnlink(path, opt.tmp, p => rmdirFixEPERM(p, opt)))
   return true
 }
 
@@ -145,18 +150,20 @@ const rimrafMoveRemoveDirSync = (
     // swapped out with a file at just the right moment.
     /* c8 ignore start */
     if (entries) {
-      if (entries.code === 'ENOENT') {
+      if (errorCode(entries) === Codes.ENOENT) {
         return true
       }
-      if (entries.code !== 'ENOTDIR') {
+      if (errorCode(entries) !== Codes.ENOTDIR) {
         throw entries
       }
     }
     /* c8 ignore stop */
     if (opt.filter && !opt.filter(path, ent)) {
       return false
     }
-    ignoreENOENTSync(() => tmpUnlinkSync(path, tmp, unlinkFixEPERMSync))
+    ignoreENOENTSync(() =>
+      tmpUnlinkSync(path, tmp, p => unlinkFixEPERMSync(p, opt)),
+    )
     return true
   }
 
@@ -174,7 +181,9 @@ const rimrafMoveRemoveDirSync = (
   if (opt.filter && !opt.filter(path, ent)) {
     return false
   }
-  ignoreENOENTSync(() => tmpUnlinkSync(path, tmp, rmdirSync))
+  ignoreENOENTSync(() =>
+    tmpUnlinkSync(path, tmp, p => rmdirFixEPERMSync(p, opt)),
+  )
   return true
 }