Skip to content

Commit aefad2d

Browse files
committed
fix(fs): implement retry logic for safeDelete and safeDeleteSync
The maxRetries and retryDelay options in RemoveOptions were documented but not actually implemented. Previously, maxRetries was incorrectly passed as the 'concurrency' option to del, which controls parallelism not retries. Changes: - safeDelete now wraps deleteAsync with pRetry for exponential backoff - safeDeleteSync implements sync retry loop with Atomics.wait for sleep - Both use backoffFactor: 2 (delay doubles each retry) - Added tests for retry options
1 parent 807857f commit aefad2d

2 files changed

Lines changed: 78 additions & 12 deletions

File tree

src/fs.ts

Lines changed: 46 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { getAbortSignal } from './constants/process'
1919

2020
import { isArray } from './arrays'
2121
import { deleteAsync, deleteSync } from './external/del'
22+
import { pRetry } from './promises'
2223
import { defaultIgnore, getGlobMatcher } from './globs'
2324
import type { JsonReviver } from './json/types'
2425
import { jsonParse } from './json/parse'
@@ -1270,13 +1271,25 @@ export async function safeDelete(
12701271
}
12711272
}
12721273

1274+
const maxRetries = opts.maxRetries ?? defaultRemoveOptions.maxRetries
1275+
const retryDelay = opts.retryDelay ?? defaultRemoveOptions.retryDelay
1276+
12731277
/* c8 ignore start - External del call */
1274-
await deleteAsync(patterns, {
1275-
concurrency: opts.maxRetries || defaultRemoveOptions.maxRetries,
1276-
dryRun: false,
1277-
force: shouldForce,
1278-
onlyFiles: false,
1279-
})
1278+
await pRetry(
1279+
async () => {
1280+
await deleteAsync(patterns, {
1281+
dryRun: false,
1282+
force: shouldForce,
1283+
onlyFiles: false,
1284+
})
1285+
},
1286+
{
1287+
retries: maxRetries,
1288+
baseDelayMs: retryDelay,
1289+
backoffFactor: 2,
1290+
signal: opts.signal,
1291+
},
1292+
)
12801293
/* c8 ignore stop */
12811294
}
12821295

@@ -1351,13 +1364,34 @@ export function safeDeleteSync(
13511364
}
13521365
}
13531366

1367+
const maxRetries = opts.maxRetries ?? defaultRemoveOptions.maxRetries
1368+
const retryDelay = opts.retryDelay ?? defaultRemoveOptions.retryDelay
1369+
13541370
/* c8 ignore start - External del call */
1355-
deleteSync(patterns, {
1356-
concurrency: opts.maxRetries || defaultRemoveOptions.maxRetries,
1357-
dryRun: false,
1358-
force: shouldForce,
1359-
onlyFiles: false,
1360-
})
1371+
let lastError: Error | undefined
1372+
let delay = retryDelay
1373+
for (let attempt = 0; attempt <= maxRetries; attempt++) {
1374+
try {
1375+
deleteSync(patterns, {
1376+
dryRun: false,
1377+
force: shouldForce,
1378+
onlyFiles: false,
1379+
})
1380+
return
1381+
} catch (error) {
1382+
lastError = error as Error
1383+
if (attempt < maxRetries) {
1384+
// Sync sleep using Atomics.wait on a SharedArrayBuffer.
1385+
// This is a blocking wait that doesn't spin the CPU.
1386+
const waitMs = delay
1387+
Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, waitMs)
1388+
delay *= 2 // Exponential backoff
1389+
}
1390+
}
1391+
}
1392+
if (lastError) {
1393+
throw lastError
1394+
}
13611395
/* c8 ignore stop */
13621396
}
13631397

test/unit/fs.test.mts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -816,6 +816,22 @@ describe('fs', () => {
816816
expect(exists).toBe(false)
817817
}, 'safeDelete-force-')
818818
})
819+
820+
it('should respect maxRetries and retryDelay options', async () => {
821+
await runWithTempDir(async tmpDir => {
822+
const testFile = path.join(tmpDir, 'file.txt')
823+
await fs.writeFile(testFile, '', 'utf8')
824+
825+
// Delete with explicit retry options (should succeed on first attempt)
826+
await safeDelete(testFile, { maxRetries: 2, retryDelay: 50 })
827+
828+
const exists = await fs
829+
.access(testFile)
830+
.then(() => true)
831+
.catch(() => false)
832+
expect(exists).toBe(false)
833+
}, 'safeDelete-retry-')
834+
})
819835
})
820836

821837
describe('safeDeleteSync', () => {
@@ -875,6 +891,22 @@ describe('fs', () => {
875891
it('should not throw for non-existent files', () => {
876892
expect(() => safeDeleteSync('/nonexistent/file.txt')).not.toThrow()
877893
})
894+
895+
it('should respect maxRetries and retryDelay options', async () => {
896+
await runWithTempDir(async tmpDir => {
897+
const testFile = path.join(tmpDir, 'file.txt')
898+
await fs.writeFile(testFile, '', 'utf8')
899+
900+
// Delete with explicit retry options (should succeed on first attempt)
901+
safeDeleteSync(testFile, { maxRetries: 2, retryDelay: 50 })
902+
903+
const exists = await fs
904+
.access(testFile)
905+
.then(() => true)
906+
.catch(() => false)
907+
expect(exists).toBe(false)
908+
}, 'safeDeleteSync-retry-')
909+
})
878910
})
879911

880912
describe('safeReadFile', () => {

0 commit comments

Comments
 (0)