Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/vitest-pool-workers-waituntil-timeout.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
"@cloudflare/vitest-pool-workers": patch
---

Add a 30-second timeout to `waitUntil` promise draining to prevent hanging tests

Previously, if a `ctx.waitUntil()` promise never resolved, the test suite would hang indefinitely after the test file finished. Now, any `waitUntil` promises that haven't settled within 30 seconds are abandoned with a warning, allowing the test suite to continue. This aligns with the [production `waitUntil` limit](https://developers.cloudflare.com/workers/platform/limits/#duration).
50 changes: 46 additions & 4 deletions packages/vitest-pool-workers/src/worker/wait-until.ts
Original file line number Diff line number Diff line change
@@ -1,21 +1,63 @@
import { AsyncLocalStorage } from "node:async_hooks";

/**
* In production, Workers have a 30-second limit for `waitUntil` promises.
* We use the same limit here. If promises are still pending after this,
* they almost certainly indicate a bug (e.g. a `waitUntil` promise that
* will never resolve). We log a warning and move on so the test suite
* doesn't hang indefinitely.
*/
let WAIT_UNTIL_TIMEOUT = 30_000;

/** @internal — only exposed for tests */
export function setWaitUntilTimeout(ms: number): void {
WAIT_UNTIL_TIMEOUT = ms;
}

const kTimedOut = Symbol("kTimedOut");

/**
* Empty array and wait for all promises to resolve until no more added.
* If a single promise rejects, the rejection will be passed-through.
* If multiple promises reject, the rejections will be aggregated.
*
* If any batch of promises hasn't settled after {@link WAIT_UNTIL_TIMEOUT}ms,
* a warning is logged and the remaining promises are abandoned.
*/
export async function waitForWaitUntil(
/* mut */ waitUntil: unknown[]
): Promise<void> {
const errors: unknown[] = [];

while (waitUntil.length > 0) {
const results = await Promise.allSettled(waitUntil.splice(0));
const batch = waitUntil.splice(0);
let timeoutId: ReturnType<typeof setTimeout> | undefined;
const result = await Promise.race([
Promise.allSettled(batch).then((results) => ({ results })),
new Promise<typeof kTimedOut>(
(resolve) =>
(timeoutId = setTimeout(() => resolve(kTimedOut), WAIT_UNTIL_TIMEOUT))
),
]);
Comment thread
penalosa marked this conversation as resolved.
clearTimeout(timeoutId);

if (result === kTimedOut) {
__console.warn(
`[vitest-pool-workers] ${batch.length} waitUntil promise(s) did not ` +
`resolve within ${WAIT_UNTIL_TIMEOUT / 1000}s and will be abandoned. ` +
`This normally means your Worker's waitUntil handler has a bug ` +
`that prevents it from settling (e.g. a fetch that never completes ` +
`or a missing resolve/reject call).`
);
// Stop draining — any promises added during this batch are also abandoned
waitUntil.length = 0;
break;
}

// Record all rejected promises
for (const result of results) {
if (result.status === "rejected") {
errors.push(result.reason);
for (const settled of result.results) {
if (settled.status === "rejected") {
errors.push(settled.reason);
}
}
}
Expand Down
31 changes: 31 additions & 0 deletions packages/vitest-pool-workers/test/wait-until-timeout.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import dedent from "ts-dedent";
import { test, vitestConfig } from "./helpers";

test("waitForWaitUntil abandons promises that never resolve", async ({
expect,
seed,
vitestRun,
}) => {
await seed({
"vitest.config.mts": vitestConfig(),
"index.test.ts": dedent`
import {
setWaitUntilTimeout,
waitForWaitUntil,
} from "cloudflare:test-internal";
import { expect, it } from "vitest";

it("returns after timeout instead of hanging", async () => {
setWaitUntilTimeout(100);
const waitUntil = [new Promise(() => {})];
await waitForWaitUntil(waitUntil);
// If we get here, the timeout worked — the function didn't hang
expect(waitUntil).toHaveLength(0);
});
`,
});
const result = await vitestRun();
expect(await result.exitCode).toBe(0);
const output = result.stdout + result.stderr;
expect(output).toContain("waitUntil promise(s) did not resolve within");
});
Loading