Skip to content

Commit 603fe18

Browse files
petebacondarwinpetebacondarwin
authored
fix: add maxRetries to recursive directory removal for Windows EBUSY (#12629)
1 parent 19df099 commit 603fe18

File tree

44 files changed

+627
-209
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+627
-209
lines changed

‎.changeset/keen-fish-melt.md‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
---
2+
"@cloudflare/workers-utils": patch
3+
---
4+
5+
Add `removeDir` and `removeDirSync` helpers with automatic retry logic for Windows EBUSY errors
6+
7+
These new helpers wrap `fs.rm`/`fs.rmSync` with `maxRetries: 5` and `retryDelay: 100` to handle cases where file handles aren't immediately released (common on Windows with workerd).
8+
The async helper also has a `fireAndForget` option to silently swallow errors and not await removal.
9+
10+
This improves reliability of cleanup operations across the codebase.

‎fixtures/additional-modules/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"@cloudflare/eslint-config-shared": "workspace:*",
1515
"@cloudflare/workers-tsconfig": "workspace:*",
1616
"@cloudflare/workers-types": "catalog:default",
17+
"@fixture/shared": "workspace:*",
1718
"typescript": "catalog:default",
1819
"undici": "catalog:default",
1920
"vitest": "catalog:default",

‎fixtures/additional-modules/test/index.test.ts‎

Lines changed: 3 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { existsSync } from "node:fs";
33
import fs from "node:fs/promises";
44
import os from "node:os";
55
import path from "node:path";
6+
import { removeDir } from "@fixture/shared/src/fs-helpers";
67
import { afterAll, assert, beforeAll, describe, test, vi } from "vitest";
78
import { unstable_startWorker } from "wrangler";
89
import { wranglerEntryPath } from "../../shared/src/run-wrangler-long-lived";
@@ -41,21 +42,7 @@ describe("find_additional_modules dev", () => {
4142
});
4243
afterAll(async () => {
4344
await worker.dispose();
44-
try {
45-
await fs.rm(tmpDir, { recursive: true, force: true });
46-
} catch (e) {
47-
// It seems that Windows doesn't let us delete this, with errors like:
48-
//
49-
// Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ'
50-
// ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
51-
// Serialized Error: {
52-
// "code": "EBUSY",
53-
// "errno": -4082,
54-
// "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ",
55-
// "syscall": "rmdir",
56-
// }
57-
console.error(e);
58-
}
45+
removeDir(tmpDir, { fireAndForget: true });
5946
});
6047

6148
test("supports bundled modules", async ({ expect }) => {
@@ -141,9 +128,7 @@ describe("find_additional_modules deploy", () => {
141128
beforeAll(async () => {
142129
tmpDir = await getTmpDir();
143130
});
144-
afterAll(async () => {
145-
await fs.rm(tmpDir, { recursive: true, force: true });
146-
});
131+
afterAll(async () => await removeDir(tmpDir, { fireAndForget: true }));
147132

148133
test("doesn't bundle additional modules", async ({ expect }) => {
149134
const outDir = path.join(tmpDir, "out");

‎fixtures/interactive-dev-tests/tests/index.test.ts‎

Lines changed: 9 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import rl from "node:readline";
66
import stream from "node:stream";
77
import { setTimeout } from "node:timers/promises";
88
import { stripVTControlCharacters } from "node:util";
9+
import { removeDir } from "@fixture/shared/src/fs-helpers";
910
import { fetch } from "undici";
1011
/* eslint-disable workers-sdk/no-vitest-import-expect -- complex test with .each patterns */
1112
import {
@@ -380,22 +381,8 @@ if (process.platform === "win32") {
380381
});
381382
}
382383
});
383-
afterAll(async () => {
384-
try {
385-
fs.rmSync(tmpDir, { recursive: true, force: true });
386-
} catch (e) {
387-
// It seems that Windows doesn't let us delete this, with errors like:
388-
//
389-
// Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ'
390-
// ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
391-
// Serialized Error: {
392-
// "code": "EBUSY",
393-
// "errno": -4082,
394-
// "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ",
395-
// "syscall": "rmdir",
396-
// }
397-
console.error(e);
398-
}
384+
afterAll(() => {
385+
removeDir(tmpDir, { fireAndForget: true });
399386
});
400387

401388
it("should print rebuild containers hotkey", async () => {
@@ -560,22 +547,8 @@ if (process.platform === "win32") {
560547
}
561548
});
562549

563-
afterAll(async () => {
564-
try {
565-
fs.rmSync(tmpDir, { recursive: true, force: true });
566-
} catch (e) {
567-
// It seems that Windows doesn't let us delete this, with errors like:
568-
//
569-
// Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ'
570-
// ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
571-
// Serialized Error: {
572-
// "code": "EBUSY",
573-
// "errno": -4082,
574-
// "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ",
575-
// "syscall": "rmdir",
576-
// }
577-
console.error(e);
578-
}
550+
afterAll(() => {
551+
removeDir(tmpDir, { fireAndForget: true });
579552
});
580553

581554
it("should allow quitting while the image is building", async () => {
@@ -675,22 +648,8 @@ if (process.platform === "win32") {
675648
});
676649
}
677650
});
678-
afterAll(async () => {
679-
try {
680-
fs.rmSync(tmpDir, { recursive: true, force: true });
681-
} catch (e) {
682-
// It seems that Windows doesn't let us delete this, with errors like:
683-
//
684-
// Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ'
685-
// ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
686-
// Serialized Error: {
687-
// "code": "EBUSY",
688-
// "errno": -4082,
689-
// "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ",
690-
// "syscall": "rmdir",
691-
// }
692-
console.error(e);
693-
}
651+
afterAll(() => {
652+
removeDir(tmpDir, { fireAndForget: true });
694653
});
695654

696655
it("should print build logs for all the containers", async () => {
@@ -837,22 +796,8 @@ if (process.platform === "win32") {
837796
});
838797
}
839798
});
840-
afterAll(async () => {
841-
try {
842-
fs.rmSync(tmpDir, { recursive: true, force: true });
843-
} catch (e) {
844-
// It seems that Windows doesn't let us delete this, with errors like:
845-
//
846-
// Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ'
847-
// ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
848-
// Serialized Error: {
849-
// "code": "EBUSY",
850-
// "errno": -4082,
851-
// "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ",
852-
// "syscall": "rmdir",
853-
// }
854-
console.error(e);
855-
}
799+
afterAll(() => {
800+
removeDir(tmpDir, { fireAndForget: true });
856801
});
857802

858803
it("should be able to interact with both workers, rebuild the containers with the hotkey and all containers should be cleaned up at the end", async () => {

‎fixtures/shared/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"private": true,
44
"description": "Shared fixtures for testing",
55
"devDependencies": {
6+
"@cloudflare/workers-utils": "workspace:*",
67
"wrangler": "workspace:*"
78
},
89
"volta": {

‎fixtures/shared/src/fs-helpers.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Re-exported so that fixtures can import from `@fixture/shared/src/fs-helpers`
2+
// without each fixture needing a direct dependency on `@cloudflare/workers-utils`.
3+
export { removeDir, removeDirSync } from "@cloudflare/workers-utils";

‎fixtures/wildcard-modules/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
"@cloudflare/eslint-config-shared": "workspace:*",
1515
"@cloudflare/workers-tsconfig": "workspace:*",
1616
"@cloudflare/workers-types": "catalog:default",
17+
"@fixture/shared": "workspace:*",
1718
"undici": "catalog:default",
1819
"wrangler": "workspace:*"
1920
},

‎fixtures/wildcard-modules/test/index.test.ts‎

Lines changed: 4 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import fs from "node:fs/promises";
44
import os from "node:os";
55
import path from "node:path";
66
import { setTimeout } from "node:timers/promises";
7+
import { removeDir } from "@fixture/shared/src/fs-helpers";
78
import { fetch } from "undici";
89
import { afterAll, assert, beforeAll, describe, test } from "vitest";
910
import {
@@ -55,21 +56,7 @@ describe("wildcard imports: dev", () => {
5556
});
5657
afterAll(async () => {
5758
await worker.stop();
58-
try {
59-
await fs.rm(tmpDir, { recursive: true, force: true });
60-
} catch (e) {
61-
// It seems that Windows doesn't let us delete this, with errors like:
62-
//
63-
// Error: EBUSY: resource busy or locked, rmdir 'C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ'
64-
// ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
65-
// Serialized Error: {
66-
// "code": "EBUSY",
67-
// "errno": -4082,
68-
// "path": "C:\Users\RUNNER~1\AppData\Local\Temp\wrangler-modules-pKJ7OQ",
69-
// "syscall": "rmdir",
70-
// }
71-
console.error(e);
72-
}
59+
removeDir(tmpDir, { fireAndForget: true });
7360
});
7461

7562
test("supports bundled modules", async ({ expect }) => {
@@ -152,8 +139,8 @@ describe("wildcard imports: deploy", () => {
152139
beforeAll(async () => {
153140
tmpDir = await getTmpDir();
154141
});
155-
afterAll(async () => {
156-
await fs.rm(tmpDir, { recursive: true, force: true });
142+
afterAll(() => {
143+
removeDir(tmpDir, { fireAndForget: true });
157144
});
158145

159146
test("bundles wildcard modules", async ({ expect }) => {

‎packages/create-cloudflare/e2e/helpers/index.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,10 +48,11 @@ const testProjectDir = (suite: string, test: string) => {
4848

4949
realpathSync(mkdtempSync(nodePath.join(tmpdir(), `c3-tests-${suite}`)));
5050
const filepath = getPath();
51+
// eslint-disable-next-line workers-sdk/no-direct-recursive-rm -- see log-stream.ts for rationale
5152
rmSync(filepath, {
5253
recursive: true,
5354
force: true,
54-
maxRetries: 10,
55+
maxRetries: 5,
5556
retryDelay: 100,
5657
});
5758
} catch (e) {

‎packages/create-cloudflare/e2e/helpers/log-stream.ts‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,19 @@ export function createTestLogStream(task: RunnerTask) {
2020

2121
export function recreateLogFolder(suite: RunnerTestSuite) {
2222
// Clean the old folder if exists (useful for dev)
23+
//
24+
// Note: this is intentionally inlined rather than importing `removeDirSync`
25+
// from `@cloudflare/workers-utils`. That package's barrel export pulls in CJS
26+
// dependencies (e.g. `xdg-app-paths`) that break when Vite bundles the vitest
27+
// config (which imports this file) into ESM — the shimmed `require()` calls
28+
// throw "Dynamic require of 'path' is not supported".
29+
// Keep aligned with `removeDirSync()` in `packages/workers-utils/src/fs-helpers.ts`.
30+
// eslint-disable-next-line workers-sdk/no-direct-recursive-rm
2331
rmSync(getLogPath(suite), {
2432
recursive: true,
2533
force: true,
34+
maxRetries: 5,
35+
retryDelay: 100,
2636
});
2737

2838
mkdirSync(getLogPath(suite), { recursive: true });

0 commit comments

Comments
 (0)