From 0063308abf8066d23b4256bec1576ad8aefb22e9 Mon Sep 17 00:00:00 2001 From: Rolando Bosch Date: Sun, 6 Sep 2026 22:38:31 -0400 Subject: [PATCH] fix: restore retry backoff when removing files --- src/lib/files.ts | 4 +- test/local/lib/files.test.ts | 120 +++++++++++++++++++++++++++++++++++ 2 files changed, 123 insertions(+), 1 deletion(-) create mode 100644 test/local/lib/files.test.ts diff --git a/src/lib/files.ts b/src/lib/files.ts index bdf6b735a..82419d0da 100644 --- a/src/lib/files.ts +++ b/src/lib/files.ts @@ -50,7 +50,9 @@ export const ensureFolderExistsSync = (rootPath: string, folderPath?: string) => export const rimrafPromised = async (pathToBeRemoved: string | string[]) => { const paths = Array.isArray(pathToBeRemoved) ? pathToBeRemoved : [pathToBeRemoved]; - await Promise.all(paths.map(async (path) => rm(path, { recursive: true, force: true }))); + await Promise.all( + paths.map(async (path) => rm(path, { recursive: true, force: true, maxRetries: 10, retryDelay: 100 })), + ); }; export const deleteFile = async (filePath: string) => { diff --git a/test/local/lib/files.test.ts b/test/local/lib/files.test.ts new file mode 100644 index 000000000..1c4c37da1 --- /dev/null +++ b/test/local/lib/files.test.ts @@ -0,0 +1,120 @@ +import { existsSync } from 'node:fs'; +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { rimrafPromised } from '../../../src/lib/files.js'; + +describe('rimrafPromised()', () => { + let testRoot: string; + + beforeEach(async () => { + testRoot = await mkdtemp(join(tmpdir(), 'apify-cli-rimraf-')); + }); + + afterEach(async () => { + await rm(testRoot, { recursive: true, force: true }); + }); + + it('removes a nested directory tree given a single string path', async () => { + const dir = join(testRoot, 'single'); + await mkdir(join(dir, 'nested'), { recursive: true }); + await writeFile(join(dir, 'nested', 'file.txt'), 'content'); + + await rimrafPromised(dir); + + expect(existsSync(dir)).toBe(false); + }); + + it('removes every path when given an array', async () => { + const dirA = join(testRoot, 'a'); + const dirB = join(testRoot, 'b'); + await mkdir(dirA); + await mkdir(dirB); + await writeFile(join(dirA, 'file.txt'), 'a'); + await writeFile(join(dirB, 'file.txt'), 'b'); + + await rimrafPromised([dirA, dirB]); + + expect(existsSync(dirA)).toBe(false); + expect(existsSync(dirB)).toBe(false); + }); + + it('resolves without throwing when the path does not exist', async () => { + const missing = join(testRoot, 'does-not-exist'); + + await expect(rimrafPromised(missing)).resolves.toBeUndefined(); + }); + + it('resolves without throwing when an array mixes existing and missing paths', async () => { + const existing = join(testRoot, 'existing'); + await mkdir(existing); + const missing = join(testRoot, 'missing'); + + await expect(rimrafPromised([existing, missing])).resolves.toBeUndefined(); + expect(existsSync(existing)).toBe(false); + }); +}); + +// Check the retry options delegated to fs.rm; these mocks do not exercise native retry timing. +describe('rimrafPromised() retry configuration', () => { + const mockRm = (impl: (path: string, options: unknown) => Promise) => { + vi.doMock('node:fs/promises', async (importOriginal) => { + const original = await importOriginal(); + return { ...original, rm: vi.fn(impl) }; + }); + }; + + it('passes maxRetries and retryDelay to fs.rm alongside recursive/force', async () => { + vi.resetModules(); + mockRm(async () => undefined); + + const { rm: mockedRm } = await import('node:fs/promises'); + const { rimrafPromised: rimrafPromisedFresh } = await import('../../../src/lib/files.js'); + + await rimrafPromisedFresh('/tmp/apify-cli-fake-path'); + + expect(mockedRm).toHaveBeenCalledWith('/tmp/apify-cli-fake-path', { + recursive: true, + force: true, + maxRetries: 10, + retryDelay: 100, + }); + }); + + it('passes the same retry options to every path in an array', async () => { + vi.resetModules(); + mockRm(async () => undefined); + + const { rm: mockedRm } = await import('node:fs/promises'); + const { rimrafPromised: rimrafPromisedFresh } = await import('../../../src/lib/files.js'); + + await rimrafPromisedFresh(['/tmp/apify-cli-fake-a', '/tmp/apify-cli-fake-b']); + + expect(mockedRm).toHaveBeenCalledTimes(2); + expect(mockedRm).toHaveBeenNthCalledWith(1, '/tmp/apify-cli-fake-a', { + recursive: true, + force: true, + maxRetries: 10, + retryDelay: 100, + }); + expect(mockedRm).toHaveBeenNthCalledWith(2, '/tmp/apify-cli-fake-b', { + recursive: true, + force: true, + maxRetries: 10, + retryDelay: 100, + }); + }); + + it('propagates the fs.rm rejection unchanged', async () => { + vi.resetModules(); + const originalError = Object.assign(new Error('EBUSY: resource busy or locked'), { code: 'EBUSY' }); + mockRm(async () => { + throw originalError; + }); + + const { rimrafPromised: rimrafPromisedFresh } = await import('../../../src/lib/files.js'); + + await expect(rimrafPromisedFresh('/tmp/apify-cli-fake-path')).rejects.toBe(originalError); + }); +});