From 42467de3b177301e93423b5749dfbce91161a3e3 Mon Sep 17 00:00:00 2001 From: Oleksandr Zhuravlov Date: Mon, 6 Jul 2026 20:42:58 +0300 Subject: [PATCH 1/2] fix(core): reject partial (206) download responses; add a test rig The `download` surface's `interpret` accepted any sub-400 status as a complete Blob, so a stray `206 Partial Content` (a range-serving proxy/CDN answering a request that never sent a Range) was handed back as if it were the whole file. `interpret` now accepts only a complete body (200/204) and rejects everything else. Adds a controllable download test rig (extending the zero-dep node:http mock server) that reproduces real-world server/network behavior and pins the download surface against it: - byte accounting: stray-206 rejection; Content-Length-mismatch truncation (undici hangs on a clean FIN, cured by `timeout`) - network faults: mid-body RST, idle stall, slow TTFB, steady throttle - redirects: cross-origin credential strip proven end-to-end through download() (the signed-URL -> CDN pattern), same-origin no-over-strip, redirect-loop -> interpret-reject, retry x redirect No engine change; the rig is test-only. Two findings recorded in the specs for the future @stitchapi/download package: the engine has no idle/forward-progress timeout (a healthy-but-slow download dies like a stall), and the transport `.cause` (UND_ERR_SOCKET) is stripped before callers see it. Co-Authored-By: Claude Opus 4.8 --- packages/core/src/download.ts | 19 ++ .../download-redirect-cross-origin.spec.ts | 117 ++++++++ .../test/gaps/download-redirect-loop.spec.ts | 53 ++++ .../test/gaps/download-redirect-retry.spec.ts | 91 ++++++ .../download-redirect-same-origin.spec.ts | 84 ++++++ .../test/gaps/download-reset-midbody.spec.ts | 72 +++++ .../core/test/gaps/download-slow-ttfb.spec.ts | 75 +++++ .../test/gaps/download-slow-vs-stall.spec.ts | 92 ++++++ .../core/test/gaps/download-stall.spec.ts | 67 +++++ .../core/test/gaps/download-stray-206.spec.ts | 45 +++ .../test/gaps/download-truncation.spec.ts | 58 ++++ packages/core/test/support/mock-server.ts | 266 +++++++++++++++++- 12 files changed, 1037 insertions(+), 2 deletions(-) create mode 100644 packages/core/test/gaps/download-redirect-cross-origin.spec.ts create mode 100644 packages/core/test/gaps/download-redirect-loop.spec.ts create mode 100644 packages/core/test/gaps/download-redirect-retry.spec.ts create mode 100644 packages/core/test/gaps/download-redirect-same-origin.spec.ts create mode 100644 packages/core/test/gaps/download-reset-midbody.spec.ts create mode 100644 packages/core/test/gaps/download-slow-ttfb.spec.ts create mode 100644 packages/core/test/gaps/download-slow-vs-stall.spec.ts create mode 100644 packages/core/test/gaps/download-stall.spec.ts create mode 100644 packages/core/test/gaps/download-stray-206.spec.ts create mode 100644 packages/core/test/gaps/download-truncation.spec.ts diff --git a/packages/core/src/download.ts b/packages/core/src/download.ts index 71b2bcdd..2cf9c224 100644 --- a/packages/core/src/download.ts +++ b/packages/core/src/download.ts @@ -84,6 +84,25 @@ export const downloadSurface: Surface = { responseType: 'blob', }), interpret: (res): SurfaceOutcome => { + // A buffered download resolves to a WHOLE Blob, so only a complete-body status is a success: + // `200 OK`, or `204 No Content` (a legitimately empty body). The engine's attempt loop only + // throws on `>= 400` (engine.ts), so every other sub-400 status — most importantly `206 + // Partial Content` — flows here. `download` never sends a `Range`, so a `206` is a partial + // body the server volunteered (a range-serving proxy/CDN, a resumed-and-mismatched cache); if + // it were accepted it would hand the caller a truncated file as if it were the whole thing. + // Reject anything that is not a complete 2xx. + if (res.status !== 200 && res.status !== 204) + return { + ok: false, + message: `download: expected a complete body (200/204) but got HTTP ${res.status}${res.status === 206 ? ' — a partial (206) response was not requested (no Range header is ever sent)' : ''}`, + status: res.status, + }; + // No `blob.size` vs `content-length` cross-check here: a truncated body (Content-Length + // advertises more bytes than are sent) never reaches this hook. undici HANGS on a clean-FIN + // short read until the caller's `timeout` fires, and REJECTS on an abrupt socket close — so a + // truncation surfaces as a timeout / transport error, never as a short Blob delivered here. A + // length check would be dead code (and a gzip/br body legitimately has `size != content-length` + // once decoded — it would be a false positive). See test/gaps/download-truncation.spec.ts. const value: DownloadResult = { blob: res.body as Blob }; const filename = filenameFromDisposition(res.headers['content-disposition']) ?? diff --git a/packages/core/test/gaps/download-redirect-cross-origin.spec.ts b/packages/core/test/gaps/download-redirect-cross-origin.spec.ts new file mode 100644 index 00000000..0b1f29e0 --- /dev/null +++ b/packages/core/test/gaps/download-redirect-cross-origin.spec.ts @@ -0,0 +1,117 @@ +// Pins: a `download` that follows a CROSS-ORIGIN redirect must NOT forward credential/custom headers +// to the redirect target — proven end-to-end through the download surface, over real sockets. +// +// This is the signed-URL → CDN pattern (X2/D2b): a client hits a trusted API with an API key / AWS +// SigV4 headers; the API answers `302 Location: https://cdn.other/…` (a different origin); the CDN +// serves the actual bytes. The default HTTP redirect policy re-sends every request header to the +// target, and undici/axios strip only `authorization`/`cookie` on a cross-origin hop — never custom +// headers like `x-api-key`/`x-amz-*`. `download()` rides fetchAdapter, which follows redirects +// itself (`redirect: 'manual'` + `headersForRedirect`) and drops every non-CORS-safelisted header on +// a cross-origin hop, so the strip is INHERITED here — this is not a new fix, it pins the composed +// behavior: the strip + the buffered download + filename resolution across the hop. +// +// Two independent mock servers → a real cross-origin pair: each `startMockServer()` binds its own +// ephemeral 127.0.0.1 port, and `sameOrigin` treats different ports as cross-origin (host includes +// the port). A serves the 302; B serves the file. We assert on B's RECEIVED request headers (what +// actually crossed the wire), that the download RESOLVES with B's body, and that `out.filename` comes +// from B's `Content-Disposition` — the FINAL hop, not A. +// +// Real-timer, loose bounds (a socket test): the hop is local and prompt; the only time assertion is a +// wide ceiling proving it never hangs. Both servers are closed in afterAll. +import { apiKey, env } from '../../src'; +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer, ReqInfo } from '../support/mock-server'; + +// The sensitive headers that must never survive a cross-origin hop (an API key + a SigV4 set). +const API_KEY = 'sk-secret-cross-origin'; +const AWS_AUTH = 'AWS4-HMAC-SHA256 Credential=AKIA.../...'; + +// Pull a header case-insensitively from a recorded request (matches redirect-credential-leak.spec). +const hdr = (h: Record, name: string): string | undefined => { + const k = Object.keys(h).find( + (x) => x.toLowerCase() === name.toLowerCase(), + ); + return k ? h[k] : undefined; +}; + +let a: MockServer; // the trusted API that 302s away +let b: MockServer; // the "CDN" on another origin that serves the file + +beforeAll(async () => { + a = await startMockServer(); + b = await startMockServer(); +}); +afterAll(async () => { + // Close BOTH servers so the suite exits cleanly. + await Promise.all([a.close(), b.close()]); +}); +beforeEach(() => { + a.reset(); + b.reset(); +}); + +// The file B hands back once the redirect lands — asserted byte-for-byte on the resolved Blob. +const FILE_BYTES = 'CDN-SERVED-ASSET-BODY-0123456789'; + +test('a cross-origin redirect drops x-api-key / authorization / x-amz-* before the target is hit, resolves B’s body, and names the file from B’s final-hop Content-Disposition', async () => { + // A: any request 302s to B's /file (an ABSOLUTE, cross-origin URL — different ephemeral port). + a.route('GET', '/signed', { + statuses: [302], + redirectTo: `${b.url}/file`, + }); + // B: serves the actual bytes + a Content-Disposition naming the download. + b.route('GET', '/file', { + statuses: [200], + rawBody: FILE_BYTES, + headers: { + 'content-disposition': 'attachment; filename="asset.bin"', + }, + }); + + // Distinct ephemeral ports ⇒ a real cross-origin pair. Guards the fixture's core premise. + expect(new URL(a.url).port).not.toBe(new URL(b.url).port); + + process.env['XORIGIN_REDIRECT_KEY'] = API_KEY; + const getAsset = download({ + baseUrl: a.url, + path: '/signed', + // apiKey() writes `x-api-key`; the SigV4-style headers are set directly (awsSigV4 isn't a + // public export) — together they cover the full credential/custom-header set the strip must drop. + auth: apiKey({ value: env('XORIGIN_REDIRECT_KEY') }), + headers: { + authorization: AWS_AUTH, + 'x-amz-date': '20260706T000000Z', + 'x-amz-content-sha256': 'abc123def456', + }, + timeout: 5000, + retry: { attempts: 1 }, + }); + + const started = Date.now(); + const out = await getAsset(); + expect(Date.now() - started).toBeLessThan(5000); // never hangs + + // (a) NONE of the credential/custom headers reached B (the cross-origin target). + const received: ReqInfo | undefined = b.calls('/file')[0]; + expect(received).toBeDefined(); + const leaked = received?.headers ?? {}; + expect(hdr(leaked, 'x-api-key')).toBeUndefined(); + expect(hdr(leaked, 'authorization')).toBeUndefined(); + expect(hdr(leaked, 'x-amz-date')).toBeUndefined(); + expect(hdr(leaked, 'x-amz-content-sha256')).toBeUndefined(); + + // Sanity: A DID receive the key on the first (trusted, same-origin) hop — the strip is scoped to + // the cross-origin follow, not an over-strip that never sent the credential at all. + expect(hdr(a.calls('/signed')[0]?.headers ?? {}, 'x-api-key')).toBe( + API_KEY, + ); + + // (b) The download RESOLVES with B's body. + expect(new TextDecoder().decode(await out.blob.arrayBuffer())).toBe( + FILE_BYTES, + ); + + // (c) The filename comes from B's Content-Disposition — the FINAL hop, not A's URL/headers. + expect(out.filename).toBe('asset.bin'); +}); diff --git a/packages/core/test/gaps/download-redirect-loop.spec.ts b/packages/core/test/gaps/download-redirect-loop.spec.ts new file mode 100644 index 00000000..6a9e37aa --- /dev/null +++ b/packages/core/test/gaps/download-redirect-loop.spec.ts @@ -0,0 +1,53 @@ +// Pins: a `download` caught in a redirect LOOP must REJECT — bounded, never a hang. +// +// A route that 302s to itself forever would spin an uncapped follower. fetchAdapter's manual-follow +// loop caps at MAX_REDIRECTS (~20, matching the platform default) and, once the cap is hit, returns +// the LAST 3xx response instead of following again. That 30x then flows into `download`'s M1 +// `interpret`, which accepts only a complete 200/204 — so the capped loop surfaces as a +// `download:`-interpret REJECT (an HTTP 30x message), NOT a hang and NOT a silently-accepted Blob. +// This pins that composition end-to-end over a real socket; no `src/` change is involved. +// +// Real-timer, loose bounds (a socket test): the assertion is a wide wall-clock ceiling proving the +// call returns promptly (bounded hops, each a local round-trip), plus a bounded hop count from the +// server's own call log. The server is closed in afterAll. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +test('a self-redirect loop is bounded and REJECTS (the capped 30x fails interpret) — never hangs', async () => { + // Every GET /loop answers 302 → /loop again: an infinite chain if uncapped. + server.route('GET', '/loop', { + statuses: [302], + redirectTo: '/loop', + }); + + const getLooping = download({ + baseUrl: server.url, + path: '/loop', + retry: { attempts: 1 }, // don't let a retry re-run the whole loop + }); + + const started = Date.now(); + // The capped loop yields a 30x; interpret rejects it as a non-200/204 body. The message is the + // download surface's "expected a complete body (200/204) but got HTTP 30x". + await expect(getLooping()).rejects.toThrow(/30\d|complete body|download/i); + // Bounded wall-clock: the whole capped chain of local round-trips finishes well under the ceiling. + expect(Date.now() - started).toBeLessThan(10000); + + // Bounded hop count: the initial request + at most MAX_REDIRECTS (~20) follows. The loop stopped; + // it did not spin. (> 1 proves it actually followed at least one redirect.) + const hops = server.callCount('/loop'); + expect(hops).toBeGreaterThan(1); + expect(hops).toBeLessThanOrEqual(21); +}); diff --git a/packages/core/test/gaps/download-redirect-retry.spec.ts b/packages/core/test/gaps/download-redirect-retry.spec.ts new file mode 100644 index 00000000..68adbc70 --- /dev/null +++ b/packages/core/test/gaps/download-redirect-retry.spec.ts @@ -0,0 +1,91 @@ +// Pins (D6): the cross-origin credential strip holds even when the RETRIED attempt is the one that +// redirects. A transient failure (503) is retried by the engine; the retried attempt answers a +// cross-origin 302, and the strip must still drop the credential/custom headers on that hop. +// +// This composes the retry policy with the redirect strip through the download surface, over real +// sockets. A's route is scripted `[503, 302]`: the first attempt gets a retryable 503 (503 is in the +// default retry set), the engine retries, and the second attempt 302s to B on another origin. B +// serves the file. We assert B NEVER received the credential/custom headers (the strip held on the +// retried attempt), the download resolved with B's body, and — belt-and-braces — that A really was +// hit twice (the 503 then the 302). No `src/` change is involved; the strip is inherited from +// fetchAdapter, the retry from the engine. +// +// Real-timer, loose bounds (a socket test): a tiny `baseDelay` keeps the single backoff negligible; +// the only time assertion is a wide ceiling. Both servers are closed in afterAll. +import { apiKey, env } from '../../src'; +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +const API_KEY = 'sk-secret-retry-redirect'; +const AWS_AUTH = 'AWS4-HMAC-SHA256 Credential=AKIA.../...'; + +const hdr = (h: Record, name: string): string | undefined => { + const k = Object.keys(h).find( + (x) => x.toLowerCase() === name.toLowerCase(), + ); + return k ? h[k] : undefined; +}; + +let a: MockServer; +let b: MockServer; +beforeAll(async () => { + a = await startMockServer(); + b = await startMockServer(); +}); +afterAll(async () => { + await Promise.all([a.close(), b.close()]); +}); +beforeEach(() => { + a.reset(); + b.reset(); +}); + +const FILE_BYTES = 'RETRIED-THEN-REDIRECTED-CDN-BODY'; + +test('the strip holds when the RETRIED attempt is the one that redirects cross-origin', async () => { + // A: first call 503 (retryable), second call 302 → B (cross-origin). The 503 falls through to a + // normal body response; only the 302 call redirects. + a.route('GET', '/flaky', { + statuses: [503, 302], + redirectTo: `${b.url}/file`, + }); + b.route('GET', '/file', { + statuses: [200], + rawBody: FILE_BYTES, + headers: { 'content-disposition': 'attachment; filename="late.bin"' }, + }); + + process.env['RETRY_REDIRECT_KEY'] = API_KEY; + const getAsset = download({ + baseUrl: a.url, + path: '/flaky', + auth: apiKey({ value: env('RETRY_REDIRECT_KEY') }), + headers: { + authorization: AWS_AUTH, + 'x-amz-date': '20260706T000000Z', + 'x-amz-content-sha256': 'abc123def456', + }, + // Allow one retry so the 503 is not terminal; a tiny backoff keeps the test fast. + retry: { attempts: 2, on: [503], baseDelay: 5 }, + timeout: 5000, + }); + + const out = await getAsset(); + + // A was hit TWICE: the 503, then the 302 (guards that the retry actually happened). + expect(a.callCount('/flaky')).toBe(2); + + // The strip held on the retried, cross-origin hop: NONE of the sensitive headers reached B. + const leaked = b.calls('/file')[0]?.headers ?? {}; + expect(hdr(leaked, 'x-api-key')).toBeUndefined(); + expect(hdr(leaked, 'authorization')).toBeUndefined(); + expect(hdr(leaked, 'x-amz-date')).toBeUndefined(); + expect(hdr(leaked, 'x-amz-content-sha256')).toBeUndefined(); + + // The download still resolved with B's body + final-hop filename. + expect(new TextDecoder().decode(await out.blob.arrayBuffer())).toBe( + FILE_BYTES, + ); + expect(out.filename).toBe('late.bin'); +}); diff --git a/packages/core/test/gaps/download-redirect-same-origin.spec.ts b/packages/core/test/gaps/download-redirect-same-origin.spec.ts new file mode 100644 index 00000000..619f061e --- /dev/null +++ b/packages/core/test/gaps/download-redirect-same-origin.spec.ts @@ -0,0 +1,84 @@ +// Pins: a `download` that follows a SAME-ORIGIN redirect KEEPS its credential/custom headers — the +// cross-origin strip must not OVER-strip on a hop that never leaves the origin. Proven end-to-end +// through the download surface, over a real socket. +// +// The companion to download-redirect-cross-origin.spec.ts. When an API answers with a redirect to a +// path on the SAME origin (same scheme + host + port), auth has to keep working: `headersForRedirect` +// returns the headers unchanged for a same-origin hop, and `download()` inherits that via +// fetchAdapter's manual-follow loop. Here A 302s to another path on A itself; the second (same-origin) +// request must still carry `x-api-key` and the SigV4-style headers, and the download must resolve +// with the file. A single server ⇒ one origin ⇒ same-origin by construction. +// +// Real-timer, loose bounds (a socket test): the hop is local and prompt. The server is closed in +// afterAll. +import { apiKey, env } from '../../src'; +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +const API_KEY = 'sk-secret-same-origin'; +const AWS_AUTH = 'AWS4-HMAC-SHA256 Credential=AKIA.../...'; + +const hdr = (h: Record, name: string): string | undefined => { + const k = Object.keys(h).find( + (x) => x.toLowerCase() === name.toLowerCase(), + ); + return k ? h[k] : undefined; +}; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const FILE_BYTES = 'SAME-ORIGIN-REDIRECT-TARGET-BODY'; + +test('a same-origin redirect KEEPS x-api-key / authorization / x-amz-* on the second request and resolves the file (no over-strip)', async () => { + // A 302s to another PATH on the SAME server (a relative Location → same scheme+host+port). + server.route('GET', '/entry', { + statuses: [302], + redirectTo: '/final', + }); + server.route('GET', '/final', { + statuses: [200], + rawBody: FILE_BYTES, + headers: { 'content-disposition': 'attachment; filename="kept.bin"' }, + }); + + process.env['SAMEORIGIN_REDIRECT_KEY'] = API_KEY; + const getFile = download({ + baseUrl: server.url, + path: '/entry', + auth: apiKey({ value: env('SAMEORIGIN_REDIRECT_KEY') }), + headers: { + authorization: AWS_AUTH, + 'x-amz-date': '20260706T000000Z', + 'x-amz-content-sha256': 'abc123def456', + }, + timeout: 5000, + retry: { attempts: 1 }, + }); + + const out = await getFile(); + + // The second (same-origin) request STILL carries every sensitive header — auth keeps working. + const second = server.calls('/final')[0]; + expect(second).toBeDefined(); + const kept = second?.headers ?? {}; + expect(hdr(kept, 'x-api-key')).toBe(API_KEY); + expect(hdr(kept, 'authorization')).toBe(AWS_AUTH); + expect(hdr(kept, 'x-amz-date')).toBe('20260706T000000Z'); + expect(hdr(kept, 'x-amz-content-sha256')).toBe('abc123def456'); + + // The download resolves with the redirect target's body + filename. + expect(new TextDecoder().decode(await out.blob.arrayBuffer())).toBe( + FILE_BYTES, + ); + expect(out.filename).toBe('kept.bin'); +}); diff --git a/packages/core/test/gaps/download-reset-midbody.spec.ts b/packages/core/test/gaps/download-reset-midbody.spec.ts new file mode 100644 index 00000000..7651352d --- /dev/null +++ b/packages/core/test/gaps/download-reset-midbody.spec.ts @@ -0,0 +1,72 @@ +// Pins: a download whose socket is RST mid-body must REJECT — never resolve a partial Blob. +// +// The abrupt-close sibling of download-truncation.spec.ts's clean FIN. Where a short read + clean +// `end()` makes undici HANG (cured only by a timeout), an abrupt socket close mid-body makes undici +// REJECT at the transport (`UND_ERR_SOCKET`, "other side closed"), and the engine's try/catch +// (engine.ts:687) turns that into a rejected call. So there is nothing for the surface's `interpret` +// to accept — a partial body is never handed back as a complete Blob. No `src/` change is involved; +// this pins that the existing transport + engine path already fails closed on a real ECONNRESET. +// +// EMPIRICAL FINDING (Node 24 undici, measured for this rig): at the transport boundary the RST is a +// TypeError: "fetch failed" +// └─ cause: SocketError { code: 'UND_ERR_SOCKET', message: 'other side closed' } +// but the engine reduces a transport failure to its top-level MESSAGE on the error event and rebuilds +// a `StitchError('fetch failed', …)` WITHOUT the `.cause` chain — so the caller (both the throwing +// AND the `.safe()` path) sees `StitchError: fetch failed` with `err.cause === undefined`. The +// `UND_ERR_SOCKET` detail is therefore NOT visible to callers today; only the generic "fetch failed" +// survives. (A candidate future improvement: carry the transport `cause` through the error event so +// callers can distinguish a socket reset from other transport failures.) This spec asserts the shape +// that actually surfaces — a reject whose message is undici's transport-failure text — not the +// stripped socket cause. `retry: { attempts: 1 }` makes the first transport failure terminal, so the +// assertion is a clean single-shot reject (no backoff to wait on). +// +// Real-timer, loose bounds (a socket test): no timeout is needed — undici rejects promptly on the +// RST — but we keep `retry: { attempts: 1 }` so the reset is not papered over by a retry. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +// 64 bytes advertised via Content-Length, but only the first 8 are written before the socket is +// destroyed (a TCP RST) — a real ECONNRESET mid-body. undici rejects; the call must too. +test('a socket RST mid-body rejects the download — never a partial Blob', async () => { + server.route('GET', '/reset', { + statuses: [200], + rawBody: + 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ012', // 64 bytes + declaredLength: 64, // advertise 64… + resetAfterBytes: 8, // …but write only 8, then RST the socket + }); + + const getReset = download({ + baseUrl: server.url, + path: '/reset', + retry: { attempts: 1 }, // the reset is terminal, not a transient to retry + }); + + // The call must reject — a partial body is never handed back as a complete Blob. The + // engine-surfaced message is undici's transport-failure text ("fetch failed"); the underlying + // UND_ERR_SOCKET/"other side closed" cause is stripped by the engine (see the header note), so we + // pin the message that actually reaches the caller. It is NOT a surface-level ("download: …") + // reject — the failure happens at the transport, before `interpret` ever runs. + const err = await getReset().then( + () => { + throw new Error('reset-mid-body download unexpectedly resolved'); + }, + (e: unknown) => e, + ); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/fetch failed/i); + // Guard that it was a transport reject, not the stray-206 / interpret-level path. + expect((err as Error).message).not.toMatch(/^download:/); +}); diff --git a/packages/core/test/gaps/download-slow-ttfb.spec.ts b/packages/core/test/gaps/download-slow-ttfb.spec.ts new file mode 100644 index 00000000..6e5342b9 --- /dev/null +++ b/packages/core/test/gaps/download-slow-ttfb.spec.ts @@ -0,0 +1,75 @@ +// Pins: a slow TIME-TO-FIRST-BYTE (the server accepts the request, then delays the status line + +// headers) rides the SAME engine `timeout` as everything else — the per-attempt timeout wraps the +// whole transport, TTFB included (engine.ts:681). So: +// (a) TTFB > timeout → the call REJECTS (the timeout fires before any response arrives); +// (b) TTFB < timeout → the call RESOLVES (a slow start is tolerated, not a fault). +// There is no separate "connect"/"first-byte" timeout — `TimeoutOptions` is `{ total, perAttempt }`, +// both wall-clock. This pins that a slow start is only fatal once it eats the wall-clock budget, and +// needs no `src/` change. +// +// EMPIRICAL FINDING (Node 24 undici): case (a) rejects with the engine's timeout/abort error +// (asserted loosely below); case (b) resolves with the full body once the delayed headers arrive. +// +// Real-timer, LOOSE bounds (a socket test): delays and timeouts are milliseconds apart but chosen +// with wide margins so scheduler jitter can't flip the outcome. `ttfbDelayMs` holds the socket open +// during the wait; teardown force-destroys any lingering socket (mock-server tracks live sockets), +// so the suite exits. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +// (a) A TTFB delay LONGER than the timeout: the timeout fires before any byte arrives → reject. +test('a slow TTFB longer than the timeout rejects (TTFB rides the same timeout)', async () => { + server.route('GET', '/slow-headers', { + statuses: [200], + rawBody: 'a complete body that the client never gets to see', + ttfbDelayMs: 400, // headers held for 400ms… + }); + + const getSlow = download({ + baseUrl: server.url, + path: '/slow-headers', + timeout: 150, // …but the budget is only 150ms → the timeout wins + retry: { attempts: 1 }, + }); + + const started = Date.now(); + await expect(getSlow()).rejects.toThrow(/timeout|timed out|abort|aborted/i); + expect(Date.now() - started).toBeLessThan(5000); // never hangs +}); + +// (b) A TTFB delay SHORTER than the timeout: the (slightly late) headers + body arrive in budget → +// resolve. Proves a slow start is tolerated, not treated as a fault. +test('a slow TTFB shorter than the timeout resolves (a slow start is tolerated)', async () => { + const payload = 'the-late-but-complete-body'; + server.route('GET', '/slow-ok', { + statuses: [200], + rawBody: payload, + ttfbDelayMs: 120, // a 120ms slow start… + headers: { 'content-disposition': 'attachment; filename=late.bin' }, + }); + + const getSlowOk = download({ + baseUrl: server.url, + path: '/slow-ok', + timeout: 2000, // …well within a 2s budget → resolves + retry: { attempts: 1 }, + }); + + const out = await getSlowOk(); + expect(out.filename).toBe('late.bin'); + expect(new TextDecoder().decode(await out.blob.arrayBuffer())).toBe( + payload, + ); +}); diff --git a/packages/core/test/gaps/download-slow-vs-stall.spec.ts b/packages/core/test/gaps/download-slow-vs-stall.spec.ts new file mode 100644 index 00000000..4e0c77bf --- /dev/null +++ b/packages/core/test/gaps/download-slow-vs-stall.spec.ts @@ -0,0 +1,92 @@ +// Pins (THE FINDING): a HEALTHY-but-slow download — one whose bytes keep arriving, just slowly — is +// killed by the SAME wall-clock `timeout` as a dead stall. The engine's `TimeoutOptions` is +// `{ total, perAttempt }`, BOTH wall-clock (types.ts:317); there is NO idle / forward-progress / +// no-bytes-for-N-ms timeout. So once total elapsed exceeds the budget, the call rejects even though +// the transfer was making steady forward progress the whole time — indistinguishable, to the engine, +// from a socket that stalled at byte 0. +// +// This test proves the body was HEALTHY (progressing), not stalled, by threading an `onProgress` +// callback: it records a strictly-increasing `loaded` across several chunks BEFORE the timeout +// reject. A stall (download-stall.spec.ts) would show `loaded` frozen; here it climbs, then the call +// still fails — that is the finding. +// +// DESIGN FINDING, NOT A BUG TO FIX NOW: this motivates an IDLE / forward-progress timeout — one that +// resets its countdown on each `onProgress` chunk, so a slow-but-progressing transfer is allowed to +// continue while only a genuine no-bytes stall trips it. The engine lacks that today (only wall-clock +// total/perAttempt). A future `@stitchapi/download` should add an idle-timeout option layered on the +// `onProgress` byte stream. No `src/` change is made here — this pins present behavior and documents +// the gap. +// +// Real-timer, LOOSE bounds (a socket test): chunk cadence (120ms × 8 = ~960ms of streaming) is set +// well above the 400ms budget so the timeout reliably fires mid-stream after a few progress ticks, +// with wide margins against scheduler jitter. `chunkDelayMs` streams a real chunked body; teardown +// force-destroys any lingering socket (mock-server tracks live sockets), so the suite exits. +import { download } from '../../src/download'; +import type { AdapterProgress } from '../../src/types'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +// 64 bytes streamed as 8 chunks of 8 bytes, 120ms apart (~960ms total) — a steady trickle. A 400ms +// budget cuts it after ~3 chunks. The body is HEALTHY (bytes flowing), yet the wall-clock timeout +// kills it anyway; `onProgress` proves the progress was real before the reject. +test('a healthy-but-slow (steadily progressing) download is killed by the same wall-clock timeout as a stall', async () => { + server.route('GET', '/trickle', { + statuses: [200], + rawBody: + 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ012', // 64 bytes + chunkBytes: 8, // 8 bytes per chunk… + chunkDelayMs: 120, // …every 120ms → ~960ms to stream the whole body + }); + + const seen: AdapterProgress[] = []; + const getTrickle = download({ + baseUrl: server.url, + path: '/trickle', + timeout: 400, // ≡ { total: 400 } — wall-clock; no idle/progress timeout exists + retry: { attempts: 1 }, + }); + + const started = Date.now(); + const err = await getTrickle({ + onProgress: (p) => { + seen.push(p); + }, + }).then( + () => { + throw new Error( + 'slow-but-progressing download unexpectedly resolved', + ); + }, + (e: unknown) => e, + ); + const elapsed = Date.now() - started; + + // It REJECTED (the wall-clock budget fired), promptly, and cut by the timeout — not resolved. + expect(err).toBeInstanceOf(Error); + expect(elapsed).toBeLessThan(5000); + expect((err as Error).message).toMatch(/timeout|timed out|abort|aborted/i); + + // …and it was HEALTHY, not stalled: progress fired and `loaded` climbed across chunks BEFORE the + // reject. This is the evidence that a *progressing* transfer — not a dead one — was killed by the + // wall-clock timeout, which is exactly the finding an idle/forward-progress timeout would fix. + expect(seen.length).toBeGreaterThan(1); + expect(seen.every((p) => p.phase === 'download')).toBe(true); + const loaded = seen.map((p) => p.loaded); + const lastLoaded = loaded[loaded.length - 1] ?? 0; + const firstLoaded = loaded[0] ?? 0; + expect(lastLoaded).toBeGreaterThan(firstLoaded); // strictly increasing → real forward progress + // Non-decreasing throughout (a chunked read only ever adds bytes). + for (let i = 1; i < loaded.length; i++) + expect(loaded[i]!).toBeGreaterThanOrEqual(loaded[i - 1]!); +}); diff --git a/packages/core/test/gaps/download-stall.spec.ts b/packages/core/test/gaps/download-stall.spec.ts new file mode 100644 index 00000000..050e1b04 --- /dev/null +++ b/packages/core/test/gaps/download-stall.spec.ts @@ -0,0 +1,67 @@ +// Pins: a download that STALLS mid-body (bytes stop arriving, socket held open) must REJECT with a +// timeout — never hang, never resolve a partial Blob. +// +// The stall is the worst case of M1's clean-FIN truncation: the server writes some bytes and then +// simply stops, holding the connection open with no FIN and no reset. undici blocks forever waiting +// for the rest of the promised body, so the buffered `download` never resolves. The ONLY thing that +// cuts it is the engine's per-attempt/total `timeout` (TimeoutOptions, both wall-clock — there is no +// idle/forward-progress timeout), which aborts the in-flight fetch (engine.ts:681 withTimeout links +// the abort signal to the transport). No `src/` change: this pins that a bounded `timeout` turns an +// idle stall into a deterministic reject. +// +// EMPIRICAL FINDING (Node 24 undici): the reject is the engine's timeout/abort error — reported in +// the loose message assertion below. +// +// Real-timer, LOOSE bounds (a socket test): a short `timeout` cuts the hang; the assertion only +// requires the call to reject comfortably before a generous real-time ceiling, so it can't itself +// hang the suite. `stallAfterBytes` holds a socket open — teardown force-destroys it (mock-server +// tracks live sockets and destroys them on reset()/close()), so the suite still exits. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +// 64 bytes advertised, 8 written, then the connection is held open with no further bytes — an idle +// stall. A 300ms total timeout must cut it; the call rejects well under a 5s real-time ceiling and +// never resolves a partial Blob. +test('an idle stall mid-body rejects via timeout — never hangs, never a partial Blob', async () => { + server.route('GET', '/stall', { + statuses: [200], + rawBody: + 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ012', // 64 bytes + declaredLength: 64, // advertise 64… + stallAfterBytes: 8, // …write 8, then hold the socket open forever + }); + + const getStall = download({ + baseUrl: server.url, + path: '/stall', + timeout: 300, // ≡ { total: 300 } — the only cure for an idle stall + retry: { attempts: 1 }, + }); + + const started = Date.now(); + const err = await getStall().then( + () => { + throw new Error('download resolved despite a mid-body stall'); + }, + (e: unknown) => e, + ); + const elapsed = Date.now() - started; + + // Rejected (not resolved), and cut by the timeout — comfortably under a loose real-time ceiling. + expect(err).toBeInstanceOf(Error); + expect(elapsed).toBeLessThan(5000); + // The reject is the engine's timeout/abort — assert loosely so it survives wording drift. + expect((err as Error).message).toMatch(/timeout|timed out|abort|aborted/i); +}); diff --git a/packages/core/test/gaps/download-stray-206.spec.ts b/packages/core/test/gaps/download-stray-206.spec.ts new file mode 100644 index 00000000..c3ecdad6 --- /dev/null +++ b/packages/core/test/gaps/download-stray-206.spec.ts @@ -0,0 +1,45 @@ +// Pins: the `download` surface must REJECT a `206 Partial Content` it never asked for. +// +// `download` NEVER sends a `Range` header, yet a `206` is `< 400`, so the engine's attempt loop +// (engine.ts:775 throws only on `>= 400`) lets it through to the surface's `interpret`. Pre-fix, +// `interpret` returned `{ ok: true }` unconditionally (download.ts) — so a stray 206 was accepted as +// a COMPLETE Blob, silently handing the caller a partial body as if it were the whole file. The fix +// makes `interpret` reject any non-`200`/`204` status. This test drives the real fetch adapter over a +// mock server that answers a range-less GET with a 206 body. +// +// Pre-fix behavior (before the interpret fix): the call RESOLVED and `out.blob` held the partial +// bytes. This committed test asserts the FIXED behavior — the call rejects with a 206 message. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +test('a stray 206 (no Range was sent) is rejected, not accepted as a complete Blob', async () => { + server.route('GET', '/partial', { + statuses: [206], + rawBody: 'PARTIAL', // a real body under a 206 status, to a request carrying no Range + headers: { + // A 206 in the wild carries a Content-Range; include one so the fixture is realistic. + 'content-range': 'bytes 0-6/1024', + }, + }); + + const getPartial = download({ baseUrl: server.url, path: '/partial' }); + + // The surface must treat the unsolicited partial as a failure — the promise rejects with a + // 206-flavored message. (Pre-fix this RESOLVED with `out.blob` holding the 7 partial bytes.) + await expect(getPartial()).rejects.toThrow(/206|partial/i); + + // And it really did answer 206 to a range-less request (guards the fixture itself). + expect(server.calls('/partial')[0]?.headers['range']).toBeUndefined(); +}); diff --git a/packages/core/test/gaps/download-truncation.spec.ts b/packages/core/test/gaps/download-truncation.spec.ts new file mode 100644 index 00000000..31d6ae83 --- /dev/null +++ b/packages/core/test/gaps/download-truncation.spec.ts @@ -0,0 +1,58 @@ +// Pins: a truncated download — `200 OK` + `Content-Length: N` but a body of fewer than N bytes — +// must NEVER resolve as a complete Blob. It has to fail instead. +// +// EMPIRICAL FINDING (Node 20/22/24 undici, measured for this rig): +// * CL-mismatch + a CLEAN FIN (server writes 8 of a promised 64 bytes, then `res.end()`): +// undici does NOT reject — it HANGS waiting for the missing bytes until the caller's +// timeout/abort fires. The body never finishes buffering, so the surface's `interpret` is +// never even reached. The real cure for this mode is a `timeout` (engine-level), not a +// length cross-check in `interpret`. +// * CL-mismatch + an ABRUPT socket close (RST): undici DOES reject (`UND_ERR_SOCKET`, +// "other side closed"); the engine's try/catch turns that into a rejected call already. +// Because undici never hands a SHORT Blob to `interpret` while a numeric `content-length` +// promised more (it hangs or it rejects at the transport), an interpret-level +// `blob.size !== content-length` check would be dead code — so `download.ts` deliberately does +// NOT add one (see the comment there). This test pins the clean-FIN mode: under a bounded +// `timeout`, the truncated download FAILS rather than silently resolving with a partial body. +// +// Real-timer, loose bounds (a socket test): the timeout is short but the assertion only cares that +// the call rejects well before the promised body could ever arrive. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +// 64 bytes of body advertised via Content-Length, but only the first 8 are written before a clean +// end — a real short read over the socket. undici blocks waiting for the other 56; a bounded +// per-attempt timeout must cut it and fail the call (never resolve with the 8 partial bytes). +test('a Content-Length-mismatch truncation (clean FIN) fails under a bounded timeout — never a partial Blob', async () => { + server.route('GET', '/truncated', { + statuses: [200], + rawBody: + 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ012', // 64 bytes + declaredLength: 64, // advertise 64… + truncateAfterBytes: 8, // …but write only 8, then a clean FIN + }); + + // A short total timeout so the undici hang is cut deterministically; no retry (a truncation is + // not a transient the surface should paper over here). + const getTruncated = download({ + baseUrl: server.url, + path: '/truncated', + timeout: 500, + retry: { attempts: 1 }, + }); + + // The call must reject — a partial body is never handed back as a complete Blob. + await expect(getTruncated()).rejects.toThrow(); +}); diff --git a/packages/core/test/support/mock-server.ts b/packages/core/test/support/mock-server.ts index 24558011..864d358f 100644 --- a/packages/core/test/support/mock-server.ts +++ b/packages/core/test/support/mock-server.ts @@ -1,5 +1,6 @@ import { createServer } from 'node:http'; import type { IncomingMessage, Server, ServerResponse } from 'node:http'; +import type { Socket } from 'node:net'; export interface ReqInfo { method: string; @@ -28,6 +29,91 @@ export interface RouteBehavior { * `'text/event-stream'`); the loop stops as soon as the client goes away. */ stream?: { chunks: (string | Uint8Array)[]; chunkDelayMs?: number }; + /** + * Send this body verbatim over the socket — raw bytes (Buffer/Uint8Array) or a string encoded + * utf8 — bypassing the JSON/`{}` default of `body`. Pairs with `declaredLength` / + * `truncateAfterBytes` for the buffered-download fault cases (M1 download rig): together they let + * a route lie about `Content-Length` or short-write the socket. Content-type still defaults to + * `application/octet-stream` unless `headers['content-type']` overrides it. + */ + rawBody?: string | Uint8Array; + /** + * Advertise this exact `Content-Length` (in bytes) regardless of how many bytes are actually + * written — so a route can claim a body larger than it sends (a truncation fault). Omitted ⇒ + * the transport frames the response itself (chunked / the real length). Only honoured on the + * `rawBody` path. + */ + declaredLength?: number; + /** + * Write only the first N bytes of `rawBody` and then cleanly `end()` the response — a real short + * read over the socket (a clean FIN after fewer bytes than promised). Combine with a larger + * `declaredLength` to reproduce "200 + Content-Length: N, body < N". Only honoured on the + * `rawBody` path. + */ + truncateAfterBytes?: number; + /** + * Server-side Range scaffolding (M1: a target for a FUTURE resume feature — no client code uses + * it yet). When `true` AND the request carries a `Range: bytes=START-[END]` header, the route + * answers `206 Partial Content` with the requested slice of `rawBody`/`body` and a correct + * `Content-Range: bytes START-END/TOTAL`. A request with NO `Range` header is served normally + * (a full `200`), so the same route reproduces the "stray 206" gap only when the client actually + * asked for a range. Ignored unless a byte body is available. + */ + serveRange?: boolean; + /** + * Write the first N bytes of `rawBody` and then **destroy the socket** — a real `ECONNRESET` + * mid-body (M2 network-fault rig). The abrupt-close sibling of `truncateAfterBytes`: where that + * one `end()`s cleanly (a FIN, which undici HANGS on), this one RSTs the connection (which undici + * REJECTS with `UND_ERR_SOCKET`). The `Content-Length` still advertises the whole body + * (`declaredLength ?? rawBody.length`), so the client is mid-buffer when the reset lands. Only + * honoured on the `rawBody` path; wins over `truncateAfterBytes` if both are set. + */ + resetAfterBytes?: number; + /** + * Write the first N bytes of `rawBody` and then **hold the connection open with no further + * bytes** — an idle stall (M2). The body never finishes, so a buffered `download` blocks; only + * the caller's `timeout` (engine-level) can cut it. The held socket is force-destroyed on BOTH + * `reset()` and `close()` (the server tracks live sockets), so a stalled test can't wedge + * teardown. `Content-Length` advertises the whole body. Only honoured on the `rawBody` path. + */ + stallAfterBytes?: number; + /** + * Delay the status line + headers by N ms **after** the request is fully received, before any + * response is written — a slow time-to-first-byte (M2). Distinct from `delayMs` (which is the + * JSON-`body` path's pre-send pause): `ttfbDelayMs` is honoured on the `rawBody` path so a + * download can pin that a slow TTFB rides the same `timeout` as everything else. The socket is + * held open during the wait and is force-destroyed on `reset()`/`close()`. + */ + ttfbDelayMs?: number; + /** + * Stream `rawBody` to the client in fixed-size slices of `chunkBytes` (default: the whole body in + * one chunk), pausing `chunkDelayMs` **before each** slice — a steady bandwidth-throttle profile + * (M2). Unlike `stallAfterBytes`, bytes keep flowing the whole time, so `onProgress` records a + * rising `loaded`: this is the "healthy-but-slow" body the slow-vs-stall finding needs. The + * response is a real chunked send (no `Content-Length`), so download progress reports `loaded` + * with no `total`. Only honoured on the `rawBody` path; ignored if `chunkDelayMs` is unset (a + * plain framed send covers the no-throttle case). The socket is tracked for teardown. + */ + chunkBytes?: number; + /** + * Pause before each write, in ms (M2). On the `rawBody` chunked path (`chunkBytes`) it throttles + * the body so bytes trickle out steadily; it is the top-level twin of `stream.chunkDelayMs` (the + * SSE/stream path keeps its own nested field). Only meaningful on the `rawBody` chunked path. + */ + chunkDelayMs?: number; + /** + * Answer with an HTTP redirect to `redirectTo` (M3 redirect-fault rig). The response is the + * route's status (drawn from the `statuses` array so 301/302/303/307/308 can be scripted per + * call; **defaults to 302** when the resolved status isn't itself a 3xx redirect code) plus a + * `Location: ` header and a tiny body. `redirectTo` may be an **absolute** URL — e.g. + * `${otherServer.url}/file`, which is cross-origin because a second `startMockServer()` binds a + * different ephemeral port — or a **path** on the same server (same-origin). Wins over every + * other body path (`rawBody`/`stream`/`body`), so a redirecting route never also writes a + * payload. The request is still recorded (`calls()`/`callCount()` count the hop), and the + * auth/counter checks above still run, so a `[503, 302]`-style transient-then-redirect is + * expressible. + */ + redirectTo?: string; } export interface MockServer { @@ -89,6 +175,11 @@ export function startMockServer(): Promise { const routes = new Map(); const counters = new Map(); const log: ReqInfo[] = []; + // Every live TCP connection, tracked so a stalled/held socket (M2 `stallAfterBytes` / + // `ttfbDelayMs`) can be force-destroyed on BOTH `reset()` and `close()` — otherwise a socket the + // server is deliberately holding open would keep the event loop alive and hang test teardown. + // `server.closeAllConnections()` only fires on close; `reset()` (per-test) needs this explicit set. + const sockets = new Set(); const key = (method: string, path: string): string => `${method.toUpperCase()} ${path}`; @@ -153,6 +244,50 @@ export function startMockServer(): Promise { ); res.end(isBytes ? Buffer.from(payload) : JSON.stringify(payload)); }; + // Send a byte body verbatim, optionally lying about `Content-Length` (`declaredLength`) and/or + // short-writing the socket (`truncateAfterBytes`) — the buffered-download fault path. Unlike + // `send`, the length header is set explicitly (a lie survives) and only a prefix may be + // written before a clean `end()`. Swallows a client-abort reset so a half-read body can't + // crash the handler. + const sendRaw = ( + status: number, + bytes: Buffer, + extra: RouteBehavior, + ): void => { + res.on('error', () => { + /* client went away mid-write */ + }); + const out = buildHeaders('application/octet-stream', extra); + // A declared length wins even when it disagrees with the bytes on the wire (the truncation + // lie); otherwise advertise the real length so the response is a plain framed 200. + out['content-length'] = String( + extra.declaredLength ?? bytes.length, + ); + res.writeHead(status, out); + const cut = + extra.truncateAfterBytes !== undefined + ? bytes.subarray(0, extra.truncateAfterBytes) + : bytes; + res.end(cut); + }; + // Answer a `Range: bytes=START-[END]` request with `206 Partial Content` + the requested slice + // and a correct `Content-Range` (M1 server-side scaffolding only — no client resume yet). + // Returns true when it handled the request; false when there was no usable `Range` (the caller + // then serves the full body — which for a byte body is the "stray 206"-free 200 path). + const serveRangeIf = (full: Buffer, extra: RouteBehavior): boolean => { + const m = /^bytes=(\d+)-(\d*)$/.exec(headers['range'] ?? ''); + if (!m) return false; + const start = Number(m[1]); + const end = m[2] ? Number(m[2]) : full.length - 1; + if (start > end || start >= full.length) return false; + const slice = full.subarray(start, end + 1); + const out = buildHeaders('application/octet-stream', extra); + out['content-range'] = `bytes ${start}-${end}/${full.length}`; + out['content-length'] = String(slice.length); + res.writeHead(206, out); + res.end(slice); + return true; + }; // Write the body as a real chunked response: one `res.write` per chunk (an optional pause // before each), then `res.end`. Bails the moment the client disconnects (abort / early // break) so a half-read stream can't wedge the server's `close()`. @@ -184,6 +319,75 @@ export function startMockServer(): Promise { } if (!res.writableEnded && !res.destroyed) res.end(); }; + // ---- M2 network-fault sends (rawBody path) -------------------------------------------- + // Write the first N bytes of `bytes`, then RST the underlying socket → a real ECONNRESET + // mid-body (the abrupt-close sibling of `truncateAfterBytes`'s clean FIN). `Content-Length` + // still advertises the whole body, so the client is mid-buffer when the reset lands and undici + // rejects with `UND_ERR_SOCKET`. `res.socket.destroy()` sends the RST; `res.destroy()` alone + // can FIN cleanly, so we hit the socket directly. + const sendReset = ( + status: number, + bytes: Buffer, + extra: RouteBehavior, + ): void => { + res.on('error', () => { + /* socket torn down under us */ + }); + const out = buildHeaders('application/octet-stream', extra); + out['content-length'] = String( + extra.declaredLength ?? bytes.length, + ); + res.writeHead(status, out); + const n = extra.resetAfterBytes ?? 0; + res.write(bytes.subarray(0, n)); + // Abrupt close: destroy the raw socket to force a TCP RST rather than a graceful FIN. + res.socket?.destroy(); + }; + // Write the first N bytes, then HOLD the connection open forever (no `end`, no further + // bytes) — an idle stall. The body never completes, so a buffered download blocks until the + // caller's timeout fires. The socket is in the tracked `sockets` set, so `reset()`/`close()` + // destroy it — this handler intentionally never finishes the response. + const sendStall = ( + status: number, + bytes: Buffer, + extra: RouteBehavior, + ): void => { + res.on('error', () => { + /* torn down at teardown */ + }); + const out = buildHeaders('application/octet-stream', extra); + out['content-length'] = String( + extra.declaredLength ?? bytes.length, + ); + res.writeHead(status, out); + const n = extra.stallAfterBytes ?? 0; + if (n > 0) res.write(bytes.subarray(0, n)); + // Deliberately do NOT end: hold the socket open. Teardown destroys it. + }; + // Stream `bytes` in fixed `chunkBytes`-sized slices, pausing `chunkDelayMs` before each — a + // steady bandwidth throttle. Bytes keep flowing, so `onProgress` sees a rising `loaded`. Sent + // as a real chunked response (no Content-Length), matching how `readWithProgress` reports + // `loaded` without a `total`. Bails if the client disconnects so it can't wedge teardown. + const sendChunked = async ( + status: number, + bytes: Buffer, + extra: RouteBehavior, + ): Promise => { + res.on('error', () => { + /* client went away mid-write */ + }); + res.writeHead( + status, + buildHeaders('application/octet-stream', extra), + ); + const size = extra.chunkBytes ?? bytes.length; + for (let off = 0; off < bytes.length; off += size) { + if (extra.chunkDelayMs) await sleep(extra.chunkDelayMs); + if (res.writableEnded || res.destroyed) break; + res.write(bytes.subarray(off, off + size)); + } + if (!res.writableEnded && !res.destroyed) res.end(); + }; if (!behavior) { send(404, { error: 'not_found' }); @@ -218,12 +422,59 @@ export function startMockServer(): Promise { ? (at(behavior.statuses, idx) as number) : 200; + // A redirecting route (M3): emit `Location: ` + a tiny body and return. It fires + // when the resolved `status` is a 3xx redirect code (so a `statuses` array scripts + // 301/302/303/307/308 per call), OR when no `statuses` array was given at all (default 302 — + // a plain `redirectTo` with no status still redirects). When a `statuses` array yields a + // NON-3xx (e.g. a `[503, 302]` transient-then-redirect), the non-3xx call FALLS THROUGH to + // the normal body path below and answers that status verbatim — only the 3xx call redirects. + // A redirect wins over the body paths, so a redirecting route never also writes a payload. + // `redirectTo` may be absolute (cross-origin, a different ephemeral port) or a path (same-origin). + const isRedirectCode = status >= 300 && status < 400; + if ( + behavior.redirectTo !== undefined && + (isRedirectCode || !behavior.statuses) + ) { + const out = buildHeaders('text/plain', behavior); + out['location'] = behavior.redirectTo; + res.writeHead(isRedirectCode ? status : 302, out); + res.end('redirecting'); + return; + } + // A streaming route writes a real chunked response (auth/counter checks above still apply). if (behavior.stream) { await streamResponse(status, behavior); return; } + // A raw byte body: honours `serveRange` (206 when the client sent a `Range`), then — after + // an optional slow-TTFB pause — the M2 network faults (RST / stall / throttle) or the M1 + // truncation/`declaredLength` path, else a plain framed send. `body` (JSON) is ignored here — + // `rawBody` is the explicit byte channel these download-fault cases use. + if (behavior.rawBody !== undefined) { + const full = + typeof behavior.rawBody === 'string' + ? Buffer.from(behavior.rawBody, 'utf8') + : Buffer.from(behavior.rawBody); + // serveRange short-circuits before any TTFB delay (it's an M1 range case, not a fault). + if (behavior.serveRange && serveRangeIf(full, behavior)) return; + // Slow time-to-first-byte: the request is fully received; hold before writing the status + // line + headers. The socket is tracked, so teardown can cut a mid-wait hold. + if (behavior.ttfbDelayMs) await sleep(behavior.ttfbDelayMs); + if (res.writableEnded || res.destroyed) return; // torn down during the TTFB wait + // Exactly one fault path wins, in precedence order: abrupt reset, idle stall, steady + // throttle (chunked), then the M1 clean-FIN truncation / plain framed send. + if (behavior.resetAfterBytes !== undefined) + sendReset(status, full, behavior); + else if (behavior.stallAfterBytes !== undefined) + sendStall(status, full, behavior); + else if (behavior.chunkDelayMs !== undefined) + await sendChunked(status, full, behavior); + else sendRaw(status, full, behavior); + return; + } + let body: unknown = {}; if (typeof behavior.body === 'function') { body = (behavior.body as (i: number, r: ReqInfo) => unknown)( @@ -255,6 +506,11 @@ export function startMockServer(): Promise { res.end(JSON.stringify({ error: 'internal' })); }); }); + // Track live sockets for deterministic teardown of held/stalled connections (see `sockets`). + server.on('connection', (socket: Socket) => { + sockets.add(socket); + socket.on('close', () => sockets.delete(socket)); + }); const filter = (path?: string): ReqInfo[] => path ? log.filter((r) => r.path === path) : log.slice(); @@ -274,11 +530,17 @@ export function startMockServer(): Promise { routes.clear(); counters.clear(); log.length = 0; + // Destroy any socket a prior test left deliberately open (a `stallAfterBytes` + // hold, a mid-`ttfbDelayMs` wait) so it can't leak into the next test or keep the + // loop alive. Each destroy fires the socket's own `close` → removed from the set. + for (const socket of sockets) socket.destroy(); }, close: () => new Promise((res) => { - // Force-drop any still-open connection (a half-read stream from an early - // break) so close() can't hang waiting on it (Node ≥ 18.2). + // Force-drop any still-open connection (a held/stalled M2 socket, a half-read + // stream from an early break) so close() can't hang waiting on it. Belt-and- + // braces: destroy tracked sockets AND call closeAllConnections (Node ≥ 18.2). + for (const socket of sockets) socket.destroy(); server.closeAllConnections(); server.close(() => { res(); From ad91f737429a9de893e04b1ab7a4db64b9959fe7 Mon Sep 17 00:00:00 2001 From: rejifald <42000283+rejifald@users.noreply.github.com> Date: Mon, 6 Jul 2026 23:53:30 +0300 Subject: [PATCH 2/2] =?UTF-8?q?test(core):=20download=20test=20rig=20?= =?UTF-8?q?=E2=80=94=20batch/concurrency=20+=20connection/retry/encoding/s?= =?UTF-8?q?ize=20(M4+M5)=20(#448)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(core): download batch/concurrency rig (M4) + de-flake throttle-host-pooling Milestone 4 of the download test rig — pin the substrate the future @stitchapi/download batch downloader stands on: per-request concurrency, host-pooled budgets, partial-failure isolation, multi-host fairness, and stall isolation. All test-only; no src/ change. - MockServer concurrency probe: maxOpen/openNow/arrivals(path?) — the count of requests SIMULTANEOUSLY open on the wire (high-water mark), marked at handler entry and released on res 'close', so a test asserts ACTUAL wire overlap. - 5 download-concurrency-*.spec.ts, all asserting overlap COUNT (a hard limiter invariant), never a timing gap: ceiling — <= k requests open at once under a shared host throttle pool — pool:'host' shares one budget vs pool:'stitch' independent partial-failure — allSettled: 200 + 404 + RST each settle on their own multihost — two hosts = two budgets; a saturated host can't stall the other stall-isolation — one stalled item times out alone; siblings' bytes intact - Root-cause + fix the flaky throttle-host-pooling spec: it measured a server-arrival wall-clock gap (conflated the 500ms rate spacing with first-fetch cold-start / event-loop lag). Rewritten on mockAdapter + a shared manualClock, proving pooling by virtual-time gating — deterministic, zero wall-clock. Deferred to the unbuilt batch orchestrator (each noted in a // Deferred: comment): FIFO admission order, cancel one/all, aggregate progress/ETA, same-URL dedupe. Co-Authored-By: Claude Opus 4.8 * test(core): finish the download rig — connection faults, retry, encoding, size, range (M5) Completes the download test rig beyond M4: the connection-level faults, status/retry semantics, content-encoding, size boundaries, and Range scaffolding a downloader must survive. All test-only; no src/ change. Infra: - test/support/hostile-net.ts — raw net.Server escape hatch (unusedPort, startRawServer) for faults node:http normalizes away (ECONNREFUSED, immediate FIN, accept-then-silence, broken TLS handshake). - mock-server.ts — contentEncoding (gzip/br via node:zlib), etag/lastModified/acceptRanges validators, forceStatusOnRange (416 / 200-ignore-Range). Plus a fix: reset() now destroys ONLY sockets with an in-flight response, leaving idle keep-alive sockets alive — destroying them made undici reuse a dead connection on the next test's first request (the N12 stale-keep-alive "fetch failed"), a latent flake across every multi-test spec. Specs (real-timer, loose bounds; manualClock for the retry-after delta): - download-conn-{dns,refused,hangup,no-response,tls} — N7–N11: DNS surfaces generically with no host leak; ECONNREFUSED is prompt; a FIN / no-response is cut by the timeout; a TLS handshake failure rejects cleanly. - download-retry-{transient,after,policy} — X9–X11: a 503 is retried and the WHOLE file re-downloaded (no Range — buffered surface has no resume); a 429 Retry-After waits the exact delta (manualClock); 500 and 408 are NOT retried by default, opt-in via retry.on restores it. - download-{content-encoding,size-boundaries,html-error-page,range-416} — B7/B8 gzip+br "progress lies" (loaded > compressed total, body intact); Z1–Z3 + 204 boundaries; Z4 a 200 HTML error page returns as a Blob (caller must check); R4 a 416 is rejected. Deferred (noted per spec) to the future @stitchapi/download package / resume feature: client-side Range/If-Range resume, plus the M4 batch-orchestrator properties. Verified: check:types 0, full core suite 1248 green, download+throttle 5x green (28 files / 35 tests), build+size unchanged (24.58/19.80 — subpath-only), eslint+prettier clean. Co-Authored-By: Claude Opus 4.8 * fix(core): carry the transport cause through to the error the caller sees (#450) When a transport call fails (undici `TypeError: fetch failed` whose `.cause` is a `SocketError { code: 'UND_ERR_SOCKET' }`, a DNS/connect failure, an AbortError, …), the engine reduced it to its top-level MESSAGE on the error event and rebuilt a `StitchError('fetch failed', …)` with NO `.cause` — so a caller (both the awaited and `.safe()` paths) saw only the generic message and could not tell a socket reset from any other transport failure (`err.cause === undefined`). Surfaced by the download rig's download-reset-midbody spec. Fix, entirely on the existing non-enumerable ERROR_SOURCE channel: - engine.ts `errEvt`: pin the live error for a bare transport/internal error too (not just RateLimitError / HTTP `.response` failures). A thrown `StitchError` stays excluded, so it keeps being rebuilt from the event (unchanged behaviour). - stitch.ts `rebuildError`: re-surface RateLimitError / contract-violation StitchError UNCHANGED (as before), and otherwise build a StitchError carrying the pinned transport error as `cause`. `cause` is non-enumerable, so it never leaks into a trace sink — only the enumerable `status`/`message` do. This is what lets @stitchapi/download's classifier read `err.cause`/`.code` to distinguish an RST from a generic "fetch failed". download-reset-midbody.spec.ts turns from pinning the gap to pinning the fix: it now asserts `err.cause` is defined and `UND_ERR_SOCKET` is reachable through the cause chain, on both the throwing and `.safe()` paths. Verified: core typecheck 0; full core suite 1248/1248 green; bundle 24.61/19.84 within the 24.80/20.00 budget. Co-authored-by: Claude Opus 4.8 --------- Co-authored-by: Claude Opus 4.8 --- packages/core/src/engine.ts | 11 ++ packages/core/src/stitch.ts | 21 ++- .../gaps/download-concurrency-ceiling.spec.ts | 86 +++++++++ .../download-concurrency-multihost.spec.ts | 106 +++++++++++ ...wnload-concurrency-partial-failure.spec.ts | 108 +++++++++++ .../gaps/download-concurrency-pool.spec.ts | 74 ++++++++ ...wnload-concurrency-stall-isolation.spec.ts | 100 +++++++++++ .../core/test/gaps/download-conn-dns.spec.ts | 37 ++++ .../test/gaps/download-conn-hangup.spec.ts | 36 ++++ .../gaps/download-conn-no-response.spec.ts | 42 +++++ .../test/gaps/download-conn-refused.spec.ts | 34 ++++ .../core/test/gaps/download-conn-tls.spec.ts | 41 +++++ .../gaps/download-content-encoding.spec.ts | 56 ++++++ .../gaps/download-html-error-page.spec.ts | 40 +++++ .../core/test/gaps/download-range-416.spec.ts | 56 ++++++ .../test/gaps/download-reset-midbody.spec.ts | 44 +++-- .../test/gaps/download-retry-after.spec.ts | 79 ++++++++ .../test/gaps/download-retry-policy.spec.ts | 83 +++++++++ .../gaps/download-retry-transient.spec.ts | 49 +++++ .../gaps/download-size-boundaries.spec.ts | 56 ++++++ .../test/gaps/throttle-host-pooling.spec.ts | 104 +++++------ packages/core/test/support/hostile-net.ts | 76 ++++++++ packages/core/test/support/mock-server.ts | 168 +++++++++++++++++- 23 files changed, 1425 insertions(+), 82 deletions(-) create mode 100644 packages/core/test/gaps/download-concurrency-ceiling.spec.ts create mode 100644 packages/core/test/gaps/download-concurrency-multihost.spec.ts create mode 100644 packages/core/test/gaps/download-concurrency-partial-failure.spec.ts create mode 100644 packages/core/test/gaps/download-concurrency-pool.spec.ts create mode 100644 packages/core/test/gaps/download-concurrency-stall-isolation.spec.ts create mode 100644 packages/core/test/gaps/download-conn-dns.spec.ts create mode 100644 packages/core/test/gaps/download-conn-hangup.spec.ts create mode 100644 packages/core/test/gaps/download-conn-no-response.spec.ts create mode 100644 packages/core/test/gaps/download-conn-refused.spec.ts create mode 100644 packages/core/test/gaps/download-conn-tls.spec.ts create mode 100644 packages/core/test/gaps/download-content-encoding.spec.ts create mode 100644 packages/core/test/gaps/download-html-error-page.spec.ts create mode 100644 packages/core/test/gaps/download-range-416.spec.ts create mode 100644 packages/core/test/gaps/download-retry-after.spec.ts create mode 100644 packages/core/test/gaps/download-retry-policy.spec.ts create mode 100644 packages/core/test/gaps/download-retry-transient.spec.ts create mode 100644 packages/core/test/gaps/download-size-boundaries.spec.ts create mode 100644 packages/core/test/support/hostile-net.ts diff --git a/packages/core/src/engine.ts b/packages/core/src/engine.ts index d654a5a9..3f25a46a 100644 --- a/packages/core/src/engine.ts +++ b/packages/core/src/engine.ts @@ -368,6 +368,17 @@ function errEvt(err: unknown, name: string, attempts: number): StitchEvent { value: err, enumerable: false, }); + } else if (err instanceof Error && !(err instanceof StitchError)) { + // Bare transport / internal error (undici UND_ERR_SOCKET / ECONNRESET / DNS, an AbortError, …): + // pin the live instance on the SAME non-enumerable channel so the awaited / `.safe()` path can + // carry its `.cause` through as `StitchError.cause` — letting a caller tell a socket reset from + // a generic "fetch failed" (previously only the top-level message survived). Non-enumerable ⇒ + // it never serialises into a trace sink (only the enumerable `status`/`message` do). A thrown + // `StitchError` is excluded so it keeps being rebuilt from the event (unchanged behaviour). + Object.defineProperty(evt, ERROR_SOURCE, { + value: err, + enumerable: false, + }); } return evt; } diff --git a/packages/core/src/stitch.ts b/packages/core/src/stitch.ts index 672677c1..fd81a2db 100644 --- a/packages/core/src/stitch.ts +++ b/packages/core/src/stitch.ts @@ -353,13 +353,20 @@ function rebuildError(ev: Extract): Error { cause: source, }); } - return ( - source ?? - new StitchError(ev.message, { - status: ev.status, - attempts: ev.attempts, - }) - ); + // A pass-through terminal — a delegate-backoff RateLimitError or a contract-violation StitchError + // (`.inspect()` retain path) — is re-surfaced UNCHANGED. + if (source instanceof StitchError || source instanceof RateLimitError) + return source; + // Otherwise a StitchError from the event, carrying any bare transport/internal `source` the engine + // pinned (an undici UND_ERR_SOCKET / ECONNRESET, a DNS failure, an AbortError, …) as `cause` — so + // callers can read `err.cause` (and its `.code`) to tell a socket reset from a generic "fetch + // failed". Absent a source, no cause (unchanged). `cause` is non-enumerable, so it never leaks into + // a trace sink. + return new StitchError(ev.message, { + status: ev.status, + attempts: ev.attempts, + ...(source !== undefined ? { cause: source } : {}), + }); } // Build the `Inspection` wrapper (ADR 0016 / ADR 0019): every field enumerable EXCEPT `raw`, which is diff --git a/packages/core/test/gaps/download-concurrency-ceiling.spec.ts b/packages/core/test/gaps/download-concurrency-ceiling.spec.ts new file mode 100644 index 00000000..4b96e6f1 --- /dev/null +++ b/packages/core/test/gaps/download-concurrency-ceiling.spec.ts @@ -0,0 +1,86 @@ +// Pins P1 (download-test-rig-spec §2.8): under a shared host throttle, no more than `k` download +// requests are OPEN ON THE WIRE at any instant. Asserted from the mock server's own concurrency probe +// (`maxOpen`) — ACTUAL on-the-wire overlap, not "we decided to be concurrent". This is the ceiling the +// future @stitchapi/download batch layer stands on: it sequences a caller's LIST, but the per-request +// admission cap is core `throttle`, and it must genuinely bound the wire. +// +// N INDEPENDENT download() stitches (a batch of distinct files) share ONE host-keyed concurrency +// budget via `throttle: { concurrency: k, pool: 'host' }` — P2, the cross-instance pooling substrate +// (resilience.ts `hostStates`, keyed by URL host). Each route holds its response with `ttfbDelayMs` so +// the k admitted slots stay open together long enough to observe the peak. +// +// Real-timer, LOOSE bounds (a socket test): the hold (150ms) is wide vs. loopback dispatch jitter +// (two near-simultaneous fetches land sub-ms apart), so the k concurrent slots reliably overlap; no +// exact timing is asserted, only a COUNT. Held sockets are force-destroyed at teardown (the server +// tracks live sockets), so the suite exits. +// +// Deferred (needs the unbuilt @stitchapi/download batch orchestrator, NOT raw download()+throttle): +// • FIFO admission ORDER — that queued items (k+1)…N start in enqueue order as slots free (P6). The +// limiter serves concurrency waiters FIFO, but PROVING per-item order needs the batch API's stable +// item identity + start events, which don't exist yet. +// • Aggregate progress / ETA across the N concurrent streams (P8) — a batch-level roll-up, not a +// property of a single download(). +// Here we pin only the wire CEILING + host pooling those features are built on. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +test('≤ k download requests are open on the wire at once under a shared host throttle', async () => { + const K = 2; + const N = 6; + const paths = Array.from({ length: N }, (_, i) => `/file-${i}`); + for (const p of paths) + server.route('GET', p, { + statuses: [200], + rawBody: `contents-of${p}`, + ttfbDelayMs: 150, // hold each admitted slot open long enough to observe the overlap + }); + + // N INDEPENDENT download stitches (distinct files), all sharing one host-keyed budget of K. + const calls = paths.map((p) => + download({ + baseUrl: server.url, + path: p, + throttle: { concurrency: K, pool: 'host' }, + retry: { attempts: 1 }, + }), + ); + + // `Promise.all` subscribes to every cold StitchResult in the same tick → all N are launched + // concurrently; the throttle, not the test, decides how many actually reach the wire. + const results = await Promise.all(calls.map((c) => c())); + + // THE CEILING: the server never saw more than K requests open on the wire at any instant. + expect(server.maxOpen()).toBeLessThanOrEqual(K); + // …and the budget was genuinely SATURATED (real concurrency, not accidental serialization) — else + // "≤ K" would pass vacuously at 1. With N > K and a 150ms hold, the peak must reach K. + expect(server.maxOpen()).toBe(K); + // Every file still downloaded correctly and completely — the cap PACES work, it never drops it. + expect(results).toHaveLength(N); + for (let i = 0; i < N; i++) + expect(await blobText(results[i]!.blob)).toBe( + `contents-of${paths[i]!}`, + ); + // Sanity: all N requests actually reached the server (nothing was silently dropped by the gate). + expect(server.callCount()).toBe(N); + // The probe also records a per-request arrival timestamp, in receipt order (the observable the + // future batch layer's ETA/progress math will read); one per request, monotonically non-decreasing. + const ts = server.arrivals(); + expect(ts).toHaveLength(N); + for (let i = 1; i < ts.length; i++) + expect(ts[i]!).toBeGreaterThanOrEqual(ts[i - 1]!); +}); diff --git a/packages/core/test/gaps/download-concurrency-multihost.spec.ts b/packages/core/test/gaps/download-concurrency-multihost.spec.ts new file mode 100644 index 00000000..a5711d33 --- /dev/null +++ b/packages/core/test/gaps/download-concurrency-multihost.spec.ts @@ -0,0 +1,106 @@ +// Pins P7 (download-test-rig-spec §2.8): multi-host fairness — a SATURATED host must not stall an +// INDEPENDENT host's schedule. `pool:'host'` keys the budget by URL host, so two mock servers on +// different ephemeral ports are two distinct hosts with two independent budgets (the same mechanism +// download-redirect-cross-origin.spec.ts uses for a cross-origin pair). A batch that spans hosts must +// let each host make progress at its own pace; host A being pinned at its cap can't starve host B. +// +// Setup: host A is deliberately throttled to concurrency 1 and given M slow items → it drains SERIALLY. +// Host B has concurrency 2 and a couple of items → it runs them concurrently. All are fired together. +// The oracles are COUNTS + ordering (robust), not a wall-clock gap: +// • B finishes while A is still draining → A.callCount() < M at the moment B is done (B was not +// stuck behind A's saturated queue). +// • B genuinely ran its own concurrency → B.maxOpen() reaches 2. +// • A really was capped at 1 → A.maxOpen() === 1. +// Real-timer, LOOSE bounds: A's serial drain (M×150ms) dwarfs B's (~150ms), a wide margin; held +// sockets are destroyed at teardown on BOTH servers. +// +// Deferred (batch-orchestrator scope): aggregate cross-host progress/ETA (P8) and priority/fair +// SCHEDULING across hosts (P21, which core `throttle` has no knob for) — this pins only that the two +// hosts' budgets are independent, the substrate any cross-host scheduler builds on. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let a: MockServer; // the saturated host (concurrency 1) +let b: MockServer; // the independent host that must not be stalled +beforeAll(async () => { + a = await startMockServer(); + b = await startMockServer(); +}); +afterAll(async () => { + await Promise.all([a.close(), b.close()]); +}); +beforeEach(() => { + a.reset(); + b.reset(); +}); + +const blobText = async (blob: Blob): Promise => + new TextDecoder().decode(await blob.arrayBuffer()); + +test('a saturated host does not stall an independent host’s schedule (per-host budgets)', async () => { + const M = 4; // host A items — drained one at a time (concurrency 1) + const aPaths = Array.from({ length: M }, (_, i) => `/a-${i}`); + const bPaths = ['/b-0', '/b-1']; + + // Distinct ephemeral ports ⇒ two real, independent hosts under pool:'host'. + expect(new URL(a.url).port).not.toBe(new URL(b.url).port); + + for (const p of aPaths) + a.route('GET', p, { + statuses: [200], + rawBody: `A${p}`, + ttfbDelayMs: 150, // each A item holds its single slot ~150ms → serial ~600ms total + }); + for (const p of bPaths) + b.route('GET', p, { + statuses: [200], + rawBody: `B${p}`, + ttfbDelayMs: 50, // B is fast — it finishes well before A drains, a wide margin + }); + + const aRuns = aPaths.map((p) => + download({ + baseUrl: a.url, + path: p, + throttle: { concurrency: 1, pool: 'host' }, // A: saturated, serial + retry: { attempts: 1 }, + })(), + ); + const bRuns = bPaths.map((p) => + download({ + baseUrl: b.url, + path: p, + throttle: { concurrency: 2, pool: 'host' }, // B: its own budget, unaffected by A + retry: { attempts: 1 }, + })(), + ); + + // StitchResults are COLD — they start on subscribe, not on creation. Launch A NOW (concurrently + // with B) by subscribing via `allSettled`, and keep the promise to await after we've measured — + // otherwise A would not even start until we awaited it, and the probe would read an idle host A. + const aDone = Promise.allSettled(aRuns); + + // B completes on its OWN schedule — it does not wait behind A's saturated queue. (Reaching here + // means neither B call rejected — `Promise.all` would have thrown otherwise.) + const bResults = await Promise.all(bRuns); + expect(bResults).toHaveLength(2); + expect(await blobText(bResults[0]!.blob)).toBe(`B${bPaths[0]!}`); + expect(await blobText(bResults[1]!.blob)).toBe(`B${bPaths[1]!}`); + + // At the instant B is done, A is STILL draining — proof B was never stalled behind A. + expect(a.callCount()).toBeLessThan(M); + // B genuinely ran its two downloads concurrently (its budget, not throttled down by A). + expect(b.maxOpen()).toBe(2); + // …and A really was pinned at its cap of 1 the whole time (the saturation was real). + expect(a.maxOpen()).toBe(1); + + // Let A finish so teardown is clean; every A item still downloads correctly once its turn comes. + const aSettled = await aDone; + expect(aSettled.map((s) => s.status)).toEqual(Array(M).fill('fulfilled')); + for (let i = 0; i < M; i++) { + const r = aSettled[i] as PromiseFulfilledResult<{ blob: Blob }>; + expect(await blobText(r.value.blob)).toBe(`A${aPaths[i]!}`); + } + expect(a.maxOpen()).toBe(1); // never exceeded its cap across the whole drain +}); diff --git a/packages/core/test/gaps/download-concurrency-partial-failure.spec.ts b/packages/core/test/gaps/download-concurrency-partial-failure.spec.ts new file mode 100644 index 00000000..34177bbf --- /dev/null +++ b/packages/core/test/gaps/download-concurrency-partial-failure.spec.ts @@ -0,0 +1,108 @@ +// Pins P4 (download-test-rig-spec §2.8): across a batch of INDEPENDENT downloads sharing a throttle, +// each call SETTLES ON ITS OWN — one bad URL (a 404, or a mid-body RST) never fails the others, and a +// successful item's bytes are complete and uncorrupted despite failing siblings sharing the budget. +// +// This is the isolation property the future @stitchapi/download batch API MUST preserve: a naive +// `Promise.all` over the calls would reject the WHOLE batch on the first failure; a batch downloader +// has to be per-item (the `Promise.allSettled` shape modelled here). We pin it at the raw +// download()+throttle layer so the batch API inherits an already-proven substrate — the two failure +// MODES compose with concurrency, not just in isolation. +// +// Deferred (an orchestrator CONTRACT, not a property of raw download()): same-URL DEDUPE (P18) — +// whether enqueuing the same URL twice reuses one in-flight fetch or runs two independent ones. +// StitchAPI's only coalescing is cache-gated (and download blobs are typically uncacheable), so a +// dedupe map is NEW batch-layer code; every call here is independent by construction. The batch API +// picks the contract (recommended default: independent fetches) and a future spec pins whichever. +import { download } from '../../src/download'; +import type { DownloadResult } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +// A mixed batch: good / 404 / good / RST / good — failures interleaved with successes, all sharing one +// host-keyed budget of 2. `ok` items hold briefly so the batch genuinely overlaps failures & successes +// under the gate (isolation must survive real concurrency, not just a serial run). +const ITEMS = [ + { path: '/ok-0', kind: 'ok' as const }, + { path: '/bad-404', kind: 'notfound' as const }, + { path: '/ok-1', kind: 'ok' as const }, + { path: '/bad-rst', kind: 'reset' as const }, + { path: '/ok-2', kind: 'ok' as const }, +]; + +test('a partial-failure batch settles per-item — one bad URL never fails the others (404 + RST isolated)', async () => { + for (const it of ITEMS) { + if (it.kind === 'ok') + server.route('GET', it.path, { + statuses: [200], + rawBody: `ok-body${it.path}`, + ttfbDelayMs: 60, + }); + else if (it.kind === 'notfound') + server.route('GET', it.path, { + statuses: [404], + body: { error: 'not_found' }, + }); + else + server.route('GET', it.path, { + statuses: [200], + rawBody: 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789', // 36 bytes advertised… + declaredLength: 36, + resetAfterBytes: 6, // …only 6 written, then a real ECONNRESET mid-body + }); + } + + const calls = ITEMS.map((it) => + download({ + baseUrl: server.url, + path: it.path, + throttle: { concurrency: 2, pool: 'host' }, + retry: { attempts: 1 }, // failures terminal — no backoff to wait on + }), + ); + + // Per-item settling — the batch shape a downloader must use (NOT `Promise.all`, which would + // collapse on the first reject). + const settled = await Promise.allSettled(calls.map((c) => c())); + + // Each item settled strictly on its OWN outcome; a sibling's failure never leaked across. + expect(settled.map((s) => s.status)).toEqual([ + 'fulfilled', // ok-0 + 'rejected', // bad-404 + 'fulfilled', // ok-1 + 'rejected', // bad-rst + 'fulfilled', // ok-2 + ]); + + // The successful items carry their COMPLETE, correct bytes — not truncated or cross-contaminated + // by the failing siblings that shared the throttle budget. + for (const i of [0, 2, 4]) { + const r = settled[i] as PromiseFulfilledResult; + expect(await blobText(r.value.blob)).toBe(`ok-body${ITEMS[i]!.path}`); + } + + // The failures are Errors, each its OWN distinct terminal shape — a 404 (engine throws `HTTP 404` + // before interpret) and a transport RST (undici "fetch failed") — never a resolved partial Blob. + const err404 = (settled[1] as PromiseRejectedResult).reason as Error; + const errRst = (settled[3] as PromiseRejectedResult).reason as Error; + expect(err404).toBeInstanceOf(Error); + expect(err404.message).toMatch(/404/); + expect(errRst).toBeInstanceOf(Error); + expect(errRst.message).toMatch(/fetch failed/i); + + // All five requests reached the server — the gate paced them, it didn't drop the failing ones. + expect(server.callCount()).toBe(ITEMS.length); +}); diff --git a/packages/core/test/gaps/download-concurrency-pool.spec.ts b/packages/core/test/gaps/download-concurrency-pool.spec.ts new file mode 100644 index 00000000..008aa927 --- /dev/null +++ b/packages/core/test/gaps/download-concurrency-pool.spec.ts @@ -0,0 +1,74 @@ +// Pins P2/P3 (download-test-rig-spec §2.8): `throttle.pool` decides whether a batch of INDEPENDENT +// download() stitches shares ONE concurrency budget or keeps N private ones — the difference the batch +// layer relies on to bound a whole host. +// +// • pool:'host' → all stitches hitting the same host share one budget (resilience.ts `hostStates`, +// keyed by URL host). N downloads, budget k ⇒ at most k open on the wire. +// • pool:'stitch' → each stitch owns its budget (closure-local). N downloads each called once ⇒ each +// admits its own call ⇒ more than k open at once. +// +// Proven by the mock server's overlap COUNT (`maxOpen`), NOT a wall-clock gap — so, unlike the old +// rate-pooling test, this can't inherit a real-timer flake (a count is a hard limiter invariant; a +// slow runner only ever makes fewer overlap, never more). `ttfbDelayMs` holds every admitted slot open +// together so the peak is observable. Real-timer, LOOSE bounds; held sockets destroyed at teardown. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +test('pool:"host" shares one budget (≤k open) while pool:"stitch" keeps independent budgets (>k open)', async () => { + const K = 2; + const N = 5; + const paths = Array.from({ length: N }, (_, i) => `/f-${i}`); + const route = (): void => { + for (const p of paths) + server.route('GET', p, { + statuses: [200], + rawBody: `body${p}`, + ttfbDelayMs: 150, // hold admitted slots open together to observe the peak overlap + // `Connection: close` so the client keeps NO keep-alive socket: the mid-test + // `server.reset()` (below) force-destroys live sockets, and a kept-alive one would be + // reused stale in phase 2 → a spurious ECONNRESET ("fetch failed", the spec's N12 + // stale-keep-alive hazard). Closing per response sidesteps it without weakening the + // overlap measurement (concurrent requests still open concurrent fresh sockets). + headers: { connection: 'close' }, + }); + }; + const fire = (pool: 'host' | 'stitch'): Promise => + Promise.all( + paths.map((p) => + download({ + baseUrl: server.url, + path: p, + throttle: { concurrency: K, pool }, + retry: { attempts: 1 }, + })(), + ), + ); + + // Phase 1 — host pooling: the whole host is capped at K regardless of how many stitches there are. + route(); + await fire('host'); + const hostMax = server.maxOpen(); + + // Reset the probe (and routes) and repeat with stitch-local pooling. + server.reset(); + route(); + await fire('stitch'); + const stitchMax = server.maxOpen(); + + // The contrast is the whole point: one shared host budget vs. N independent ones. + expect(hostMax).toBeLessThanOrEqual(K); // shared → never more than K on the wire + expect(stitchMax).toBeGreaterThan(K); // independent → the host cap does NOT bind across stitches + expect(stitchMax).toBeGreaterThan(hostMax); // measurably different, as documented +}); diff --git a/packages/core/test/gaps/download-concurrency-stall-isolation.spec.ts b/packages/core/test/gaps/download-concurrency-stall-isolation.spec.ts new file mode 100644 index 00000000..c0830083 --- /dev/null +++ b/packages/core/test/gaps/download-concurrency-stall-isolation.spec.ts @@ -0,0 +1,100 @@ +// Pins P9 (download-test-rig-spec §2.8): under concurrency, ONE stalled download must time out ALONE +// without taking its siblings down — the others complete with their bytes intact. Composes the M2 +// `stallAfterBytes` injector (a socket held open mid-body, which undici waits on forever) with a +// per-call `timeout` (the only cure — there is no idle/forward-progress timeout; see +// download-slow-vs-stall.spec.ts). The stalled item holds one slot of the shared budget until its +// timeout fires; the other slot keeps draining healthy siblings, which succeed and carry complete, +// uncorrupted blobs. This is the batch-level "one dead stream doesn't corrupt the rest" guarantee, at +// the raw download()+throttle layer. +// +// Real-timer, LOOSE bounds (a socket test): the stall's 250ms timeout is the only real duration that +// matters; healthy items are fast, and the suite closes well under a generous ceiling. The held stall +// socket is force-destroyed at teardown (the server tracks live sockets), so the suite exits. +// +// Deferred (needs the unbuilt @stitchapi/download batch API's cancellation surface): cancel-ONE +// in-flight → only that item aborts, its slot returns to the queue (P10); cancel a QUEUED item → frees +// no slot, doesn't skip the next (P11); CANCEL-ALL → every in-flight aborts, the queue drains, and the +// shared `pool:'host'` state is left clean for a later batch (P12/P13). Those are properties of a batch +// controller's AbortSignal wiring, not of a single download() call — pinned when that API exists. +import { download } from '../../src/download'; +import type { DownloadResult } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +// The stalled item sits SECOND so it is admitted in the first wave (K=2) alongside a healthy one — it +// then pins one slot for the whole timeout while the other slot drains the rest. +const ITEMS = [ + { path: '/well-0', kind: 'ok' as const }, + { path: '/stalls', kind: 'stall' as const }, + { path: '/well-1', kind: 'ok' as const }, + { path: '/well-2', kind: 'ok' as const }, +]; + +test('a stalled item under concurrency times out ALONE — siblings finish with intact blobs', async () => { + for (const it of ITEMS) { + if (it.kind === 'ok') + server.route('GET', it.path, { + statuses: [200], + rawBody: `well-body${it.path}`, + ttfbDelayMs: 40, // brief hold so healthy items genuinely overlap the stall under the gate + }); + else + server.route('GET', it.path, { + statuses: [200], + rawBody: + 'ABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ012', // 64 bytes + declaredLength: 64, // advertise 64… + stallAfterBytes: 8, // …write 8, then hold the socket open forever + }); + } + + const calls = ITEMS.map((it) => + download({ + baseUrl: server.url, + path: it.path, + throttle: { concurrency: 2, pool: 'host' }, + // The timeout is scoped to the STALLED item alone. `timeout.total` counts throttle-queue + // wait against the budget (engine.ts), so putting a 250ms timeout on the healthy siblings + // too would let the stall's held slot push a queued sibling over its OWN deadline — the + // very cross-contamination this test proves does NOT happen. Healthy items carry no + // timeout: they simply wait for a slot and complete. + ...(it.kind === 'stall' ? { timeout: 250 } : {}), + retry: { attempts: 1 }, + }), + ); + + const settled = await Promise.allSettled(calls.map((c) => c())); + + // The stalled item (index 1) rejects on its own timeout; every sibling fulfils. + expect(settled.map((s) => s.status)).toEqual([ + 'fulfilled', // well-0 + 'rejected', // stalls → timeout + 'fulfilled', // well-1 + 'fulfilled', // well-2 + ]); + + // The stall was cut by the TIMEOUT (an abort), not resolved as a partial Blob. + const stallErr = (settled[1] as PromiseRejectedResult).reason as Error; + expect(stallErr).toBeInstanceOf(Error); + expect(stallErr.message).toMatch(/timeout|timed out|abort|aborted/i); + + // Siblings' bytes are COMPLETE and uncorrupted — the stall did not distort their result. + for (const i of [0, 2, 3]) { + const r = settled[i] as PromiseFulfilledResult; + expect(await blobText(r.value.blob)).toBe(`well-body${ITEMS[i]!.path}`); + } +}); diff --git a/packages/core/test/gaps/download-conn-dns.spec.ts b/packages/core/test/gaps/download-conn-dns.spec.ts new file mode 100644 index 00000000..7f0d1975 --- /dev/null +++ b/packages/core/test/gaps/download-conn-dns.spec.ts @@ -0,0 +1,37 @@ +// Pins N7: a download to a hostname that does not resolve (DNS failure) REJECTS, and the caller-visible +// error does NOT leak the internal host — the `getaddrinfo ENOTFOUND ` string rides the transport +// cause, which the engine strips (the M2 finding), so only the generic "fetch failed" crosses the trust +// boundary. This is the download-path angle of the StitchError message-leak class. +// +// No mock server: a `.invalid` host (RFC 6761 — reserved to never resolve) makes an NXDOMAIN +// deterministic with no network dependency. The message assertion is LOOSE (a pathological/slow +// resolver could let the caller `timeout` fire first) — the invariant under test is the NO-LEAK, which +// holds either way. +import { download } from '../../src/download'; + +test('a download to an unresolvable host rejects without leaking the host', async () => { + const BOGUS = 'no-such-host-9f3a2b.invalid'; + const getIt = download({ + baseUrl: `http://${BOGUS}`, + path: '/file.bin', + timeout: 4000, // ceiling so a slow resolver can't wedge the suite + retry: { attempts: 1 }, + }); + + const started = Date.now(); + const err = await getIt().then( + () => { + throw new Error('DNS-failing download unexpectedly resolved'); + }, + (e: unknown) => e, + ); + expect(Date.now() - started).toBeLessThan(4000); // never hangs + expect(err).toBeInstanceOf(Error); + + const msg = (err as Error).message; + // A transport/DNS failure reduces to undici's generic text (or, worst case, the caller timeout). + expect(msg).toMatch(/fetch failed|timeout|timed out|abort|aborted/i); + // THE INVARIANT: the internal host and the raw resolver error never reach the caller. + expect(msg).not.toContain(BOGUS); + expect(msg).not.toMatch(/ENOTFOUND|getaddrinfo|EAI_AGAIN/i); +}); diff --git a/packages/core/test/gaps/download-conn-hangup.spec.ts b/packages/core/test/gaps/download-conn-hangup.spec.ts new file mode 100644 index 00000000..4f845004 --- /dev/null +++ b/packages/core/test/gaps/download-conn-hangup.spec.ts @@ -0,0 +1,36 @@ +// Pins N11: a server that ACCEPTS the connection then immediately FINs (closes) WITHOUT ever writing an +// HTTP status line — "no HTTP response" — must make the download REJECT, never hang. `node:http` can't +// express "accept then send nothing", so this needs the raw-socket escape hatch (startRawServer). +import { download } from '../../src/download'; +import { startRawServer } from '../support/hostile-net'; +import type { RawServer } from '../support/hostile-net'; + +let raw: RawServer; +beforeAll(async () => { + // FIN the socket the instant it connects — a graceful close before any HTTP bytes are written. + raw = await startRawServer((socket) => socket.end()); +}); +afterAll(async () => { + await raw.close(); +}); + +test('a server that hangs up with no HTTP response rejects the download (never hangs)', async () => { + const getIt = download({ + baseUrl: raw.url(), + path: '/file.bin', + timeout: 4000, + retry: { attempts: 1 }, + }); + + const started = Date.now(); + const err = await getIt().then( + () => { + throw new Error('hangup download unexpectedly resolved'); + }, + (e: unknown) => e, + ); + expect(Date.now() - started).toBeLessThan(4000); // never hangs + expect(err).toBeInstanceOf(Error); + // undici: the socket closed before a response line arrived → a generic transport failure. + expect((err as Error).message).toMatch(/fetch failed/i); +}); diff --git a/packages/core/test/gaps/download-conn-no-response.spec.ts b/packages/core/test/gaps/download-conn-no-response.spec.ts new file mode 100644 index 00000000..e6008556 --- /dev/null +++ b/packages/core/test/gaps/download-conn-no-response.spec.ts @@ -0,0 +1,42 @@ +// Pins N10: a server that ACCEPTS the connection and then goes silent — never sending a status line, +// holding the socket open — is a black hole at the HTTP layer. There is no idle/forward-progress +// timeout (see download-slow-vs-stall.spec.ts), so ONLY the caller's wall-clock `timeout` can cut it; +// without one it would hang forever. A raw net.Server that does nothing on connect models it +// deterministically (the analogue of an SYN-accepted-but-blackholed connection). +import { download } from '../../src/download'; +import { startRawServer } from '../support/hostile-net'; +import type { RawServer } from '../support/hostile-net'; + +let raw: RawServer; +beforeAll(async () => { + // Accept and hold: never write a byte, never close. Teardown force-destroys the held socket. + raw = await startRawServer(() => { + /* silence — the black hole */ + }); +}); +afterAll(async () => { + await raw.close(); +}); + +test('a connect that never produces an HTTP response is cut by the caller timeout, not hung', async () => { + const getIt = download({ + baseUrl: raw.url(), + path: '/file.bin', + timeout: 300, // the ONLY cure for a no-response black hole + retry: { attempts: 1 }, + }); + + const started = Date.now(); + const err = await getIt().then( + () => { + throw new Error('no-response download unexpectedly resolved'); + }, + (e: unknown) => e, + ); + const elapsed = Date.now() - started; + expect(err).toBeInstanceOf(Error); + expect(elapsed).toBeLessThan(5000); // never hangs + // It was the ~300ms TIMEOUT that cut it, not an instant transport reject — a clear lower margin. + expect(elapsed).toBeGreaterThanOrEqual(150); + expect((err as Error).message).toMatch(/timeout|timed out|abort|aborted/i); +}); diff --git a/packages/core/test/gaps/download-conn-refused.spec.ts b/packages/core/test/gaps/download-conn-refused.spec.ts new file mode 100644 index 00000000..19eaf83c --- /dev/null +++ b/packages/core/test/gaps/download-conn-refused.spec.ts @@ -0,0 +1,34 @@ +// Pins N9: a download to a port where nothing is listening REJECTS promptly with ECONNREFUSED, +// surfaced as the generic transport error (the ECONNREFUSED detail rides the stripped cause). Returns +// effectively instantly — there is nothing to wait for — so it must never hang. +// +// Deterministic: `unusedPort()` binds an ephemeral port, captures it, and closes it, so the connect is +// refused (nothing accepts) rather than hitting a live listener. +import { download } from '../../src/download'; +import { unusedPort } from '../support/hostile-net'; + +test('a download to a refused connection (nothing listening) rejects promptly', async () => { + const port = await unusedPort(); + const getIt = download({ + baseUrl: `http://127.0.0.1:${port}`, + path: '/file.bin', + timeout: 4000, + retry: { attempts: 1 }, + }); + + const started = Date.now(); + const err = await getIt().then( + () => { + throw new Error( + 'refused-connection download unexpectedly resolved', + ); + }, + (e: unknown) => e, + ); + // ECONNREFUSED returns at once (no listener), comfortably under the ceiling — never a hang. + expect(Date.now() - started).toBeLessThan(4000); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/fetch failed/i); + // The raw ECONNREFUSED detail rides the stripped transport cause; the caller sees only the generic text. + expect((err as Error).message).not.toMatch(/ECONNREFUSED/); +}); diff --git a/packages/core/test/gaps/download-conn-tls.spec.ts b/packages/core/test/gaps/download-conn-tls.spec.ts new file mode 100644 index 00000000..ad063c0c --- /dev/null +++ b/packages/core/test/gaps/download-conn-tls.spec.ts @@ -0,0 +1,41 @@ +// Pins N8 (best-effort): a TLS handshake failure must REJECT cleanly and never hang, surfacing the +// generic transport error (the specific TLS/cert cause rides the stripped transport cause, per the M2 +// finding). Modelled deterministically by pointing an `https://` request at a server that is NOT +// speaking TLS — a raw net.Server that destroys the socket the moment the client's ClientHello arrives, +// so the handshake cannot complete. A genuine expired/self-signed-CERT failure needs cert-generation +// infra (out of scope for a zero-dep test); a non-TLS peer is a handshake failure that needs none, and +// undici (which rejects unauthorized TLS by default) treats both as the same generic transport failure. +import { download } from '../../src/download'; +import { startRawServer } from '../support/hostile-net'; +import type { RawServer } from '../support/hostile-net'; + +let raw: RawServer; +beforeAll(async () => { + // Destroy the socket as soon as the client's TLS ClientHello arrives → the handshake fails. + raw = await startRawServer((socket) => { + socket.on('data', () => socket.destroy()); + }); +}); +afterAll(async () => { + await raw.close(); +}); + +test('an https request whose TLS handshake fails rejects cleanly (never hangs)', async () => { + const getIt = download({ + baseUrl: raw.url('https'), + path: '/file.bin', + timeout: 4000, + retry: { attempts: 1 }, + }); + + const started = Date.now(); + const err = await getIt().then( + () => { + throw new Error('TLS-failing download unexpectedly resolved'); + }, + (e: unknown) => e, + ); + expect(Date.now() - started).toBeLessThan(4000); // never hangs + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/fetch failed/i); +}); diff --git a/packages/core/test/gaps/download-content-encoding.spec.ts b/packages/core/test/gaps/download-content-encoding.spec.ts new file mode 100644 index 00000000..80cf192a --- /dev/null +++ b/packages/core/test/gaps/download-content-encoding.spec.ts @@ -0,0 +1,56 @@ +// Pins B7/B8 (the "progress lies" trap): when the body is served with a real `Content-Encoding` +// (gzip/br), undici auto-decodes it, so the download resolves with the DECOMPRESSED bytes intact — but +// byte progress reports `loaded` in DECOMPRESSED bytes against a `total` taken from the COMPRESSED +// `Content-Length`, so `loaded` can EXCEED `total`. The percentage a naive UI computes would be wrong; +// the future @stitchapi/download must suppress it (byte-count/spinner) when the response is encoded. +// This pins that the bytes are correct AND that the progress numbers are the misleading ones. +import { download } from '../../src/download'; +import type { AdapterProgress } from '../../src/types'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +// Highly compressible so the compressed Content-Length is far below the decoded size — the gap is the lie. +const BIG = 'A'.repeat(8192); + +for (const encoding of ['gzip', 'br'] as const) { + test(`a ${encoding} body decodes intact while progress total (compressed) is below loaded (decoded)`, async () => { + server.route('GET', '/asset', { + statuses: [200], + rawBody: BIG, + contentEncoding: encoding, // server sends compressed bytes + compressed Content-Length + }); + + const seen: AdapterProgress[] = []; + const out = await download({ + baseUrl: server.url, + path: '/asset', + })({ onProgress: (p) => seen.push(p) }); + + // The download resolves with the FULL, correctly DECOMPRESSED body. + expect(out.blob.size).toBe(BIG.length); + expect(await blobText(out.blob)).toBe(BIG); + + // The final progress `loaded` is the decoded byte count… + const last = seen[seen.length - 1]; + expect(last?.loaded).toBe(BIG.length); + // …and the advertised `total` — the COMPRESSED Content-Length, which undici keeps verbatim + // through auto-decode — is SMALLER than the decoded `loaded`. That inversion is the lie: a + // naive `loaded/total` percentage would exceed 100%, so a downloader must suppress it when the + // response carries a real Content-Encoding. + expect(last?.total).toBeLessThan(BIG.length); + }); +} diff --git a/packages/core/test/gaps/download-html-error-page.spec.ts b/packages/core/test/gaps/download-html-error-page.spec.ts new file mode 100644 index 00000000..f0bdb7e3 --- /dev/null +++ b/packages/core/test/gaps/download-html-error-page.spec.ts @@ -0,0 +1,40 @@ +// Pins Z4: a 200 response whose body is an HTML error/login page instead of the expected binary is +// "successful but wrong". The download surface applies NO content-type enforcement (by design — it +// returns the bytes as a Blob), so it RESOLVES with the HTML as the blob rather than failing. This +// documents that detecting "looks-OK-but-isn't" is a CALLER concern (a BYO content-type / checksum +// check) — core won't silently guard it; the blob DOES carry the server's content-type, so a caller +// (or the future @stitchapi/download package, via an opt-in guard) can catch the mismatch. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +test('a 200 HTML error page is returned as a Blob (no content-type enforcement — caller must check)', async () => { + const HTML = + 'Sign inPlease sign in'; + server.route('GET', '/asset.bin', { + statuses: [200], + rawBody: HTML, + headers: { 'content-type': 'text/html; charset=utf-8' }, + }); + + const out = await download({ baseUrl: server.url, path: '/asset.bin' })(); + + // It RESOLVES (a 200 is success to the transport) with the HTML bytes verbatim — NOT rejected. + expect(await blobText(out.blob)).toBe(HTML); + // The blob carries the server's content-type, so a caller CAN detect the mismatch — but core won't. + expect(out.blob.type).toMatch(/text\/html/); +}); diff --git a/packages/core/test/gaps/download-range-416.spec.ts b/packages/core/test/gaps/download-range-416.spec.ts new file mode 100644 index 00000000..6cde6e97 --- /dev/null +++ b/packages/core/test/gaps/download-range-416.spec.ts @@ -0,0 +1,56 @@ +// Pins R4 (server-side scaffolding for the future resume feature): when a request carries a `Range` and +// the server answers 416 Range Not Satisfiable, the download FAILS CLOSED — a 416 is ≥400, so the +// engine throws it terminally; it is never silently accepted as a body. download() never sends a Range +// on its own, so we force one via a config header to exercise the path (the future resume-aware surface +// will send real Ranges). This pins the current surface's failure mode on a 416. +// +// Deferred (gated on the client-side resume feature landing, per the rig spec §2.4): the If-Range / +// ETag-change / 200-ignoring-Range (R3/R6/R8) RESUME behaviors — the mock server can now EMIT the +// validators (`etag`/`lastModified`/`acceptRanges`) and force a 200-on-Range (`forceStatusOnRange: 200`) +// as scaffolding, but there is no client that consumes them yet, so the client-side assertions wait for +// the resume feature. This spec pins only the meaningful CURRENT behavior: a 416 rejects. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +test('a 416 to a Range request is rejected, not accepted as a body', async () => { + server.route('GET', '/ranged', { + statuses: [200], + rawBody: 'HELLO-RANGE-BODY-0123456789', + acceptRanges: 'bytes', // advertise Range support (validator scaffolding) + etag: '"v1-abc"', + forceStatusOnRange: 416, // any Range request → 416 Range Not Satisfiable + }); + + // download() never sends a Range itself; force one via a config header to reach the 416 path. + const getIt = download({ + baseUrl: server.url, + path: '/ranged', + headers: { range: 'bytes=1000-2000' }, // out of bounds → the server forces 416 + retry: { attempts: 1 }, + }); + + const err = await getIt().then( + () => { + throw new Error('416 download unexpectedly resolved'); + }, + (e: unknown) => e, + ); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/416/); + // The server DID receive the Range (the fault path fired), confirming the request carried it. + expect(server.calls('/ranged')[0]?.headers['range']).toBe( + 'bytes=1000-2000', + ); +}); diff --git a/packages/core/test/gaps/download-reset-midbody.spec.ts b/packages/core/test/gaps/download-reset-midbody.spec.ts index 7651352d..089badff 100644 --- a/packages/core/test/gaps/download-reset-midbody.spec.ts +++ b/packages/core/test/gaps/download-reset-midbody.spec.ts @@ -10,15 +10,15 @@ // EMPIRICAL FINDING (Node 24 undici, measured for this rig): at the transport boundary the RST is a // TypeError: "fetch failed" // └─ cause: SocketError { code: 'UND_ERR_SOCKET', message: 'other side closed' } -// but the engine reduces a transport failure to its top-level MESSAGE on the error event and rebuilds -// a `StitchError('fetch failed', …)` WITHOUT the `.cause` chain — so the caller (both the throwing -// AND the `.safe()` path) sees `StitchError: fetch failed` with `err.cause === undefined`. The -// `UND_ERR_SOCKET` detail is therefore NOT visible to callers today; only the generic "fetch failed" -// survives. (A candidate future improvement: carry the transport `cause` through the error event so -// callers can distinguish a socket reset from other transport failures.) This spec asserts the shape -// that actually surfaces — a reject whose message is undici's transport-failure text — not the -// stripped socket cause. `retry: { attempts: 1 }` makes the first transport failure terminal, so the -// assertion is a clean single-shot reject (no backoff to wait on). +// The engine surfaces the transport failure's top-level MESSAGE on the error event ("fetch failed"), +// and the rebuilt `StitchError` keeps that message — but it now ALSO carries the live transport error +// through the non-enumerable ERROR_SOURCE channel as `StitchError.cause` (engine.ts `errEvt` + +// stitch.ts `rebuildError`), so a caller (both the throwing AND the `.safe()` path) can reach +// `err.cause` and its nested `UND_ERR_SOCKET` code to tell a socket reset from a generic "fetch +// failed". (The cause is non-enumerable, so it never leaks into a trace sink — only the enumerable +// status/message do.) This spec asserts both: the surfaced message IS undici's transport text, AND the +// socket cause is now reachable through `err.cause`. `retry: { attempts: 1 }` makes the first transport +// failure terminal, so the assertion is a clean single-shot reject (no backoff to wait on). // // Real-timer, loose bounds (a socket test): no timeout is needed — undici rejects promptly on the // RST — but we keep `retry: { attempts: 1 }` so the reset is not papered over by a retry. @@ -56,9 +56,9 @@ test('a socket RST mid-body rejects the download — never a partial Blob', asyn // The call must reject — a partial body is never handed back as a complete Blob. The // engine-surfaced message is undici's transport-failure text ("fetch failed"); the underlying - // UND_ERR_SOCKET/"other side closed" cause is stripped by the engine (see the header note), so we - // pin the message that actually reaches the caller. It is NOT a surface-level ("download: …") - // reject — the failure happens at the transport, before `interpret` ever runs. + // UND_ERR_SOCKET/"other side closed" cause is now carried on `err.cause` (asserted below). It is + // NOT a surface-level ("download: …") reject — the failure happens at the transport, before + // `interpret` ever runs. const err = await getReset().then( () => { throw new Error('reset-mid-body download unexpectedly resolved'); @@ -69,4 +69,24 @@ test('a socket RST mid-body rejects the download — never a partial Blob', asyn expect((err as Error).message).toMatch(/fetch failed/i); // Guard that it was a transport reject, not the stray-206 / interpret-level path. expect((err as Error).message).not.toMatch(/^download:/); + + // The transport cause is now carried through: `err.cause` — undefined before this change — holds + // the live undici error, and its socket `code` (UND_ERR_SOCKET) is reachable through the cause + // chain. This is what lets a caller (e.g. @stitchapi/download's classifier) tell an RST from any + // other "fetch failed". + const cause = (err as { cause?: unknown }).cause; + expect(cause).toBeDefined(); + const codes: string[] = []; + let cur: unknown = cause; + for (let i = 0; cur != null && i < 5; i++) { + const code = (cur as { code?: unknown }).code; + if (typeof code === 'string') codes.push(code); + cur = (cur as { cause?: unknown }).cause; + } + expect(codes).toContain('UND_ERR_SOCKET'); + + // The `.safe()` path carries it too — both terminals go through the same rebuild. + const safe = await getReset().safe(); + expect(safe.ok).toBe(false); + expect((safe.error as { cause?: unknown }).cause).toBeDefined(); }); diff --git a/packages/core/test/gaps/download-retry-after.spec.ts b/packages/core/test/gaps/download-retry-after.spec.ts new file mode 100644 index 00000000..74ad5d70 --- /dev/null +++ b/packages/core/test/gaps/download-retry-after.spec.ts @@ -0,0 +1,79 @@ +// Pins X10 (S3): a 429 carrying `Retry-After` is honored on the DOWNLOAD path — the retry waits EXACTLY +// the Retry-After delta before re-attempting, then succeeds. Driven on a `manualClock` so the wait is +// virtual-time-exact (deterministic, zero wall-clock): the retry is gated until the clock is advanced +// past the delta. 429 ∈ the default retry set; `respectRetryAfter` routes the wait through the header +// value instead of the computed backoff (resilience.ts `parseRetryAfter`). +import { download } from '../../src/download'; +import type { DownloadResult } from '../../src/download'; +import { manualClock } from '../../src/test-clock'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +// Poll a real condition (a real fetch landing) up to a ceiling — an event-wait, not a timing measure. +const until = async (cond: () => boolean, ms = 2000): Promise => { + const start = Date.now(); + while (!cond()) { + if (Date.now() - start > ms) + throw new Error(`until: condition not met within ${ms}ms`); + await new Promise((r) => setTimeout(r, 5)); + } +}; + +const FILE = 'RATE-LIMITED-THEN-OK-9876543210'; + +test('a 429 Retry-After is honored on the download path — the retry waits the delta, then succeeds', async () => { + const clock = manualClock(); + server.route('GET', '/limited', { + statuses: [429, 200], + retryAfter: 1, // Retry-After: 1 (delta-seconds → 1000ms of virtual wait) + rawBody: FILE, + }); + + const getLimited = download({ + baseUrl: server.url, + path: '/limited', + retry: { attempts: 2, respectRetryAfter: true }, // 429 ∈ default on + clock, + }); + + // Drive the cold StitchResult in the background; capture its settlement for later. + let out: DownloadResult | undefined; + let error: unknown; + const done = (async () => { + try { + out = await getLimited(); + } catch (e) { + error = e; + } + })(); + + // Attempt 1 hits the server (429) and schedules the Retry-After wait on the virtual clock. + await until( + () => server.callCount('/limited') === 1 && clock.pending() === 1, + ); + // The retry is GATED on virtual time — it has NOT re-attempted yet (proof the wait is real, not 0). + expect(server.callCount('/limited')).toBe(1); + + // Advance past the 1s Retry-After delta → the retry fires → attempt 2 (200) → resolves. + await clock.advance(1000); + await done; + + expect(error).toBeUndefined(); + expect(server.callCount('/limited')).toBe(2); + expect(await blobText(out!.blob)).toBe(FILE); + expect(clock.pending()).toBe(0); // nothing leaked +}); diff --git a/packages/core/test/gaps/download-retry-policy.spec.ts b/packages/core/test/gaps/download-retry-policy.spec.ts new file mode 100644 index 00000000..e46b99d3 --- /dev/null +++ b/packages/core/test/gaps/download-retry-policy.spec.ts @@ -0,0 +1,83 @@ +// Pins X11 (S8/S9): the DOWNLOAD path inherits stitch's retry-SET decisions — 500 and 408 are NOT in +// the default retry set ([429,502,503,504]), so a download sees them ONCE and fails; opting in via +// `retry.on` restores retries. Two easy-to-miss defaults a downloader built on top must respect or +// deliberately override. (The engine retries a status only when `retryMatch(status) && attempt { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +const FILE = 'RECOVERED-BODY-abcdefghij'; + +test('500 is NOT retried by default — one attempt, then it fails', async () => { + server.route('GET', '/err', { statuses: [500, 200], rawBody: FILE }); + + const getErr = download({ + baseUrl: server.url, + path: '/err', + retry: { attempts: 3 }, // 500 ∉ default on → the extra attempts are never used + }); + + const err = await getErr().then( + () => { + throw new Error('500 download unexpectedly resolved'); + }, + (e: unknown) => e, + ); + // Exactly ONE request — a 500 is terminal by default, the [500→200] recovery is never reached. + expect(server.callCount('/err')).toBe(1); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/500/); +}); + +test('500 IS retried when opted in via retry.on', async () => { + server.route('GET', '/err', { statuses: [500, 200], rawBody: FILE }); + + const getErr = download({ + baseUrl: server.url, + path: '/err', + retry: { attempts: 2, on: [500], backoff: 'fixed', baseMs: 10 }, + }); + + const out = await getErr(); + // Opting 500 into the retry set reaches the [500→200] recovery: two attempts, then success. + expect(server.callCount('/err')).toBe(2); + expect(await blobText(out.blob)).toBe(FILE); +}); + +test('408 is NOT retried by default — one attempt, then it fails', async () => { + server.route('GET', '/req-timeout', { + statuses: [408, 200], + rawBody: FILE, + }); + + const getIt = download({ + baseUrl: server.url, + path: '/req-timeout', + retry: { attempts: 3 }, // 408 ∉ default on + }); + + const err = await getIt().then( + () => { + throw new Error('408 download unexpectedly resolved'); + }, + (e: unknown) => e, + ); + expect(server.callCount('/req-timeout')).toBe(1); + expect(err).toBeInstanceOf(Error); + expect((err as Error).message).toMatch(/408/); +}); diff --git a/packages/core/test/gaps/download-retry-transient.spec.ts b/packages/core/test/gaps/download-retry-transient.spec.ts new file mode 100644 index 00000000..d8cf7e82 --- /dev/null +++ b/packages/core/test/gaps/download-retry-transient.spec.ts @@ -0,0 +1,49 @@ +// Pins X9 (S7/S11): a transient 503 is RETRIED, and each attempt re-downloads the WHOLE file from +// byte 0 — a buffered download has no resume, so NO attempt ever sends a `Range` — finally resolving +// with the intact body once the server recovers. 503 ∈ the default retry set ([429,502,503,504]); a +// small fixed backoff keeps the retry prompt (real timer, loose — the assertion is on the OUTCOME, not +// the delay). +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +const FILE = 'THE-REAL-FILE-BODY-0123456789-abcdefghij'; + +test('a transient 503 is retried and the whole file re-downloaded (no Range), resolving intact', async () => { + server.route('GET', '/flaky', { + statuses: [503, 200], // first attempt fails transiently, second succeeds + rawBody: FILE, // the 503 body is irrelevant (engine throws ≥400 → retry) + }); + + const getFlaky = download({ + baseUrl: server.url, + path: '/flaky', + retry: { attempts: 2, backoff: 'fixed', baseMs: 10 }, // 503 ∈ default on; prompt backoff + }); + + const out = await getFlaky(); + + // Retried exactly once — two attempts reached the server (503 then 200). + expect(server.callCount('/flaky')).toBe(2); + // Every attempt was a FULL GET from byte 0: a retry RE-DOWNLOADS, it does not resume, so no attempt + // ever carries a `Range` header (the buffered surface has no byte-offset resume — that is new code + // for the future @stitchapi/download package, not a property of raw download()). + for (const c of server.calls('/flaky')) + expect(c.headers['range']).toBeUndefined(); + // …and it resolved with the complete, correct file. + expect(await blobText(out.blob)).toBe(FILE); +}); diff --git a/packages/core/test/gaps/download-size-boundaries.spec.ts b/packages/core/test/gaps/download-size-boundaries.spec.ts new file mode 100644 index 00000000..44769b77 --- /dev/null +++ b/packages/core/test/gaps/download-size-boundaries.spec.ts @@ -0,0 +1,56 @@ +// Pins Z1/Z2/Z3 + the 204 path: the buffered download handles the size boundary values correctly — +// a 0-byte body (no divide-by-zero), a 204 No Content (interpret's other accepted status), a 1-byte +// body, and a multi-MB body (generated, not stored) buffered intact. These are the corners a +// Blob-buffering surface is most likely to get subtly wrong. +import { download } from '../../src/download'; +import { startMockServer } from '../support/mock-server'; +import type { MockServer } from '../support/mock-server'; + +let server: MockServer; +beforeAll(async () => { + server = await startMockServer(); +}); +afterAll(async () => { + await server.close(); +}); +beforeEach(() => { + server.reset(); +}); + +const blobText = async (b: Blob): Promise => + new TextDecoder().decode(await b.arrayBuffer()); + +test('a 0-byte 200 resolves as an empty Blob', async () => { + server.route('GET', '/empty', { statuses: [200], rawBody: '' }); // Content-Length: 0 + const out = await download({ baseUrl: server.url, path: '/empty' })(); + expect(out.blob.size).toBe(0); +}); + +test('a 204 No Content resolves as an empty Blob (interpret accepts 204)', async () => { + server.route('GET', '/nc', { statuses: [204], rawBody: '' }); + const out = await download({ baseUrl: server.url, path: '/nc' })(); + expect(out.blob.size).toBe(0); +}); + +test('a 1-byte body resolves exactly', async () => { + server.route('GET', '/one', { statuses: [200], rawBody: 'X' }); + const out = await download({ baseUrl: server.url, path: '/one' })(); + expect(out.blob.size).toBe(1); + expect(await blobText(out.blob)).toBe('X'); +}); + +test('a multi-MB generated body buffers intact', async () => { + // 4 MB generated in-memory (not a stored fixture) — exercises buffered-Blob assembly at size. + const big = Buffer.alloc(4 * 1024 * 1024, 0x61); // 4 MiB of 'a' + server.route('GET', '/big', { statuses: [200], rawBody: big }); + + const out = await download({ baseUrl: server.url, path: '/big' })(); + expect(out.blob.size).toBe(big.length); + // Spot-check the first and last byte rather than stringifying 4 MB. + const head = new Uint8Array(await out.blob.slice(0, 1).arrayBuffer())[0]; + const tail = new Uint8Array( + await out.blob.slice(big.length - 1).arrayBuffer(), + )[0]; + expect(head).toBe(0x61); + expect(tail).toBe(0x61); +}); diff --git a/packages/core/test/gaps/throttle-host-pooling.spec.ts b/packages/core/test/gaps/throttle-host-pooling.spec.ts index 5bce755b..8b41ca0b 100644 --- a/packages/core/test/gaps/throttle-host-pooling.spec.ts +++ b/packages/core/test/gaps/throttle-host-pooling.spec.ts @@ -1,68 +1,60 @@ -// Pins docs/GAP-AUDIT.md §1.4: throttle scope:'host' must pool the budget across stitch instances in-process, as throttle.mdx documents +// Pins docs/GAP-AUDIT.md §1.4: throttle `pool:'host'` must pool the rate budget across SEPARATE +// stitch() instances hitting the same host in-process, as throttle.mdx documents. +// +// DETERMINISM — why this was rewritten (root-causing a known real-timer flake): the prior version +// measured a WALL-CLOCK gap between two SERVER-side arrival timestamps (via a real mock server) and +// asserted it was >= 250ms (the '2/s' → 500ms rate spacing). But that spacing is enforced CLIENT-side +// via `clock.sleep`; reading SERVER arrival folded in an unbounded transit / event-loop-lag term +// dominated by the FIRST fetch's cold-start (undici lazy-init + TCP handshake). On a loaded runner +// that skew ate into the 500ms and breached the 250ms floor — a flake that passed on re-run. +// +// Host pooling is a pure CLIENT-side throttle property: the budget is keyed by the request URL's host +// (resilience.ts `hostStates`), so proving it needs NO socket at all. This version drives the rate +// math on a shared `manualClock()` (ADR 0010) over the published `mockAdapter`, exactly like +// clock-seam.spec.ts — with the budget pooled, the second of two independent stitches is gated until +// the clock is advanced. Deterministic, zero wall-clock. (The new download-concurrency-*.spec.ts +// tests, which genuinely need real sockets, avoid this pattern's flake by asserting on-the-wire +// overlap COUNT via the server probe, not a measured timing gap — so they don't inherit it.) import { stitch } from '../../src'; -import { startMockServer } from '../support/mock-server'; -import type { MockServer } from '../support/mock-server'; +import { manualClock, mockAdapter } from '../../src/testing'; -import { tmpdir } from 'node:os'; -import { join } from 'node:path'; - -process.env['STITCH_TRACE_FILE'] = join( - tmpdir(), - `stitch-gap-throttle-host-pooling-${process.pid}.jsonl`, -); - -let server: MockServer; -beforeAll(async () => { - server = await startMockServer(); -}); -afterAll(async () => { - await server.close(); -}); -beforeEach(() => { - server.reset(); -}); - -describe('GAP-AUDIT §1.4 — throttle scope:"host" pools across instances', () => { - test('two separate stitches (no shared store) hitting the same host share one 2/s budget', async () => { - // Record the server-side arrival time of each request. - const hits: number[] = []; - server.route('GET', '/pooled', { - body: () => { - hits.push(Date.now()); - return { ok: true }; - }, - }); - - // Two INDEPENDENT stitch() instances — no `store` configured — both - // declaring host pooling against the same origin. throttle.mdx says - // 'host' "pools the budget across every stitch hitting the same host", - // so the 2/s budget (500ms spacing) must apply across BOTH instances. - // `a` uses the canonical `pool` (CONTRACT.md P2); `b` uses the - // `@deprecated` `scope` alias — they MUST pool together, proving the - // alias is byte-equivalent to the new field. +describe('GAP-AUDIT §1.4 — throttle pool:"host" pools the rate budget across instances', () => { + test('two independent stitches on one host share a 2/s budget — the second is gated in virtual time', async () => { + const clock = manualClock(); + // One adapter, one host URL, TWO independent stitches (no shared `store`). `a` uses the + // canonical `pool` (CONTRACT.md P2); `b` uses the @deprecated `scope` alias — they MUST pool + // together, proving the alias is byte-equivalent. Pooling can ONLY come from the module-level + // host registry (resilience.ts `hostStates`), keyed by the URL host — the property under test. + const api = mockAdapter({ respond: { body: { ok: true } } }); const a = stitch({ - baseUrl: server.url, - path: '/pooled', + url: 'https://api.test/pooled', + adapter: api, throttle: { rate: '2/s', pool: 'host' }, + clock, }); const b = stitch({ - baseUrl: server.url, - path: '/pooled', + url: 'https://api.test/pooled', + adapter: api, throttle: { rate: '2/s', scope: 'host' }, + clock, }); - await Promise.all([a(), b()]); + // `.safe()` eagerly drives each call (a cold StitchResult would not start on its own). + const pa = a.safe(); + const pb = b.safe(); + + // First grant is immediate; the second reserves the next 2/s slot (500ms out) and parks on a + // virtual-time sleep. If the budget were NOT pooled (or the `scope` alias broken), each + // instance would own a fresh 0-baseline budget and BOTH would fire at once — callCount 2, + // pending 0. So `callCount 1 + pending 1` here is precisely the pooling oracle. + await clock.advance(0); + expect(api.callCount()).toBe(1); + expect(clock.pending()).toBe(1); - expect(hits).toHaveLength(2); - hits.sort((x, y) => x - y); - const gap = (hits[1] ?? 0) - (hits[0] ?? 0); - // Pooled 2/s budget → second request paced ~500ms after the first. The - // lower bound is deliberately LOOSE (>= 250, not ~500): this is a real - // wall-clock measurement and a loaded CI runner can shave the observed - // gap well below the nominal 500ms pacing (seen at 380ms). 250ms still - // sits an order of magnitude above the ~0ms a BROKEN pool produces (each - // instance firing from its own closure-local throttle map), so the test - // keeps its meaning — it only passes when the budget is actually pooled. - expect(gap).toBeGreaterThanOrEqual(250); + // Advance past the 500ms spacing → release the second grant. Both settle; nothing leaks. + await clock.advance(500); + await Promise.all([pa, pb]); + expect(api.callCount()).toBe(2); + expect(clock.pending()).toBe(0); }); }); diff --git a/packages/core/test/support/hostile-net.ts b/packages/core/test/support/hostile-net.ts new file mode 100644 index 00000000..eebc17f8 --- /dev/null +++ b/packages/core/test/support/hostile-net.ts @@ -0,0 +1,76 @@ +// Connection-level fault helpers for the download rig — the faults that live BELOW the HTTP layer and +// that `node:http` normalizes away (spec §5 "Layer 3b — raw net.Server escape hatch"). A real +// `node:net` server (or the absence of one) is the only way to produce a genuine ECONNREFUSED, an +// immediate FIN with no HTTP response, an accept-then-silence, or a broken TLS handshake — an HTTP +// route can't. Test-only; imports `node:net` freely (never bundled). +import { createServer as createNetServer } from 'node:net'; +import type { Server, Socket } from 'node:net'; + +/** + * A TCP port that nothing is listening on: bind an ephemeral port, capture it, close, hand it back. + * A connect to `127.0.0.1:` then fails with ECONNREFUSED — nothing accepts (N9). There is an + * inherent (tiny) race — the OS could hand the port to someone else before the test connects — but on + * a loopback test host it is effectively free, and a spurious accept would only make the assertion + * fail loudly, never pass wrongly. + */ +export function unusedPort(): Promise { + return new Promise((resolve, reject) => { + const s = createNetServer(); + s.on('error', reject); + s.listen(0, '127.0.0.1', () => { + const addr = s.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + s.close(() => { + resolve(port); + }); + }); + }); +} + +export interface RawServer { + /** `http://127.0.0.1:` by default; pass `'https'` to drive a TLS-handshake fault (N8). */ + url: (scheme?: 'http' | 'https') => string; + port: number; + close: () => Promise; +} + +/** + * A raw TCP server whose per-connection behaviour is entirely up to `onConnection(socket)` — the + * escape hatch for connection-level faults an HTTP server can't express: + * • `socket.end()` → immediate FIN, no HTTP response ever (N11 hangup / "no HTTP response"). + * • `socket.destroy()` → abrupt RST. + * • do nothing → accept then silence; the client's own `timeout` must cut it (N10). + * • end/destroy on the first bytes → break a TLS ClientHello → handshake failure (N8). + * Live sockets are tracked and force-destroyed on `close()` so a held connection can't wedge teardown. + */ +export function startRawServer( + onConnection: (socket: Socket) => void, +): Promise { + const sockets = new Set(); + const server: Server = createNetServer((socket) => { + sockets.add(socket); + socket.on('close', () => sockets.delete(socket)); + // A peer reset while we're deciding what to do must not throw an unhandled 'error'. + socket.on('error', () => { + /* peer went away */ + }); + onConnection(socket); + }); + return new Promise((resolve) => { + server.listen(0, '127.0.0.1', () => { + const addr = server.address(); + const port = typeof addr === 'object' && addr ? addr.port : 0; + resolve({ + url: (scheme = 'http') => `${scheme}://127.0.0.1:${port}`, + port, + close: () => + new Promise((res) => { + for (const s of sockets) s.destroy(); + server.close(() => { + res(); + }); + }), + }); + }); + }); +} diff --git a/packages/core/test/support/mock-server.ts b/packages/core/test/support/mock-server.ts index 864d358f..ee90e71b 100644 --- a/packages/core/test/support/mock-server.ts +++ b/packages/core/test/support/mock-server.ts @@ -1,6 +1,7 @@ import { createServer } from 'node:http'; import type { IncomingMessage, Server, ServerResponse } from 'node:http'; import type { Socket } from 'node:net'; +import { brotliCompressSync, gzipSync } from 'node:zlib'; export interface ReqInfo { method: string; @@ -114,6 +115,29 @@ export interface RouteBehavior { * expressible. */ redirectTo?: string; + /** + * Serve `rawBody` COMPRESSED with this content coding (M5): sets `Content-Encoding` and a + * `Content-Length` equal to the COMPRESSED size, then sends the compressed bytes. undici's `fetch` + * auto-decodes gzip/br, so the client's blob holds the DECOMPRESSED body while byte progress sees a + * `total` (the compressed CL) SMALLER than the decoded `loaded` — the "progress lies" trap + * (B7/B8). Only on the `rawBody` path; wins over the plain framed send but yields to the fault + * paths (reset/stall/chunk), which model transport faults orthogonal to encoding. + */ + contentEncoding?: 'gzip' | 'br'; + /** Emit an `ETag` validator (C7 / future `If-Range`). M5 resume scaffolding — emittable now. */ + etag?: string; + /** Emit a `Last-Modified` validator (C8 / future `If-Range`). */ + lastModified?: string; + /** Advertise (or refuse) Range support via `Accept-Ranges: bytes|none` (C9). */ + acceptRanges?: 'bytes' | 'none'; + /** + * When a request carries a `Range` header, FORCE this status regardless of `serveRange` (M5): `416` + * (Range Not Satisfiable — emits an unsatisfiable `Content-Range`, empty body) or `200` (ignore the + * Range and send the FULL body — the silent-corruption trap R3, where a resuming client that + * appends would double the prefix). A request with NO `Range` header is served normally. Only on + * the `rawBody` path; takes precedence over `serveRange`. + */ + forceStatusOnRange?: 200 | 416; } export interface MockServer { @@ -121,6 +145,26 @@ export interface MockServer { route(method: string, path: string, behavior: RouteBehavior): void; calls(path?: string): ReqInfo[]; callCount(path?: string): number; + /** + * M4 concurrency probe — how many requests are SIMULTANEOUSLY open on the wire, so a batch/ + * throttle test asserts ACTUAL on-the-wire overlap, not "we decided to be concurrent". A request + * is counted open from receipt until its response finishes OR its socket closes (abort/reset), so + * a deliberately-held body (`ttfbDelayMs` / `stallAfterBytes` / `chunkDelayMs`) keeps its slot + * visible long enough to observe the peak. + * + * - `openNow(path?)` — the live count right now. + * - `maxOpen(path?)` — the high-water mark (max simultaneously open) since the last `reset()`. + * This is the oracle for the throttle's concurrency ceiling: `expect(maxOpen()).toBeLessThanOrEqual(k)`. + * - `arrivals(path?)` — each request's receipt timestamp (`Date.now()`), in arrival order. + * + * `path` filters to one route; omit it for the WHOLE host (every path on this server) — which, + * since one `startMockServer()` binds one origin, is exactly host-level concurrency. Multi-host + * fairness uses two servers, each with its own probe. Promotes the ad-hoc `hits.push(Date.now())` + * pattern in `throttle-host-pooling.spec.ts` to a first-class observable. + */ + openNow(path?: string): number; + maxOpen(path?: string): number; + arrivals(path?: string): number[]; reset(): void; close(): Promise; } @@ -180,6 +224,28 @@ export function startMockServer(): Promise { // server is deliberately holding open would keep the event loop alive and hang test teardown. // `server.closeAllConnections()` only fires on close; `reset()` (per-test) needs this explicit set. const sockets = new Set(); + // M4 concurrency probe. `inflight` maps a per-request token to its path while that response is + // open on the wire (from handler entry to response `close`); `openHigh` / `openHighByPath` are + // the high-water marks (max simultaneously open) since the last `reset()`; `arrivalLog` records + // each request's receipt time. Together they let a test pin the throttle's concurrency ceiling + // (`maxOpen() <= k`) from the SERVER's own view of the wire — a real-overlap oracle that a + // "we decided to be concurrent" client-side counter can't give. + const inflight = new Map(); + // The underlying socket of each IN-FLIGHT request, so `reset()` can destroy exactly the sockets + // with an incomplete response (a held `stallAfterBytes` / mid-`ttfbDelayMs` wait) and leave IDLE + // keep-alive sockets alone. Destroying an idle keep-alive socket between tests is what made undici + // reuse a dead connection on the next test's first request → a spurious ECONNRESET ("fetch failed", + // the N12 stale-keep-alive hazard). An idle socket is a valid connection to a still-listening + // server, so it must survive `reset()`; only `close()` tears every socket down. + const openSockets = new Map(); + const arrivalLog: { path: string; at: number }[] = []; + let openHigh = 0; + const openHighByPath = new Map(); + const openForPath = (p: string): number => { + let n = 0; + for (const v of inflight.values()) if (v === p) n++; + return n; + }; const key = (method: string, path: string): string => `${method.toUpperCase()} ${path}`; @@ -202,6 +268,23 @@ export function startMockServer(): Promise { body: await readBody(req), }; log.push(info); + // Mark this request open on the wire (M4 probe): counted from receipt until its response + // completes OR its socket closes. `res.once('close')` fires exactly once for BOTH a clean + // finish and an abort/reset (a held socket destroyed at teardown), so the slot is always + // released — a stalled body stays "open" until teardown, which is the truth of the wire. + const openToken = Symbol(); + inflight.set(openToken, path); + if (res.socket) openSockets.set(openToken, res.socket); + arrivalLog.push({ path, at: Date.now() }); + openHigh = Math.max(openHigh, inflight.size); + openHighByPath.set( + path, + Math.max(openHighByPath.get(path) ?? 0, openForPath(path)), + ); + res.once('close', () => { + inflight.delete(openToken); + openSockets.delete(openToken); + }); const rk = key(method, path); const behavior = routes.get(rk); @@ -224,6 +307,13 @@ export function startMockServer(): Promise { ); if (extra?.retryAfter !== undefined) out['Retry-After'] = String(extra.retryAfter); + // Validators / range-advertising headers (M5 resume scaffolding — emittable now; no client + // consumes them until the resume feature lands). Cheap: just header emission. + if (extra?.etag !== undefined) out['ETag'] = extra.etag; + if (extra?.lastModified !== undefined) + out['Last-Modified'] = extra.lastModified; + if (extra?.acceptRanges !== undefined) + out['Accept-Ranges'] = extra.acceptRanges; return out; }; const send = ( @@ -388,6 +478,27 @@ export function startMockServer(): Promise { } if (!res.writableEnded && !res.destroyed) res.end(); }; + // Compress `bytes` with the route's content coding and frame it honestly: `Content-Encoding` + + // a `Content-Length` equal to the COMPRESSED size. undici auto-decodes, so the client sees the + // full decompressed body but a `total` (compressed CL) below the decoded `loaded` — B7/B8. + const sendEncoded = ( + status: number, + bytes: Buffer, + extra: RouteBehavior, + ): void => { + res.on('error', () => { + /* client went away mid-write */ + }); + const coded = + extra.contentEncoding === 'br' + ? brotliCompressSync(bytes) + : gzipSync(bytes); + const out = buildHeaders('application/octet-stream', extra); + out['content-encoding'] = extra.contentEncoding ?? 'gzip'; + out['content-length'] = String(coded.length); + res.writeHead(status, out); + res.end(coded); + }; if (!behavior) { send(404, { error: 'not_found' }); @@ -457,20 +568,41 @@ export function startMockServer(): Promise { typeof behavior.rawBody === 'string' ? Buffer.from(behavior.rawBody, 'utf8') : Buffer.from(behavior.rawBody); - // serveRange short-circuits before any TTFB delay (it's an M1 range case, not a fault). - if (behavior.serveRange && serveRangeIf(full, behavior)) return; + // A Range request can be FORCED to 416/200 (M5 adversarial resume cases), taking + // precedence over `serveRange` — a request with no `Range` header is served normally below. + if ( + behavior.forceStatusOnRange !== undefined && + headers['range'] !== undefined + ) { + if (behavior.forceStatusOnRange === 416) { + const out = buildHeaders( + 'application/octet-stream', + behavior, + ); + out['content-range'] = `bytes */${full.length}`; + res.writeHead(416, out); + res.end(); + return; + } + // 200: fall through and serve the FULL body, deliberately ignoring the Range (R3). + } else if (behavior.serveRange && serveRangeIf(full, behavior)) { + // serveRange short-circuits before any TTFB delay (an M1 range case, not a fault). + return; + } // Slow time-to-first-byte: the request is fully received; hold before writing the status // line + headers. The socket is tracked, so teardown can cut a mid-wait hold. if (behavior.ttfbDelayMs) await sleep(behavior.ttfbDelayMs); if (res.writableEnded || res.destroyed) return; // torn down during the TTFB wait // Exactly one fault path wins, in precedence order: abrupt reset, idle stall, steady - // throttle (chunked), then the M1 clean-FIN truncation / plain framed send. + // throttle (chunked), content-encoding, then the M1 clean-FIN truncation / plain send. if (behavior.resetAfterBytes !== undefined) sendReset(status, full, behavior); else if (behavior.stallAfterBytes !== undefined) sendStall(status, full, behavior); else if (behavior.chunkDelayMs !== undefined) await sendChunked(status, full, behavior); + else if (behavior.contentEncoding !== undefined) + sendEncoded(status, full, behavior); else sendRaw(status, full, behavior); return; } @@ -526,14 +658,36 @@ export function startMockServer(): Promise { }, calls: filter, callCount: (path) => filter(path).length, + openNow: (path) => + path === undefined ? inflight.size : openForPath(path), + maxOpen: (path) => + path === undefined + ? openHigh + : (openHighByPath.get(path) ?? 0), + arrivals: (path) => + (path === undefined + ? arrivalLog + : arrivalLog.filter((a) => a.path === path) + ).map((a) => a.at), reset() { routes.clear(); counters.clear(); log.length = 0; - // Destroy any socket a prior test left deliberately open (a `stallAfterBytes` - // hold, a mid-`ttfbDelayMs` wait) so it can't leak into the next test or keep the - // loop alive. Each destroy fires the socket's own `close` → removed from the set. - for (const socket of sockets) socket.destroy(); + // M4 probe: clear the concurrency high-water marks + arrival log for the next + // test. Cleared BEFORE the socket sweep below so a late `close` (from a destroyed + // held socket) can only no-op against an already-empty `inflight`, never underflow. + inflight.clear(); + arrivalLog.length = 0; + openHigh = 0; + openHighByPath.clear(); + // Destroy ONLY the sockets with an incomplete response a prior test left open (a + // `stallAfterBytes` hold, a mid-`ttfbDelayMs` wait) so they can't leak into the next + // test or keep the loop alive. IDLE keep-alive sockets are deliberately left alive — + // they are valid connections to a still-listening server, and destroying them is + // what made undici reuse a dead connection on the next test's first request (the + // N12 stale-keep-alive "fetch failed"). `close()` still tears every socket down. + for (const socket of openSockets.values()) socket.destroy(); + openSockets.clear(); }, close: () => new Promise((res) => {