Skip to content

Commit 15922df

Browse files
ralyodioclaude
andcommitted
Keep the process alive while the request-timeout stub waits to be abandoned
The previous commit fixed a real defect in this stub and not the one that was breaking CI. The tests were still cancelled, identically, and the reason it looked like a loaded-runner race was that it reproduces on Node 22 *in isolation on an idle machine* — CI runs Node 22 to match apps/*/Dockerfile, while local development is on Node 24. `AbortSignal.timeout()` schedules an **unref'd** timer. That is deliberate: a pending deadline must not hold a program open. Verified identical on both versions — a script whose only pending work is such a deadline exits in 1ms under 22 and 24 alike. In production nothing notices, because the real in-flight fetch holds a socket open and the loop stays alive until the deadline fires. A stub that returns a promise and does nothing else gives the loop no reason to stay awake at all, so it drains and the runner reports "Promise resolution is still pending but the event loop has already resolved" and cancels the file. Node 24's test runner happens to keep the loop alive and Node 22's does not, which is the whole of the difference between green locally and red in CI. So the stub now holds a ref'd timer for the life of the request it is standing in for, cleared on every exit. Verified on both: 4 passed, 0 cancelled under Node 22 and Node 24, and the full workspace suite is clean under Node 22. The `aborted` check from the previous commit stays. It was a genuine fault — a listener added after the event has fired never hears it — and would have bitten as soon as a deadline beat the stub to its first line. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 7dff2b3 commit 15922df

1 file changed

Lines changed: 31 additions & 11 deletions

File tree

‎packages/db/test/request-timeout.test.js‎

Lines changed: 31 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,21 @@ import { withTimeout } from '../src/client.js';
1313
/**
1414
* A `fetch` that never answers, and rejects when the request is abandoned.
1515
*
16-
* The `aborted` check is the whole point of this helper existing. A real fetch
17-
* handed a signal that has *already* fired rejects immediately, but an `abort`
18-
* listener added afterwards never hears anything — the event has been and gone.
19-
* So a stub that only listens hangs for ever whenever the deadline wins the
20-
* race to the first line of the stub, which is exactly what a loaded CI runner
21-
* arranges: these four tests were cancelled on the first run of the new
22-
* workflow with "Promise resolution is still pending but the event loop has
23-
* already resolved", while passing every time on a quiet laptop.
16+
* Two things a naive stub gets wrong, both of which cost a CI run to find.
17+
*
18+
* **It has to keep the process alive.** `AbortSignal.timeout()` schedules an
19+
* *unref'd* timer — by design, so a pending deadline never holds a program
20+
* open — and a stub that merely returns a promise gives the event loop nothing
21+
* else to do. The loop drains, and the test runner reports "Promise resolution
22+
* is still pending but the event loop has already resolved" and cancels the
23+
* whole file. In production this cannot happen, because a real in-flight fetch
24+
* holds a socket open; only a stub that does literally nothing is exposed to
25+
* it. Node 24's runner happens to keep the loop alive and Node 22's does not,
26+
* which is why this passed locally and failed in CI on the same commit.
27+
*
28+
* **It has to honour a signal that has already fired.** A real fetch handed an
29+
* aborted signal rejects at once; an `abort` listener added afterwards hears
30+
* nothing, because the event has been and gone.
2431
*
2532
* @param {(reason: unknown) => Error|unknown} [reasonFor] what to reject with
2633
* @returns {(input: unknown, init?: { signal?: AbortSignal }) => Promise<never>}
@@ -29,12 +36,25 @@ function neverAnswers(reasonFor = (reason) => reason) {
2936
return (_input, init = {}) =>
3037
new Promise((_resolve, reject) => {
3138
const { signal } = init;
32-
if (!signal) return;
39+
// Deliberately ref'd, and cleared on every exit below so it cannot outlive
40+
// the request it is standing in for.
41+
const inFlight = setTimeout(() => {}, 30_000);
42+
const abandon = (reason) => {
43+
clearTimeout(inFlight);
44+
reject(reasonFor(reason));
45+
};
46+
47+
// `withTimeout` always supplies one; a stub left pending with nothing to
48+
// wake it would wedge the file for thirty seconds rather than fail.
49+
if (!signal) {
50+
clearTimeout(inFlight);
51+
return;
52+
}
3353
if (signal.aborted) {
34-
reject(reasonFor(signal.reason));
54+
abandon(signal.reason);
3555
return;
3656
}
37-
signal.addEventListener('abort', () => reject(reasonFor(signal.reason)));
57+
signal.addEventListener('abort', () => abandon(signal.reason));
3858
});
3959
}
4060

0 commit comments

Comments
 (0)