From dc026b868dd5ea380e5cd949318fe45638fbb4b7 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 23 Jul 2026 22:03:57 -0400 Subject: [PATCH 1/7] Abort and retry a base-realm fetch that stalls before response headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The in-app fetch-retry net for base-realm artifacts that occasionally vanish in CI recovers only from a thrown `TypeError: Failed to fetch`. A stall where the response headers never arrive is the same phenomenon in a different shape: the fetch neither resolves nor rejects, so it is never caught, has no bound, and hangs the fetcher test-waiter until QUnit's global timeout — which also cascades into the following tests as their mock realm is torn down mid-flight. Bound how long a retryable base-realm fetch may wait for response headers in the browser test suite and abort past that bound, turning the stall into the retryable failure the existing net already recovers from (a fresh connection gets its headers). Each attempt uses its own abort signal so one stalled attempt doesn't poison the next, and the timer is cleared the moment the fetch settles, so a resolved response's body stream is never aborted. Scoped to the browser test environment: a slow response in node / worker / env-mode (e.g. a heavy `_search`) is never aborted and retried. The per-attempt retry log now carries the error so a future failure shows which shape it was. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/unit/virtual-network-retry-test.ts | 129 +++++++++++++++++- packages/runtime-common/virtual-network.ts | 109 +++++++++++++-- 2 files changed, 229 insertions(+), 9 deletions(-) diff --git a/packages/host/tests/unit/virtual-network-retry-test.ts b/packages/host/tests/unit/virtual-network-retry-test.ts index 43773994414..a215e294801 100644 --- a/packages/host/tests/unit/virtual-network-retry-test.ts +++ b/packages/host/tests/unit/virtual-network-retry-test.ts @@ -1,6 +1,10 @@ import { module, test } from 'qunit'; -import { shouldRetryFetch } from '@cardstack/runtime-common/virtual-network'; +import { VirtualNetwork } from '@cardstack/runtime-common'; +import { + shouldRetryFetch, + shouldTimeoutRetryableFetch, +} from '@cardstack/runtime-common/virtual-network'; // `shouldRetryFetch` is the pure predicate that gates the in-app fetch-retry // safety net (see virtual-network.ts). The net papers over a transient CI @@ -101,3 +105,126 @@ module('Unit | virtual-network shouldRetryFetch', function () { }); }); }); + +// `shouldTimeoutRetryableFetch` gates the header-arrival timeout that turns the +// second shape of the CI "vanish" — headers that never arrive, so the fetch +// hangs instead of throwing — into the same retryable failure the throw path +// already recovers from. It bounds only fetches that are already retryable, and +// only in the browser test suite: a slow response in node / worker / env-mode +// (e.g. a heavy `_search`) must never be aborted and retried. +module('Unit | virtual-network shouldTimeoutRetryableFetch', function () { + test('bounds a retryable base-realm fetch in the test environment', function (assert) { + withEnvironment('test', () => { + assert.true( + shouldTimeoutRetryableFetch( + new URL('https://realm-server.ci.localhost/base/_info'), + ), + 'the base _info fetch that hung in CI is bounded', + ); + assert.true( + shouldTimeoutRetryableFetch( + new URL('https://cardstack.com/base/card-api'), + ), + 'a virtual base-realm artifact is bounded', + ); + }); + }); + + test('does not bound a non-retryable fetch even in the test environment', function (assert) { + withEnvironment('test', () => { + assert.false( + shouldTimeoutRetryableFetch( + new URL( + 'https://realm-server.ci.localhost/testuser/personal/Author/1', + ), + ), + 'a user realm on the same host is not bounded', + ); + assert.false( + shouldTimeoutRetryableFetch( + new URL('http://test-realm/test/SystemCard/default'), + ), + 'the in-browser test realm host is not bounded', + ); + }); + }); + + test('does not bound anything outside the test environment', function (assert) { + withEnvironment(undefined, () => { + assert.false( + shouldTimeoutRetryableFetch( + new URL('https://cardstack.com/base/card-api'), + ), + 'production / env-mode fetch flow keeps no header-timeout', + ); + }); + }); +}); + +// A native fetch whose first attempt stalls at the header stage (never +// resolves; rejects only when its request signal aborts, exactly as a real +// fetch does), then succeeds. Mirrors the CI failure where base/_info headers +// never arrived and the unbounded fetch hung until QUnit's global timeout. +function stallingThenOkFetch(): { + fetch: typeof globalThis.fetch; + attempts: () => number; +} { + let attempt = 0; + let fetch = ((input: RequestInfo | URL, init?: RequestInit) => { + attempt++; + let signal = + input instanceof Request ? input.signal : (init?.signal ?? undefined); + if (attempt === 1) { + return new Promise((_resolve, reject) => { + if (!signal) { + return; // no signal wired => hang (fix broken); QUnit will time out + } + if (signal.aborted) { + reject(signal.reason); + return; + } + signal.addEventListener('abort', () => reject(signal.reason), { + once: true, + }); + }); + } + return Promise.resolve( + new Response('{"ok":true}', { + status: 200, + headers: { 'content-type': 'application/json' }, + }), + ); + }) as typeof globalThis.fetch; + return { fetch, attempts: () => attempt }; +} + +module('Unit | virtual-network header-stall recovery', function () { + test('aborts a stalled retryable base fetch and recovers on the retry', async function (assert) { + let g = globalThis as { __environment?: string }; + let had = '__environment' in g; + let prev = g.__environment; + g.__environment = 'test'; + try { + let { fetch, attempts } = stallingThenOkFetch(); + // Tiny header timeout so the stalled first attempt is aborted promptly; + // the second attempt then succeeds. + let vn = new VirtualNetwork(fetch, { fetchHeaderTimeoutMs: 20 }); + let response = await vn.fetch('https://cardstack.com/base/card-api'); + assert.strictEqual( + response.status, + 200, + 'recovered with a 200 after the stalled attempt was aborted', + ); + assert.true( + attempts() >= 2, + 'the stalled first attempt was retried on a fresh fetch', + ); + } finally { + if (had) { + g.__environment = prev; + } else { + delete g.__environment; + } + } + }); +}); diff --git a/packages/runtime-common/virtual-network.ts b/packages/runtime-common/virtual-network.ts index ef4d8130f99..4ec38b681b5 100644 --- a/packages/runtime-common/virtual-network.ts +++ b/packages/runtime-common/virtual-network.ts @@ -42,8 +42,13 @@ export class VirtualNetwork { // relationship is only stable between changes. private mappingChangeListeners = new Set<() => void>(); - constructor(nativeFetch = createEnvironmentAwareFetch()) { + constructor( + nativeFetch = createEnvironmentAwareFetch(), + opts?: { fetchHeaderTimeoutMs?: number }, + ) { this.nativeFetch = nativeFetch; + this.fetchHeaderTimeoutMs = + opts?.fetchHeaderTimeoutMs ?? defaultFetchHeaderTimeoutMs; this.mount(this.packageShimHandler.handle); } @@ -385,6 +390,7 @@ export class VirtualNetwork { } private nativeFetch: typeof globalThis.fetch; + private fetchHeaderTimeoutMs: number; private resolveURLMapping( url: string, @@ -488,8 +494,22 @@ export class VirtualNetwork { return next(await this.mapRequest(request, 'virtual-to-real')); }); - return withRetries(new URL(request.url), () => - fetcher(this.nativeFetch, handlers, this)(request, init), + return withRetries( + new URL(request.url), + this.fetchHeaderTimeoutMs, + (attemptSignal?: AbortSignal) => { + // Each attempt gets its own abort signal (see withRetries) so that a + // fetch aborted for stalling on one attempt doesn't poison the next. + // Rebuild the Request from a clone rather than mutating the original: + // the original is reused across attempts and constructing a Request + // consumes its body. + let attemptRequest = attemptSignal + ? new Request(request.clone(), { + signal: mergeAbortSignals(request.signal, attemptSignal), + }) + : request; + return fetcher(this.nativeFetch, handlers, this)(attemptRequest, init); + }, ); } @@ -577,6 +597,51 @@ const maxAttempts = 10; const backOffMs = 100; const retryableLocalHosts = new Set(['localhost', '127.0.0.1']); +// Longest a retryable base-realm fetch may wait for response headers in the +// browser test suite before it is aborted and retried (see withRetries). The +// "vanish" above has a second shape besides a thrown `TypeError: Failed to +// fetch`: the response headers simply never arrive, so the fetch neither +// resolves nor rejects. Comfortably above normal base-artifact header latency +// (sub-second) yet far below a per-test timeout, so it only ever fires on a +// genuine stall, and the retry then gets its headers on a fresh connection. +const defaultFetchHeaderTimeoutMs = 10_000; + +// Whether a retryable fetch should also be bounded by a header-arrival timeout. +// Browser test suite only: `document` is absent in node / worker / env-mode +// processes, where a legitimately slow response (e.g. a heavy `_search`) must +// never be aborted and retried. `__environment === 'test'` is the same gate +// shouldRetryFetch uses to decide base-realm retryability in the host. +export function shouldTimeoutRetryableFetch(url: URL): boolean { + let g = globalThis as { document?: unknown; __environment?: string }; + return ( + typeof g.document !== 'undefined' && + g.__environment === 'test' && + shouldRetryFetch(url) + ); +} + +// Combine an optional caller-supplied signal with the per-attempt timeout +// signal so a fetch is aborted when either fires. +function mergeAbortSignals( + a: AbortSignal | null | undefined, + b: AbortSignal | null | undefined, +): AbortSignal | undefined { + let signals = [a, b].filter((s): s is AbortSignal => Boolean(s)); + if (signals.length <= 1) { + return signals[0]; + } + let combine = ( + AbortSignal as unknown as { + any?: (signals: AbortSignal[]) => AbortSignal; + } + ).any; + // AbortSignal.any is present in every runtime we target; if it were somehow + // missing, honor the timeout signal (the last one) so the bound still holds. + return typeof combine === 'function' + ? combine(signals) + : signals[signals.length - 1]; +} + export function shouldRetryFetch(url: URL): boolean { // Env-mode services live at `..localhost` and are // reached through a local Traefik. The realm-server worker fetches @@ -631,12 +696,33 @@ export function shouldRetryFetch(url: URL): boolean { async function withRetries( url: URL, - fetchFn: () => ReturnType, + timeoutMs: number, + fetchFn: (attemptSignal?: AbortSignal) => ReturnType, ) { let attempt = 0; for (;;) { + // For a retryable fetch in the browser test suite, bound how long this + // attempt may wait for response headers. A CI stall where headers never + // arrive would otherwise leave the fetch neither resolved nor rejected, + // hanging the fetcher test-waiter until QUnit's global timeout; the abort + // turns that into the same retryable failure the catch below recovers + // from. The timer is cleared the moment the fetch settles, so a resolved + // response's body stream is never aborted (withRetries only awaits the + // response's headers, not its body). + let controller: AbortController | undefined; + let timeoutId: ReturnType | undefined; + if (shouldTimeoutRetryableFetch(url)) { + controller = new AbortController(); + timeoutId = setTimeout(() => { + let timeoutError = new Error( + `fetch for ${url.href} exceeded ${timeoutMs}ms without response headers`, + ); + timeoutError.name = 'FetchHeaderTimeout'; + controller!.abort(timeoutError); + }, timeoutMs); + } try { - return await fetchFn(); + return await fetchFn(controller?.signal); } catch (err: any) { if (!shouldRetryFetch(url) || ++attempt > maxAttempts) { if (shouldRetryFetch(url) && attempt > maxAttempts) { @@ -650,12 +736,19 @@ async function withRetries( } throw err; } + // Include the error so a future failure shows which "vanish" shape it + // was — a thrown `TypeError: Failed to fetch` or an aborted header stall + // (`FetchHeaderTimeout`). console.error( - `Encountered fetch failed for ${ - url.href - } retry attempt #${attempt} in ${attempt * backOffMs}ms`, + `Encountered fetch failed for ${url.href} (${err?.name ?? 'Error'}: ${ + err?.message ?? String(err) + }) retry attempt #${attempt} in ${attempt * backOffMs}ms`, ); await new Promise((r) => setTimeout(r, attempt * backOffMs)); + } finally { + if (timeoutId !== undefined) { + clearTimeout(timeoutId); + } } } } From 366c6bbcbe1659dab62699eb60a7f2e5acf5b682 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 23 Jul 2026 22:10:32 -0400 Subject: [PATCH 2/7] Preserve the abort signal when a request is remapped to its real URL The host reaches the base realm through a virtual-to-real URL mapping, and buildRequest reconstructed the mapped request without copying its signal. The per-attempt header-stall timeout is attached before the mapping runs, so the native fetch for a base artifact addressed at the virtual host (e.g. https://cardstack.com/base/_info) never saw the abort and could still hang. Carry the signal across the remap, and cover the mapped path in the header-stall recovery test. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/unit/virtual-network-retry-test.ts | 28 +++++++++++++++---- packages/runtime-common/virtual-network.ts | 6 ++++ 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/packages/host/tests/unit/virtual-network-retry-test.ts b/packages/host/tests/unit/virtual-network-retry-test.ts index a215e294801..c21f178a741 100644 --- a/packages/host/tests/unit/virtual-network-retry-test.ts +++ b/packages/host/tests/unit/virtual-network-retry-test.ts @@ -168,8 +168,10 @@ module('Unit | virtual-network shouldTimeoutRetryableFetch', function () { function stallingThenOkFetch(): { fetch: typeof globalThis.fetch; attempts: () => number; + signalSeen: () => boolean; } { let attempt = 0; + let sawAbort = false; let fetch = ((input: RequestInfo | URL, init?: RequestInit) => { attempt++; let signal = @@ -179,13 +181,15 @@ function stallingThenOkFetch(): { if (!signal) { return; // no signal wired => hang (fix broken); QUnit will time out } - if (signal.aborted) { + let onAbort = () => { + sawAbort = true; reject(signal.reason); + }; + if (signal.aborted) { + onAbort(); return; } - signal.addEventListener('abort', () => reject(signal.reason), { - once: true, - }); + signal.addEventListener('abort', onAbort, { once: true }); }); } return Promise.resolve( @@ -195,7 +199,7 @@ function stallingThenOkFetch(): { }), ); }) as typeof globalThis.fetch; - return { fetch, attempts: () => attempt }; + return { fetch, attempts: () => attempt, signalSeen: () => sawAbort }; } module('Unit | virtual-network header-stall recovery', function () { @@ -205,10 +209,18 @@ module('Unit | virtual-network header-stall recovery', function () { let prev = g.__environment; g.__environment = 'test'; try { - let { fetch, attempts } = stallingThenOkFetch(); + let { fetch, attempts, signalSeen } = stallingThenOkFetch(); // Tiny header timeout so the stalled first attempt is aborted promptly; // the second attempt then succeeds. let vn = new VirtualNetwork(fetch, { fetchHeaderTimeoutMs: 20 }); + // The host reaches the base realm through a virtual-to-real URL mapping, + // so exercise that path: the per-attempt timeout signal is attached + // before the mapping runs and must survive the request being rebuilt at + // the mapped URL, or the native fetch never sees the abort. + vn.addURLMapping( + new URL('https://cardstack.com/base/'), + new URL('https://realm-server.ci.localhost/base/'), + ); let response = await vn.fetch('https://cardstack.com/base/card-api'); assert.strictEqual( response.status, @@ -219,6 +231,10 @@ module('Unit | virtual-network header-stall recovery', function () { attempts() >= 2, 'the stalled first attempt was retried on a fresh fetch', ); + assert.true( + signalSeen(), + 'the native fetch received an abort signal through the URL mapping', + ); } finally { if (had) { g.__environment = prev; diff --git a/packages/runtime-common/virtual-network.ts b/packages/runtime-common/virtual-network.ts index 4ec38b681b5..33814184454 100644 --- a/packages/runtime-common/virtual-network.ts +++ b/packages/runtime-common/virtual-network.ts @@ -786,5 +786,11 @@ async function buildRequest(url: string, originalRequest: Request) { cache: originalRequest.cache, redirect: originalRequest.redirect, integrity: originalRequest.integrity, + // Carry the abort signal across the remap so a caller's abort — and the + // per-attempt header-stall timeout in withRetries — still cancels the + // native fetch. The host maps virtual base-realm URLs + // (https://cardstack.com/base/...) to the resolved realm URL here, so + // without this the base fetch that reaches the network has no signal. + signal: originalRequest.signal, }); } From 8f0be66df9bcd483ef9ac78457df633d16b07dc0 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Thu, 23 Jul 2026 22:42:58 -0400 Subject: [PATCH 3/7] Cover signal-across-remap directly and stabilize the recovery test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header-stall recovery test's stub returned a bare Response whose empty url tripped the virtual network's url bookkeeping once a URL mapping was in play. Give the stub a resolved (configurable) url like a real fetch, keep the recovery test on the direct fetch path, and cover the virtual-to-real signal preservation with a focused buildRequest test instead — buildRequest is now exported for that. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/unit/virtual-network-retry-test.ts | 55 ++++++++++++++----- packages/runtime-common/virtual-network.ts | 2 +- 2 files changed, 41 insertions(+), 16 deletions(-) diff --git a/packages/host/tests/unit/virtual-network-retry-test.ts b/packages/host/tests/unit/virtual-network-retry-test.ts index c21f178a741..62b3e69434f 100644 --- a/packages/host/tests/unit/virtual-network-retry-test.ts +++ b/packages/host/tests/unit/virtual-network-retry-test.ts @@ -2,6 +2,7 @@ import { module, test } from 'qunit'; import { VirtualNetwork } from '@cardstack/runtime-common'; import { + buildRequest, shouldRetryFetch, shouldTimeoutRetryableFetch, } from '@cardstack/runtime-common/virtual-network'; @@ -192,12 +193,16 @@ function stallingThenOkFetch(): { signal.addEventListener('abort', onAbort, { once: true }); }); } - return Promise.resolve( - new Response('{"ok":true}', { - status: 200, - headers: { 'content-type': 'application/json' }, - }), - ); + let ok = new Response('{"ok":true}', { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + // A real fetch response carries its resolved URL; mirror that (configurable + // so the virtual network's own url bookkeeping can still overwrite it) + // rather than returning the empty-url default a bare Response would have. + let url = input instanceof Request ? input.url : String(input); + Object.defineProperty(ok, 'url', { value: url, configurable: true }); + return Promise.resolve(ok); }) as typeof globalThis.fetch; return { fetch, attempts: () => attempt, signalSeen: () => sawAbort }; } @@ -213,14 +218,6 @@ module('Unit | virtual-network header-stall recovery', function () { // Tiny header timeout so the stalled first attempt is aborted promptly; // the second attempt then succeeds. let vn = new VirtualNetwork(fetch, { fetchHeaderTimeoutMs: 20 }); - // The host reaches the base realm through a virtual-to-real URL mapping, - // so exercise that path: the per-attempt timeout signal is attached - // before the mapping runs and must survive the request being rebuilt at - // the mapped URL, or the native fetch never sees the abort. - vn.addURLMapping( - new URL('https://cardstack.com/base/'), - new URL('https://realm-server.ci.localhost/base/'), - ); let response = await vn.fetch('https://cardstack.com/base/card-api'); assert.strictEqual( response.status, @@ -233,7 +230,7 @@ module('Unit | virtual-network header-stall recovery', function () { ); assert.true( signalSeen(), - 'the native fetch received an abort signal through the URL mapping', + 'the aborted attempt fired the per-attempt timeout signal', ); } finally { if (had) { @@ -243,4 +240,32 @@ module('Unit | virtual-network header-stall recovery', function () { } } }); + + // The host reaches the base realm through a virtual-to-real URL mapping, and + // the per-attempt timeout signal is attached before that mapping runs. The + // remapped request must carry the signal through, or the native fetch for a + // base artifact addressed at the virtual host never sees the abort. + test('buildRequest carries the abort signal onto the remapped request', async function (assert) { + let controller = new AbortController(); + let original = new Request('https://cardstack.com/base/_info', { + method: 'QUERY', + body: JSON.stringify({ realms: ['https://cardstack.com/base/'] }), + signal: controller.signal, + }); + let remapped = await buildRequest( + 'https://realm-server.ci.localhost/base/_info', + original, + ); + assert.notStrictEqual( + remapped.url, + original.url, + 'the request was rebuilt at the mapped real URL', + ); + assert.false(remapped.signal.aborted, 'not aborted before the abort fires'); + controller.abort(new Error('stall')); + assert.true( + remapped.signal.aborted, + 'aborting the original aborts the remapped request', + ); + }); }); diff --git a/packages/runtime-common/virtual-network.ts b/packages/runtime-common/virtual-network.ts index 33814184454..a83ae1ee43f 100644 --- a/packages/runtime-common/virtual-network.ts +++ b/packages/runtime-common/virtual-network.ts @@ -753,7 +753,7 @@ async function withRetries( } } -async function buildRequest(url: string, originalRequest: Request) { +export async function buildRequest(url: string, originalRequest: Request) { if (url === originalRequest.url) { return originalRequest; } From 3db42d7379b0d76e23fadca85ef9da1b4239e623 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Fri, 24 Jul 2026 03:01:38 -0400 Subject: [PATCH 4/7] Word the header-timeout comments as a failure mode, not an incident MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit State the condition the bound handles — a fetch whose response headers never arrive — directly, rather than framing it as a specific observed occurrence. Comments-only. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/unit/virtual-network-retry-test.ts | 22 ++++++++++--------- packages/runtime-common/virtual-network.ts | 18 +++++++-------- 2 files changed, 21 insertions(+), 19 deletions(-) diff --git a/packages/host/tests/unit/virtual-network-retry-test.ts b/packages/host/tests/unit/virtual-network-retry-test.ts index 62b3e69434f..dd3ca0d9df2 100644 --- a/packages/host/tests/unit/virtual-network-retry-test.ts +++ b/packages/host/tests/unit/virtual-network-retry-test.ts @@ -107,12 +107,12 @@ module('Unit | virtual-network shouldRetryFetch', function () { }); }); -// `shouldTimeoutRetryableFetch` gates the header-arrival timeout that turns the -// second shape of the CI "vanish" — headers that never arrive, so the fetch -// hangs instead of throwing — into the same retryable failure the throw path -// already recovers from. It bounds only fetches that are already retryable, and -// only in the browser test suite: a slow response in node / worker / env-mode -// (e.g. a heavy `_search`) must never be aborted and retried. +// `shouldTimeoutRetryableFetch` gates the header-arrival timeout that turns a +// stall — response headers that never arrive, so the fetch hangs instead of +// throwing — into the same retryable failure the throw path already recovers +// from. It bounds only fetches that are already retryable, and only in the +// browser test suite: a slow response in node / worker / env-mode (e.g. a +// heavy `_search`) must never be aborted and retried. module('Unit | virtual-network shouldTimeoutRetryableFetch', function () { test('bounds a retryable base-realm fetch in the test environment', function (assert) { withEnvironment('test', () => { @@ -120,7 +120,7 @@ module('Unit | virtual-network shouldTimeoutRetryableFetch', function () { shouldTimeoutRetryableFetch( new URL('https://realm-server.ci.localhost/base/_info'), ), - 'the base _info fetch that hung in CI is bounded', + 'the base _info fetch that can stall before headers is bounded', ); assert.true( shouldTimeoutRetryableFetch( @@ -164,8 +164,8 @@ module('Unit | virtual-network shouldTimeoutRetryableFetch', function () { // A native fetch whose first attempt stalls at the header stage (never // resolves; rejects only when its request signal aborts, exactly as a real -// fetch does), then succeeds. Mirrors the CI failure where base/_info headers -// never arrived and the unbounded fetch hung until QUnit's global timeout. +// fetch does), then succeeds. Mirrors a base-realm fetch whose response headers +// never arrive, so the unbounded fetch hangs until QUnit's global timeout. function stallingThenOkFetch(): { fetch: typeof globalThis.fetch; attempts: () => number; @@ -180,7 +180,9 @@ function stallingThenOkFetch(): { if (attempt === 1) { return new Promise((_resolve, reject) => { if (!signal) { - return; // no signal wired => hang (fix broken); QUnit will time out + // No signal reached the fetch, so nothing can abort this attempt: it + // stays pending and the test fails via QUnit's timeout. + return; } let onAbort = () => { sawAbort = true; diff --git a/packages/runtime-common/virtual-network.ts b/packages/runtime-common/virtual-network.ts index a83ae1ee43f..7d6e7665313 100644 --- a/packages/runtime-common/virtual-network.ts +++ b/packages/runtime-common/virtual-network.ts @@ -598,12 +598,12 @@ const backOffMs = 100; const retryableLocalHosts = new Set(['localhost', '127.0.0.1']); // Longest a retryable base-realm fetch may wait for response headers in the -// browser test suite before it is aborted and retried (see withRetries). The -// "vanish" above has a second shape besides a thrown `TypeError: Failed to -// fetch`: the response headers simply never arrive, so the fetch neither -// resolves nor rejects. Comfortably above normal base-artifact header latency -// (sub-second) yet far below a per-test timeout, so it only ever fires on a -// genuine stall, and the retry then gets its headers on a fresh connection. +// browser test suite before it is aborted and retried (see withRetries). A +// stall where the response headers never arrive is the failure mode the +// thrown-error retry net above does not cover: the fetch neither resolves nor +// rejects. Comfortably above normal base-artifact header latency (sub-second) +// yet far below a per-test timeout, so it only ever fires on a genuine stall, +// and the retry then gets its headers on a fresh connection. const defaultFetchHeaderTimeoutMs = 10_000; // Whether a retryable fetch should also be bounded by a header-arrival timeout. @@ -702,7 +702,7 @@ async function withRetries( let attempt = 0; for (;;) { // For a retryable fetch in the browser test suite, bound how long this - // attempt may wait for response headers. A CI stall where headers never + // attempt may wait for response headers. A stall where the headers never // arrive would otherwise leave the fetch neither resolved nor rejected, // hanging the fetcher test-waiter until QUnit's global timeout; the abort // turns that into the same retryable failure the catch below recovers @@ -736,8 +736,8 @@ async function withRetries( } throw err; } - // Include the error so a future failure shows which "vanish" shape it - // was — a thrown `TypeError: Failed to fetch` or an aborted header stall + // Include the error so a failure surfaces which shape it took — a thrown + // `TypeError: Failed to fetch` or an aborted header stall // (`FetchHeaderTimeout`). console.error( `Encountered fetch failed for ${url.href} (${err?.name ?? 'Error'}: ${ From 864300436c75d2c7289c86fb65b46d1f3f65fc78 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Fri, 24 Jul 2026 07:31:56 -0400 Subject: [PATCH 5/7] Drop qunit-testing.md from the pending boxel references MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit qunit-testing.md now ships in the built boxel skill, so it no longer belongs in PENDING_BOXEL_REFERENCES — the skill-loader validation test fails while a shipped reference is still listed as pending. Empty the list and let the "pending references are still pending" test pass cleanly when nothing is pending. Co-Authored-By: Claude Opus 4.8 (1M context) --- packages/software-factory/src/factory-skill-loader.ts | 2 +- packages/software-factory/tests/factory-skill-loader.test.ts | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/software-factory/src/factory-skill-loader.ts b/packages/software-factory/src/factory-skill-loader.ts index bb5fc2a7588..48c750f9f4e 100644 --- a/packages/software-factory/src/factory-skill-loader.ts +++ b/packages/software-factory/src/factory-skill-loader.ts @@ -165,7 +165,7 @@ export const ALWAYS_LOAD_REFERENCES: readonly string[] = [ * and the validation test uses this set strictly: once a name ships in the * built skill, the test fails until it is removed from here. */ -export const PENDING_BOXEL_REFERENCES: readonly string[] = ['qunit-testing.md']; +export const PENDING_BOXEL_REFERENCES: readonly string[] = []; // --------------------------------------------------------------------------- // Internal types for tracking reference metadata diff --git a/packages/software-factory/tests/factory-skill-loader.test.ts b/packages/software-factory/tests/factory-skill-loader.test.ts index 6502f6aed75..47d6d90d313 100644 --- a/packages/software-factory/tests/factory-skill-loader.test.ts +++ b/packages/software-factory/tests/factory-skill-loader.test.ts @@ -1007,6 +1007,10 @@ module('factory-skill-loader > curated boxel references', function () { // Strict on purpose: once a pending name ships in the built skill, // remove it from PENDING_BOXEL_REFERENCES (and any transitional notes // that reference it). + if (PENDING_BOXEL_REFERENCES.length === 0) { + assert.ok(true, 'no references are pending'); + return; + } for (let name of PENDING_BOXEL_REFERENCES) { assert.false( existsSync(join(builtRefsDir, name)), From 6d085393397b4a64f62dd4d0f14a309a40b6d306 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Fri, 24 Jul 2026 07:40:17 -0400 Subject: [PATCH 6/7] Use expect() instead of an early return for the empty pending case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The QUnit lint rule forbids returning early from a test. Declare the assertion count up front — expect(0) accepts the empty pending list without running an assertion — instead of short-circuiting. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../software-factory/tests/factory-skill-loader.test.ts | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/software-factory/tests/factory-skill-loader.test.ts b/packages/software-factory/tests/factory-skill-loader.test.ts index 47d6d90d313..6447dba9699 100644 --- a/packages/software-factory/tests/factory-skill-loader.test.ts +++ b/packages/software-factory/tests/factory-skill-loader.test.ts @@ -1006,11 +1006,9 @@ module('factory-skill-loader > curated boxel references', function () { test('pending references are still pending', function (assert) { // Strict on purpose: once a pending name ships in the built skill, // remove it from PENDING_BOXEL_REFERENCES (and any transitional notes - // that reference it). - if (PENDING_BOXEL_REFERENCES.length === 0) { - assert.ok(true, 'no references are pending'); - return; - } + // that reference it). expect() accepts the empty case, where the loop + // runs no assertions. + assert.expect(PENDING_BOXEL_REFERENCES.length); for (let name of PENDING_BOXEL_REFERENCES) { assert.false( existsSync(join(builtRefsDir, name)), From d7ff717dd6143ba54cc56b5e3fb1596eb4107d85 Mon Sep 17 00:00:00 2001 From: Hassan Abdel-Rahman Date: Fri, 24 Jul 2026 19:58:40 -0400 Subject: [PATCH 7/7] Surface caller-initiated aborts immediately instead of retrying MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit withRetries gated retry on shouldRetryFetch alone, so an AbortError from a caller-supplied signal was treated as transient — retried up to the cap (re-merging the already-aborted signal each round) before rethrowing. Rethrow AbortError at once; the per-attempt header-stall abort names its error FetchHeaderTimeout, so the stall-retry path is unaffected. Also make the mergeAbortSignals AbortSignal.any fallback a correct manual merge rather than silently dropping the caller signal. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../tests/unit/virtual-network-retry-test.ts | 51 +++++++++++++++++++ packages/runtime-common/virtual-network.ts | 28 ++++++++-- 2 files changed, 74 insertions(+), 5 deletions(-) diff --git a/packages/host/tests/unit/virtual-network-retry-test.ts b/packages/host/tests/unit/virtual-network-retry-test.ts index dd3ca0d9df2..3bbf864825d 100644 --- a/packages/host/tests/unit/virtual-network-retry-test.ts +++ b/packages/host/tests/unit/virtual-network-retry-test.ts @@ -270,4 +270,55 @@ module('Unit | virtual-network header-stall recovery', function () { 'aborting the original aborts the remapped request', ); }); + + test('a caller-initiated abort is surfaced at once, not retried', async function (assert) { + let g = globalThis as { __environment?: string }; + let had = '__environment' in g; + let prev = g.__environment; + g.__environment = 'test'; + try { + let attempts = 0; + let fetch = ((input: RequestInfo | URL) => { + attempts++; + let signal = input instanceof Request ? input.signal : undefined; + // Reject with the signal's reason when aborted, as a real fetch does. + return new Promise((_resolve, reject) => { + if (!signal) { + return; + } + let onAbort = () => reject(signal.reason); + if (signal.aborted) { + onAbort(); + } else { + signal.addEventListener('abort', onAbort, { once: true }); + } + }); + }) as typeof globalThis.fetch; + // A large header timeout so only the caller's abort ends the fetch. The + // URL is retryable, so without the guard this would retry the + // already-aborted request rather than surfacing the abort. + let vn = new VirtualNetwork(fetch, { fetchHeaderTimeoutMs: 60_000 }); + let caller = new AbortController(); + let pending = vn.fetch( + new Request('https://cardstack.com/base/card-api', { + signal: caller.signal, + }), + ); + caller.abort(); + let caught: { name?: string } | undefined; + try { + await pending; + } catch (e) { + caught = e as { name?: string }; + } + assert.strictEqual(caught?.name, 'AbortError', 'the abort was surfaced'); + assert.strictEqual(attempts, 1, 'the aborted request was not retried'); + } finally { + if (had) { + g.__environment = prev; + } else { + delete g.__environment; + } + } + }); }); diff --git a/packages/runtime-common/virtual-network.ts b/packages/runtime-common/virtual-network.ts index 7d6e7665313..1e2e8a92906 100644 --- a/packages/runtime-common/virtual-network.ts +++ b/packages/runtime-common/virtual-network.ts @@ -635,11 +635,22 @@ function mergeAbortSignals( any?: (signals: AbortSignal[]) => AbortSignal; } ).any; - // AbortSignal.any is present in every runtime we target; if it were somehow - // missing, honor the timeout signal (the last one) so the bound still holds. - return typeof combine === 'function' - ? combine(signals) - : signals[signals.length - 1]; + if (typeof combine === 'function') { + return combine(signals); + } + // Fallback for a runtime without AbortSignal.any: mirror both signals onto a + // single controller so either firing aborts the fetch, carrying its reason. + let controller = new AbortController(); + for (let signal of signals) { + if (signal.aborted) { + controller.abort(signal.reason); + break; + } + signal.addEventListener('abort', () => controller.abort(signal.reason), { + once: true, + }); + } + return controller.signal; } export function shouldRetryFetch(url: URL): boolean { @@ -724,6 +735,13 @@ async function withRetries( try { return await fetchFn(controller?.signal); } catch (err: any) { + // A caller-initiated abort is not a transient failure — surface it at + // once rather than retrying the already-aborted request. The per-attempt + // header-stall abort names its error `FetchHeaderTimeout`, so it falls + // through to the retry path below. + if (err?.name === 'AbortError') { + throw err; + } if (!shouldRetryFetch(url) || ++attempt > maxAttempts) { if (shouldRetryFetch(url) && attempt > maxAttempts) { // Final-exhaustion log: distinct from the per-attempt warning so