From 12f6de738da13fd3cd0c2506d843d8c7140d7cea Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Thu, 8 Oct 2026 17:55:22 -0400 Subject: [PATCH 01/16] feat: Add optional streaming members to the platform fetch types --- .../common/src/api/platform/Requests.ts | 85 +++++++++++++++++++ .../EventSource.fetchInjection.test.ts | 36 +++++++- .../eventsource/__tests__/httpHelpers.test.ts | 6 +- .../__tests__/typeCompatibility.test.ts | 49 ++++++++++- .../shared/eventsource/src/httpHelpers.ts | 17 +++- 5 files changed, 187 insertions(+), 6 deletions(-) diff --git a/packages/shared/common/src/api/platform/Requests.ts b/packages/shared/common/src/api/platform/Requests.ts index 22924e5a02..7dba8d1931 100644 --- a/packages/shared/common/src/api/platform/Requests.ts +++ b/packages/shared/common/src/api/platform/Requests.ts @@ -51,6 +51,44 @@ export interface Headers { * @param name The name of the header to check. */ has(name: string): boolean; + + /** + * Executes the callback once for each header, with the value first. Optional so existing + * implementations remain valid. A streaming implementation must provide it. The streaming + * client reads headers only through this method, and content-type validation stops working + * without it. + * + * https://developer.mozilla.org/en-US/docs/Web/API/Headers/forEach + */ + forEach?(callback: (value: string, key: string) => void): void; +} + +/** + * A reader over a response body stream. + */ +export interface BodyReader { + /** + * Resolves with the next chunk, or with `done: true` when the server ends the stream. + * Rejects when the connection drops. + */ + read(): Promise<{ done: boolean; value?: Uint8Array }>; + + /** + * Releases the reader and its connection. + */ + cancel?(): Promise | void; +} + +/** + * A response body stream. + */ +export interface ResponseBody { + getReader(): BodyReader; + + /** + * Releases the body without reading it. + */ + cancel?(): Promise | void; } /** @@ -60,6 +98,22 @@ export interface Response { headers: Headers; status: number; + /** + * The HTTP status message of the response. Optional so existing implementations remain + * valid. + * + * https://developer.mozilla.org/en-US/docs/Web/API/Response/statusText + */ + statusText?: string; + + /** + * The response body stream. An implementation provides it for a request made with + * {@link Options.streaming}. It can also provide it for other requests. + * + * https://developer.mozilla.org/en-US/docs/Web/API/Response/body + */ + body?: ResponseBody | null; + /** * Read the response and provide it as a string. */ @@ -71,6 +125,17 @@ export interface Response { json(): Promise; } +/** + * The subset of an AbortSignal that a platform implementation consumes. The standard + * AbortSignal satisfies it, and the declaration keeps this package's types free of + * platform globals. + */ +export interface AbortSignalLike { + readonly aborted: boolean; + addEventListener(type: 'abort', listener: () => void, options?: { once?: boolean }): void; + removeEventListener(type: 'abort', listener: () => void): void; +} + export interface Options { headers?: Record; method?: string; @@ -86,6 +151,26 @@ export interface Options { * https://developer.mozilla.org/en-US/docs/Web/API/RequestInit#keepalive */ keepalive?: boolean; + + /** + * An abort signal for the request. When the signal aborts, the implementation stops the + * request and rejects any pending read. Platform support for this field is best effort. + * + * https://developer.mozilla.org/en-US/docs/Web/API/RequestInit#signal + */ + signal?: AbortSignalLike; + + /** + * True for a streaming request. The implementation delivers response chunks with low + * latency. It does not request compressed content, does not buffer the response body, + * and applies no request timeout. The response exposes its body through + * {@link Response.body}. An SDK must use a fetch as a streaming transport only when the + * implementation honors this field. A streaming implementation must provide at least one + * release path. It honors {@link Options.signal}, or it provides the body cancel. It + * preferably provides both. When `streaming` is true, the implementation ignores + * {@link Options.timeout}. + */ + streaming?: boolean; } export interface EventSourceCapabilities { diff --git a/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts b/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts index 31898d74d1..b04b199f64 100644 --- a/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts +++ b/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts @@ -191,7 +191,7 @@ it('reports a non-200 response from an injected fetch as an error', async () => } }); -it('accepts a response whose transport supplies no headers at all', async () => { +it('accepts a response whose headers iterate no entries', async () => { const injected: FetchLike = async () => idleStreamResponse(['data: hello\n\n'], { forEach() {} }); const url = `http://localhost:${deliberatelyUnusedPort}/stream`; const es = createEventSource(url, { fetch: injected }); @@ -204,6 +204,40 @@ it('accepts a response whose transport supplies no headers at all', async () => } }); +it('opens the stream when the response headers expose no forEach', async () => { + // A transport without header iteration reports no headers. A 200 with no headers is accepted, + // so the stream must open rather than fail as unreadable. + const injected: FetchLike = async () => idleStreamResponse(['data: hello\n\n'], {}); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const es = createEventSource(url, { fetch: injected }); + es.onerror = () => {}; + try { + const messages = startMessageQueue(es); + expect((await messages.take()).data).toEqual('hello'); + } finally { + es.close(); + } +}); + +it('reports an empty message for a non-200 response without a statusText', async () => { + const injected: FetchLike = async () => ({ + status: 503, + headers: {}, + body: null, + }); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const errors = new AsyncQueue(); + const es = createEventSource(url, { fetch: injected, errorFilter: () => false }); + es.onerror = (e) => errors.add(e); + try { + const err = await errors.take(); + expect(err?.status).toEqual(503); + expect(err?.message).toEqual(''); + } finally { + es.close(); + } +}); + it('aborts the request when a 200 response has no body', async () => { let signal: AbortSignal | undefined; const injected: FetchLike = async (_url, init) => { diff --git a/packages/shared/eventsource/__tests__/httpHelpers.test.ts b/packages/shared/eventsource/__tests__/httpHelpers.test.ts index c0b643ec53..6da1cf2ec6 100644 --- a/packages/shared/eventsource/__tests__/httpHelpers.test.ts +++ b/packages/shared/eventsource/__tests__/httpHelpers.test.ts @@ -1,4 +1,4 @@ -import { splitHeaderListValue } from '../src/httpHelpers'; +import { headersToObject, splitHeaderListValue } from '../src/httpHelpers'; it('splits a joined value on commas outside quotes', () => { expect(splitHeaderListValue('a, b, c')).toEqual(['a', ' b', ' c']); @@ -38,3 +38,7 @@ it('runs an unterminated quoted string to the end of the value', () => { it('keeps empty parts', () => { expect(splitHeaderListValue('a,,b,')).toEqual(['a', '', 'b', '']); }); + +it('returns an empty object when the headers expose no forEach', () => { + expect(headersToObject({})).toEqual({}); +}); diff --git a/packages/shared/eventsource/__tests__/typeCompatibility.test.ts b/packages/shared/eventsource/__tests__/typeCompatibility.test.ts index a7d7aed4d8..7cebfe2086 100644 --- a/packages/shared/eventsource/__tests__/typeCompatibility.test.ts +++ b/packages/shared/eventsource/__tests__/typeCompatibility.test.ts @@ -3,11 +3,22 @@ import type { EventSource as PlatformEventSource, EventSourceInitDict as PlatformEventSourceInitDict, EventSourceRetryDelayStrategy as PlatformRetryDelayStrategy, + Headers as PlatformHeaders, + Options as PlatformOptions, + Requests as PlatformRequests, + Response as PlatformResponse, } from '@launchdarkly/js-sdk-common'; import { makeEvent, MessageEvent, RetryEvent } from '../src/Event'; import { createEventSource } from '../src/EventSource'; -import { EventSourceInitDict, RetryDelayStrategy } from '../src/types'; +import { + EventSourceInitDict, + FetchHeaders, + FetchLike, + FetchLikeOptions, + FetchLikeResponse, + RetryDelayStrategy, +} from '../src/types'; import { deliberatelyUnusedPort } from './helpers'; /** @@ -35,6 +46,42 @@ type UnsupportedPlatformOptions = Exclude< // eslint-disable-next-line @typescript-eslint/no-unused-vars const unsupportedOptionsCheck: UnsupportedPlatformOptions extends never ? true : false = true; +/** + * The transport seam. A platform `Requests.fetch` doubles as this package's injected `FetchLike` + * through structural typing, with no dependency between the packages in either direction. The + * checks below are the enforcement point for that contract. The first check is the contract + * itself. A platform fetch function is assignable to `FetchLike`. + */ +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const platformFetchSatisfiesFetchLike: (f: PlatformRequests['fetch']) => FetchLike = (f) => f; + +/** + * Every request member this package produces must be a declared platform option, so a platform + * implementation knows to handle it. `credentials` is the deliberate exception. A platform + * transport over a raw socket API has no equivalent and ignores it. + */ +type UnsupportedFetchOptionMembers = Exclude; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const fetchOptionMembersCheck: UnsupportedFetchOptionMembers extends 'credentials' ? true : false = + true; + +/** + * Every response member this package reads must be declarable on the platform response, so a + * platform implementation knows what to expose. `url` is the deliberate exception. The client + * falls back to the request url for the message origin. + */ +type UnsupportedFetchResponseMembers = Exclude; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const fetchResponseMembersCheck: UnsupportedFetchResponseMembers extends 'url' ? true : false = + true; + +/** + * Every header member this package reads must be declarable on the platform headers. + */ +type UnsupportedFetchHeaderMembers = Exclude; +// eslint-disable-next-line @typescript-eslint/no-unused-vars +const fetchHeaderMembersCheck: UnsupportedFetchHeaderMembers extends never ? true : false = true; + /** * The retry strategy seam. An SDK adapts a `RetryState` into the platform's * `EventSourceRetryDelayStrategy` and the platform adapter spreads it into this package's init diff --git a/packages/shared/eventsource/src/httpHelpers.ts b/packages/shared/eventsource/src/httpHelpers.ts index d78149c58b..073bdb0abf 100644 --- a/packages/shared/eventsource/src/httpHelpers.ts +++ b/packages/shared/eventsource/src/httpHelpers.ts @@ -11,10 +11,12 @@ */ /** - * The response headers. `forEach` is the only member this package reads. + * The response headers. `forEach` is the only member this package reads. It is optional so a + * transport whose response headers expose no iteration can still satisfy the type. The client + * then treats the response as carrying no headers. */ export interface FetchHeaders { - forEach(callback: (value: string, key: string) => void): void; + forEach?(callback: (value: string, key: string) => void): void; } /** @@ -50,7 +52,11 @@ export interface FetchResponseBody { */ export interface FetchLikeResponse { readonly status: number; - readonly statusText: string; + /** + * The HTTP status message. It is optional so a minimal injected transport can omit it. The + * client then reports an empty message for a failed response. + */ + readonly statusText?: string; readonly headers: FetchHeaders; readonly body?: FetchResponseBody | null; /** @@ -93,6 +99,11 @@ export const bodylessMethods = ['GET', 'HEAD']; export function headersToObject(headers: FetchHeaders): Record { const result: Record = {}; + if (!headers.forEach) { + // A transport without header iteration reports no headers. The client treats that the same + // as an empty header set. + return result; + } headers.forEach((value, key) => { // A standard Headers object reports lowercase names. The fold gives a transport that reports // wire-cased names the same shape, so a consumer can look a header up by its lowercase name. From 69952fc325a7dd6502be3a26f0d4dd0cddc23e24 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 9 Oct 2026 09:41:07 -0400 Subject: [PATCH 02/16] fix: Keep the platform options assignable to a native fetch RequestInit --- .../common/src/api/platform/Requests.ts | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/packages/shared/common/src/api/platform/Requests.ts b/packages/shared/common/src/api/platform/Requests.ts index 7dba8d1931..278a4cb2c9 100644 --- a/packages/shared/common/src/api/platform/Requests.ts +++ b/packages/shared/common/src/api/platform/Requests.ts @@ -126,15 +126,19 @@ export interface Response { } /** - * The subset of an AbortSignal that a platform implementation consumes. The standard - * AbortSignal satisfies it, and the declaration keeps this package's types free of - * platform globals. + * The platform's own `AbortSignal` type when the platform declares one on the global scope. + * An implementation that passes {@link Options} to a native `fetch` therefore type-checks + * against its platform's `RequestInit`. The fallback is the minimal structural subset an + * implementation consumes, which keeps these declarations valid on a platform without the + * global. */ -export interface AbortSignalLike { - readonly aborted: boolean; - addEventListener(type: 'abort', listener: () => void, options?: { once?: boolean }): void; - removeEventListener(type: 'abort', listener: () => void): void; -} +export type AbortSignalLike = typeof globalThis extends { AbortSignal: { prototype: infer T } } + ? T + : { + readonly aborted: boolean; + addEventListener(type: 'abort', listener: () => void, options?: { once?: boolean }): void; + removeEventListener(type: 'abort', listener: () => void): void; + }; export interface Options { headers?: Record; From de805c0e8a0926fecb194a786ca751f3e5b53445 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 9 Oct 2026 09:48:26 -0400 Subject: [PATCH 03/16] fix: Convert the platform options at the Oxygen fetch boundary --- .../src/platform/OxygenRequests.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts b/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts index d46b08cabb..08d894d4b2 100644 --- a/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts +++ b/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts @@ -9,6 +9,16 @@ import type { import { OxygenCacheOptions } from '../utils/validateOptions'; +/** + * Converts the platform options into a native fetch RequestInit. The platform signal type + * falls back to a structural subset when a tsconfig declares no global AbortSignal + * constructor, and this package's test configuration is such a case. The runtime object is + * always a real signal, so the assertion only restores the type the runtime already has. + */ +function toRequestInit(options: Options): RequestInit { + return { ...options, signal: options.signal as AbortSignal | undefined }; +} + export default class OxygenRequests implements platform.Requests { private _cache: Cache | null = null; private _cacheOptions: OxygenCacheOptions; @@ -62,10 +72,10 @@ export default class OxygenRequests implements platform.Requests { // - Cache API not available // - Cache is not enabled per initialization options // - Not a GET request (for now, we mostly interested in caching the feature poll request) - return fetch(url, options); + return fetch(url, toRequestInit(options)); } - const request = new Request(url, finalOptions); + const request = new Request(url, toRequestInit(finalOptions)); const cachedResponse = await cache.match(request); @@ -81,7 +91,7 @@ export default class OxygenRequests implements platform.Requests { request: Request, cache: Cache, ): Promise { - const response = await fetch(url, options); + const response = await fetch(url, toRequestInit(options)); // Only cache successful GET requests if (cache && response.ok && (!options.method || options.method === 'GET')) { From 6befff98d0625e2786d71d3dd7bd8403fb9cda9c Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 9 Oct 2026 10:31:25 -0400 Subject: [PATCH 04/16] refactor: Commonize the native fetch request-init conversion --- .../shopify-oxygen/src/platform/OxygenRequests.ts | 12 +----------- packages/shared/common/src/api/platform/Requests.ts | 11 +++++++++++ 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts b/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts index 08d894d4b2..232d9aa091 100644 --- a/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts +++ b/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts @@ -1,4 +1,4 @@ -import { NullEventSource } from '@launchdarkly/js-server-sdk-common'; +import { NullEventSource, toRequestInit } from '@launchdarkly/js-server-sdk-common'; import type { EventSource, EventSourceCapabilities, @@ -9,16 +9,6 @@ import type { import { OxygenCacheOptions } from '../utils/validateOptions'; -/** - * Converts the platform options into a native fetch RequestInit. The platform signal type - * falls back to a structural subset when a tsconfig declares no global AbortSignal - * constructor, and this package's test configuration is such a case. The runtime object is - * always a real signal, so the assertion only restores the type the runtime already has. - */ -function toRequestInit(options: Options): RequestInit { - return { ...options, signal: options.signal as AbortSignal | undefined }; -} - export default class OxygenRequests implements platform.Requests { private _cache: Cache | null = null; private _cacheOptions: OxygenCacheOptions; diff --git a/packages/shared/common/src/api/platform/Requests.ts b/packages/shared/common/src/api/platform/Requests.ts index 278a4cb2c9..fe10662869 100644 --- a/packages/shared/common/src/api/platform/Requests.ts +++ b/packages/shared/common/src/api/platform/Requests.ts @@ -177,6 +177,17 @@ export interface Options { streaming?: boolean; } +/** + * Converts the request options into the init shape a platform's native `fetch` accepts. A + * platform can declare its `AbortSignal` as a lexical class, which {@link AbortSignalLike} + * cannot resolve to, so the `signal` member widens here. The runtime value is always a real + * signal for the running platform, so the widening only restores the type the runtime + * already has. + */ +export function toRequestInit(options: Options): Omit & { signal?: any } { + return options; +} + export interface EventSourceCapabilities { /** * If true the event source supports read timeouts. A read timeout for an From ab84b96177caa5b2828f599f1556852b2da338c9 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 9 Oct 2026 11:55:08 -0400 Subject: [PATCH 05/16] chore: keep the request-init conversion out of public api surface --- .../src/platform/OxygenRequests.ts | 8 +++---- .../common/src/api/platform/Requests.ts | 22 +++++-------------- .../shared/common/src/internal/fetch/index.ts | 1 + .../src/internal/fetch/toRequestInit.ts | 12 ++++++++++ packages/shared/common/src/internal/index.ts | 1 + .../__tests__/typeCompatibility.test.ts | 12 ++++++---- 6 files changed, 32 insertions(+), 24 deletions(-) create mode 100644 packages/shared/common/src/internal/fetch/index.ts create mode 100644 packages/shared/common/src/internal/fetch/toRequestInit.ts diff --git a/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts b/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts index 232d9aa091..58a65d2392 100644 --- a/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts +++ b/packages/sdk/shopify-oxygen/src/platform/OxygenRequests.ts @@ -1,4 +1,4 @@ -import { NullEventSource, toRequestInit } from '@launchdarkly/js-server-sdk-common'; +import { internal, NullEventSource } from '@launchdarkly/js-server-sdk-common'; import type { EventSource, EventSourceCapabilities, @@ -62,10 +62,10 @@ export default class OxygenRequests implements platform.Requests { // - Cache API not available // - Cache is not enabled per initialization options // - Not a GET request (for now, we mostly interested in caching the feature poll request) - return fetch(url, toRequestInit(options)); + return fetch(url, internal.toRequestInit(options)); } - const request = new Request(url, toRequestInit(finalOptions)); + const request = new Request(url, internal.toRequestInit(finalOptions)); const cachedResponse = await cache.match(request); @@ -81,7 +81,7 @@ export default class OxygenRequests implements platform.Requests { request: Request, cache: Cache, ): Promise { - const response = await fetch(url, toRequestInit(options)); + const response = await fetch(url, internal.toRequestInit(options)); // Only cache successful GET requests if (cache && response.ok && (!options.method || options.method === 'GET')) { diff --git a/packages/shared/common/src/api/platform/Requests.ts b/packages/shared/common/src/api/platform/Requests.ts index fe10662869..b394dc7106 100644 --- a/packages/shared/common/src/api/platform/Requests.ts +++ b/packages/shared/common/src/api/platform/Requests.ts @@ -126,11 +126,12 @@ export interface Response { } /** - * The platform's own `AbortSignal` type when the platform declares one on the global scope. - * An implementation that passes {@link Options} to a native `fetch` therefore type-checks - * against its platform's `RequestInit`. The fallback is the minimal structural subset an - * implementation consumes, which keeps these declarations valid on a platform without the - * global. + * The platform's own `AbortSignal` type when the platform declares one as a global + * variable. An implementation that passes {@link Options} to a native `fetch` then + * type-checks against its platform's `RequestInit`. A platform that declares + * `AbortSignal` as a lexical class resolves to the fallback instead. The fallback is + * the minimal structural subset an implementation consumes. It keeps these declarations + * valid on a platform without the global. */ export type AbortSignalLike = typeof globalThis extends { AbortSignal: { prototype: infer T } } ? T @@ -177,17 +178,6 @@ export interface Options { streaming?: boolean; } -/** - * Converts the request options into the init shape a platform's native `fetch` accepts. A - * platform can declare its `AbortSignal` as a lexical class, which {@link AbortSignalLike} - * cannot resolve to, so the `signal` member widens here. The runtime value is always a real - * signal for the running platform, so the widening only restores the type the runtime - * already has. - */ -export function toRequestInit(options: Options): Omit & { signal?: any } { - return options; -} - export interface EventSourceCapabilities { /** * If true the event source supports read timeouts. A read timeout for an diff --git a/packages/shared/common/src/internal/fetch/index.ts b/packages/shared/common/src/internal/fetch/index.ts new file mode 100644 index 0000000000..771e3f74b7 --- /dev/null +++ b/packages/shared/common/src/internal/fetch/index.ts @@ -0,0 +1 @@ +export * from './toRequestInit'; diff --git a/packages/shared/common/src/internal/fetch/toRequestInit.ts b/packages/shared/common/src/internal/fetch/toRequestInit.ts new file mode 100644 index 0000000000..c8b6ccb3a0 --- /dev/null +++ b/packages/shared/common/src/internal/fetch/toRequestInit.ts @@ -0,0 +1,12 @@ +import { Options } from '../../api/platform/Requests'; + +/** + * Converts the request options into the init shape a platform's native `fetch` accepts. A + * platform can declare its `AbortSignal` as a lexical class, which `AbortSignalLike` + * cannot resolve to, so the `signal` member widens here. The caller must pass a signal + * created by the running platform. The widening then only restores the type the runtime + * already has. + */ +export function toRequestInit(options: Options): Omit & { signal?: any } { + return options; +} diff --git a/packages/shared/common/src/internal/index.ts b/packages/shared/common/src/internal/index.ts index 7c1630e1b0..64bc52d368 100644 --- a/packages/shared/common/src/internal/index.ts +++ b/packages/shared/common/src/internal/index.ts @@ -3,6 +3,7 @@ export * from './diagnostics'; export * from './evaluation'; export * from './events'; export * from './fdv2'; +export * from './fetch'; export * from './metadata'; export * from './plugins'; export * from './json'; diff --git a/packages/shared/eventsource/__tests__/typeCompatibility.test.ts b/packages/shared/eventsource/__tests__/typeCompatibility.test.ts index 7cebfe2086..29474f6aed 100644 --- a/packages/shared/eventsource/__tests__/typeCompatibility.test.ts +++ b/packages/shared/eventsource/__tests__/typeCompatibility.test.ts @@ -55,6 +55,12 @@ const unsupportedOptionsCheck: UnsupportedPlatformOptions extends never ? true : // eslint-disable-next-line @typescript-eslint/no-unused-vars const platformFetchSatisfiesFetchLike: (f: PlatformRequests['fetch']) => FetchLike = (f) => f; +/** + * True only when the two types are identical. The exception checks below use it so they fail + * when an exception is added and when a documented exception stops being one. + */ +type Equals = [A] extends [B] ? ([B] extends [A] ? true : false) : false; + /** * Every request member this package produces must be a declared platform option, so a platform * implementation knows to handle it. `credentials` is the deliberate exception. A platform @@ -62,8 +68,7 @@ const platformFetchSatisfiesFetchLike: (f: PlatformRequests['fetch']) => FetchLi */ type UnsupportedFetchOptionMembers = Exclude; // eslint-disable-next-line @typescript-eslint/no-unused-vars -const fetchOptionMembersCheck: UnsupportedFetchOptionMembers extends 'credentials' ? true : false = - true; +const fetchOptionMembersCheck: Equals = true; /** * Every response member this package reads must be declarable on the platform response, so a @@ -72,8 +77,7 @@ const fetchOptionMembersCheck: UnsupportedFetchOptionMembers extends 'credential */ type UnsupportedFetchResponseMembers = Exclude; // eslint-disable-next-line @typescript-eslint/no-unused-vars -const fetchResponseMembersCheck: UnsupportedFetchResponseMembers extends 'url' ? true : false = - true; +const fetchResponseMembersCheck: Equals = true; /** * Every header member this package reads must be declarable on the platform headers. From a27de8500339120d7f1c92d1cfa3e70c9eaa81f9 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Fri, 9 Oct 2026 14:28:05 -0400 Subject: [PATCH 06/16] chore: commonizing streaming response --- .../internal/fetch/StreamingResponse.test.ts | 107 ++++++++++++ .../src/internal/fetch/StreamingResponse.ts | 157 ++++++++++++++++++ .../shared/common/src/internal/fetch/index.ts | 1 + packages/shared/common/tsconfig.json | 2 +- 4 files changed, 266 insertions(+), 1 deletion(-) create mode 100644 packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts create mode 100644 packages/shared/common/src/internal/fetch/StreamingResponse.ts diff --git a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts new file mode 100644 index 0000000000..04f14d8ca8 --- /dev/null +++ b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts @@ -0,0 +1,107 @@ +import { + createStreamingResponse, + RawHeaders, + StreamingResponseSource, +} from '../../../src/internal/fetch/StreamingResponse'; + +function utf8(text: string): Uint8Array { + return new Uint8Array(text.split('').map((char) => char.charCodeAt(0))); +} + +function makeSource( + parts: string[], + meta: { headers?: RawHeaders; statusCode?: number; statusMessage?: string } = {}, +): StreamingResponseSource { + let index = 0; + return { + headers: meta.headers ?? {}, + statusCode: meta.statusCode, + statusMessage: meta.statusMessage, + [Symbol.asyncIterator]: () => ({ + next: async () => { + if (index < parts.length) { + index += 1; + return { done: false, value: utf8(parts[index - 1]) }; + } + return { done: true }; + }, + }), + }; +} + +it('exposes the status code and status message', () => { + const res = createStreamingResponse( + makeSource([], { statusCode: 301, statusMessage: 'Moved Permanently' }), + ); + expect(res.status).toBe(301); + expect(res.statusText).toBe('Moved Permanently'); +}); + +it('defaults the status to 0 and the status message to an empty string when absent', () => { + const res = createStreamingResponse(makeSource([])); + expect(res.status).toBe(0); + expect(res.statusText).toBe(''); +}); + +it('wraps the raw headers', () => { + const res = createStreamingResponse( + makeSource([], { + headers: { + 'content-type': 'text/event-stream', + 'set-cookie': ['a=1', 'b=2'], + empty: undefined, + }, + statusCode: 200, + }), + ); + expect(res.headers.get('content-type')).toBe('text/event-stream'); + expect(res.headers.get('set-cookie')).toBe('a=1, b=2'); + expect(res.headers.get('missing')).toBeNull(); + expect(res.headers.has('content-type')).toBe(true); + expect(res.headers.has('missing')).toBe(false); + expect(Array.from(res.headers.entries())).toEqual([ + ['content-type', 'text/event-stream'], + ['set-cookie', 'a=1, b=2'], + ]); + const visited: [string, string][] = []; + res.headers.forEach?.((value, key) => visited.push([key, value])); + expect(visited).toEqual([ + ['content-type', 'text/event-stream'], + ['set-cookie', 'a=1, b=2'], + ]); +}); + +it('hands out one chunk per read and then reports done', async () => { + const res = createStreamingResponse(makeSource(['first', 'second'])); + const reader = res.body.getReader(); + const first = await reader.read(); + expect(first.done).toBe(false); + expect(first.value).toEqual(utf8('first')); + const second = await reader.read(); + expect(second.value).toEqual(utf8('second')); + const end = await reader.read(); + expect(end.done).toBe(true); +}); + +it('reads the remaining stream as text', async () => { + const res = createStreamingResponse(makeSource(['hello ', 'world'])); + await expect(res.text()).resolves.toBe('hello world'); +}); + +it('parses the remaining stream as JSON', async () => { + const res = createStreamingResponse(makeSource(['{"a":', '1}'])); + await expect(res.json()).resolves.toEqual({ a: 1 }); +}); + +it('rejects a pending read when the stream errors', async () => { + const erroring: StreamingResponseSource = { + headers: {}, + [Symbol.asyncIterator]: () => ({ + next: async () => { + throw new Error('boom'); + }, + }), + }; + const res = createStreamingResponse(erroring); + await expect(res.body.getReader().read()).rejects.toThrow('boom'); +}); diff --git a/packages/shared/common/src/internal/fetch/StreamingResponse.ts b/packages/shared/common/src/internal/fetch/StreamingResponse.ts new file mode 100644 index 0000000000..6d316b7d94 --- /dev/null +++ b/packages/shared/common/src/internal/fetch/StreamingResponse.ts @@ -0,0 +1,157 @@ +import { Headers, Response, ResponseBody } from '../../api/platform/Requests'; + +// Each platform that constructs a streaming response provides the TextDecoder global. This +// package compiles without the global type libraries, so this scoped declaration stands in +// for it. +declare const TextDecoder: new () => { decode(input?: Uint8Array): string }; + +/** + * The shape of raw HTTP headers: names mapped to a value, a list of values, or no value. + * A Node `IncomingHttpHeaders` object satisfies this shape. + */ +export type RawHeaders = Record; + +/** + * The source of response chunks. A platform provides the iterator from its own stream type. + * For example, a Node `IncomingMessage` provides a compatible iterator through its + * `Symbol.asyncIterator` method. + */ +export interface ResponseChunkIterator { + /** + * Resolves with the next chunk, or with `done: true` when the stream ends. Rejects when + * the connection drops. + */ + next(): Promise<{ done?: boolean; value?: Uint8Array }>; +} + +/** + * The key of the chunk iterator on a streaming response source. The compiled lib of a + * consumer may not declare `Symbol.asyncIterator`. This conditional resolves to that symbol + * when the lib declares it, and to no key at all otherwise. It keeps these declarations + * valid for every consumer, like `AbortSignalLike` does for the abort signal. + */ +type AsyncIteratorKey = typeof Symbol extends { readonly asyncIterator: infer S } + ? S extends keyof any + ? S + : never + : never; + +/** + * The source of a streaming response: the metadata and the chunk stream. A Node + * `IncomingMessage` satisfies this shape, so a platform can pass its message directly. + */ +export type StreamingResponseSource = { + headers: RawHeaders; + statusCode?: number; + statusMessage?: string; +} & { [K in AsyncIteratorKey]: () => ResponseChunkIterator }; + +/** + * Wraps the raw headers to match those used by fetch APIs. + */ +function wrapRawHeaders(headers: RawHeaders): Headers { + function headerVal(name: string): string | null { + const val = headers[name]; + if (val === undefined || val === null) { + return null; + } + if (Array.isArray(val)) { + return val.join(', '); + } + return val; + } + + // We want to use generators here for the simplicity of maintaining + // this interface. Also they aren't expected to be high frequency usage. + function* entries(): Iterable<[string, string]> { + for (const key of Object.keys(headers)) { + const val = headerVal(key); + if (val !== null) { + yield [key, val]; + } + } + } + + function* values(): Iterable { + for (const [, val] of entries()) { + yield val; + } + } + + return { + get: headerVal, + keys: () => Object.keys(headers), + values, + entries, + // The callback receives the value first. The order matches the fetch `Headers.forEach` + // signature. Multi-value headers are joined with a comma, and headers without a value + // are skipped, like `entries`. + forEach: (callback: (value: string, key: string) => void) => { + for (const [key, value] of entries()) { + callback(value, key); + } + }, + has: (name: string) => Object.prototype.hasOwnProperty.call(headers, name), + }; +} + +/** + * A response for a streaming request. Unlike the base response, the body and the status + * message are always present. + */ +export interface StreamingResponse extends Response { + body: ResponseBody; + statusText: string; +} + +/** + * Creates a response for a streaming request. It does not buffer the body: each read hands + * out the next chunk from the source iterator. The `text` and `json` methods read the + * remainder of the stream, so a caller uses either the reader or those methods, not both. + * + * @param source The raw platform response, for example a Node `IncomingMessage`. A missing + * status code maps to 0, and a missing status message maps to an empty string. + */ +export function createStreamingResponse(source: StreamingResponseSource): StreamingResponse { + const iterator = source[Symbol.asyncIterator](); + async function text(): Promise { + const chunks: Uint8Array[] = []; + let length = 0; + let next = await iterator.next(); + while (!next.done) { + if (next.value) { + chunks.push(next.value); + length += next.value.length; + } + // The stream provides the chunks in sequence, so the reads cannot run in parallel. + // eslint-disable-next-line no-await-in-loop + next = await iterator.next(); + } + const content = new Uint8Array(length); + let offset = 0; + chunks.forEach((chunk) => { + content.set(chunk, offset); + offset += chunk.length; + }); + return new TextDecoder().decode(content); + } + + return { + headers: wrapRawHeaders(source.headers), + status: source.statusCode ?? 0, + statusText: source.statusMessage ?? '', + body: { + getReader: () => ({ + read: async () => { + const next = await iterator.next(); + if (next.done) { + return { done: true }; + } + return { done: false, value: next.value }; + }, + }), + }, + text, + json: async () => JSON.parse(await text()), + }; +} diff --git a/packages/shared/common/src/internal/fetch/index.ts b/packages/shared/common/src/internal/fetch/index.ts index 771e3f74b7..143577f299 100644 --- a/packages/shared/common/src/internal/fetch/index.ts +++ b/packages/shared/common/src/internal/fetch/index.ts @@ -1 +1,2 @@ +export * from './StreamingResponse'; export * from './toRequestInit'; diff --git a/packages/shared/common/tsconfig.json b/packages/shared/common/tsconfig.json index b5d080223b..5727fc7f07 100644 --- a/packages/shared/common/tsconfig.json +++ b/packages/shared/common/tsconfig.json @@ -3,7 +3,7 @@ "rootDir": "src", "outDir": "dist", "target": "ES2020", - "lib": ["es6"], + "lib": ["es6", "es2018.asynciterable"], "module": "ESNext", "moduleResolution": "node", "strict": true, From a9ae10bdf936a90665c148e671b4394a339c16bd Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 12 Oct 2026 10:48:18 -0400 Subject: [PATCH 07/16] ci: bump size limit again --- .github/workflows/common.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/common.yml b/.github/workflows/common.yml index c64eed93be..7c22c2bee2 100644 --- a/.github/workflows/common.yml +++ b/.github/workflows/common.yml @@ -35,4 +35,4 @@ jobs: target_file: 'packages/shared/common/dist/esm/index.mjs' package_name: '@launchdarkly/js-sdk-common' pr_number: ${{ github.event.number }} - size_limit: 30000 + size_limit: 35000 From 1e579ced72a3e3797e7f7e7dd34f8d98b6b7e2b0 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 12 Oct 2026 12:00:28 -0400 Subject: [PATCH 08/16] fix: harden the streaming response headers and add a cancel release path --- .../internal/fetch/StreamingResponse.test.ts | 104 ++++++++++++++++-- .../src/internal/fetch/StreamingResponse.ts | 85 +++++++------- 2 files changed, 134 insertions(+), 55 deletions(-) diff --git a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts index 04f14d8ca8..97f77b31b8 100644 --- a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts +++ b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts @@ -1,3 +1,5 @@ +import { Readable } from 'node:stream'; + import { createStreamingResponse, RawHeaders, @@ -5,11 +7,11 @@ import { } from '../../../src/internal/fetch/StreamingResponse'; function utf8(text: string): Uint8Array { - return new Uint8Array(text.split('').map((char) => char.charCodeAt(0))); + return new TextEncoder().encode(text); } function makeSource( - parts: string[], + parts: Uint8Array[], meta: { headers?: RawHeaders; statusCode?: number; statusMessage?: string } = {}, ): StreamingResponseSource { let index = 0; @@ -21,7 +23,7 @@ function makeSource( next: async () => { if (index < parts.length) { index += 1; - return { done: false, value: utf8(parts[index - 1]) }; + return { done: false, value: parts[index - 1] }; } return { done: true }; }, @@ -29,23 +31,30 @@ function makeSource( }; } +function makeTextSource( + parts: string[], + meta: { headers?: RawHeaders; statusCode?: number; statusMessage?: string } = {}, +): StreamingResponseSource { + return makeSource(parts.map(utf8), meta); +} + it('exposes the status code and status message', () => { const res = createStreamingResponse( - makeSource([], { statusCode: 301, statusMessage: 'Moved Permanently' }), + makeTextSource([], { statusCode: 301, statusMessage: 'Moved Permanently' }), ); expect(res.status).toBe(301); expect(res.statusText).toBe('Moved Permanently'); }); it('defaults the status to 0 and the status message to an empty string when absent', () => { - const res = createStreamingResponse(makeSource([])); + const res = createStreamingResponse(makeTextSource([])); expect(res.status).toBe(0); expect(res.statusText).toBe(''); }); it('wraps the raw headers', () => { const res = createStreamingResponse( - makeSource([], { + makeTextSource([], { headers: { 'content-type': 'text/event-stream', 'set-cookie': ['a=1', 'b=2'], @@ -71,8 +80,34 @@ it('wraps the raw headers', () => { ]); }); +it('matches header names without case sensitivity', () => { + const res = createStreamingResponse( + makeTextSource([], { headers: { 'Content-Type': 'text/event-stream' }, statusCode: 200 }), + ); + expect(res.headers.get('content-type')).toBe('text/event-stream'); + expect(res.headers.get('CONTENT-TYPE')).toBe('text/event-stream'); + expect(res.headers.has('Content-Type')).toBe(true); + expect(Array.from(res.headers.keys())).toEqual(['content-type']); +}); + +it('reports a header without a value as absent', () => { + const res = createStreamingResponse( + makeTextSource([], { headers: { empty: undefined }, statusCode: 200 }), + ); + expect(res.headers.has('empty')).toBe(false); + expect(res.headers.get('empty')).toBeNull(); + expect(Array.from(res.headers.keys())).toEqual([]); +}); + +it('does not report names inherited from the object prototype as headers', () => { + const res = createStreamingResponse(makeTextSource([], { statusCode: 200 })); + expect(res.headers.get('constructor')).toBeNull(); + expect(res.headers.get('__proto__')).toBeNull(); + expect(res.headers.has('toString')).toBe(false); +}); + it('hands out one chunk per read and then reports done', async () => { - const res = createStreamingResponse(makeSource(['first', 'second'])); + const res = createStreamingResponse(makeTextSource(['first', 'second'])); const reader = res.body.getReader(); const first = await reader.read(); expect(first.done).toBe(false); @@ -84,12 +119,20 @@ it('hands out one chunk per read and then reports done', async () => { }); it('reads the remaining stream as text', async () => { - const res = createStreamingResponse(makeSource(['hello ', 'world'])); + const res = createStreamingResponse(makeTextSource(['hello ', 'world'])); await expect(res.text()).resolves.toBe('hello world'); }); +it('decodes a multi-byte character split across chunks', async () => { + const bytes = utf8('héllo €!'); + const res = createStreamingResponse( + makeSource([bytes.slice(0, 2), bytes.slice(2, 7), bytes.slice(7)]), + ); + await expect(res.text()).resolves.toBe('héllo €!'); +}); + it('parses the remaining stream as JSON', async () => { - const res = createStreamingResponse(makeSource(['{"a":', '1}'])); + const res = createStreamingResponse(makeTextSource(['{"a":', '1}'])); await expect(res.json()).resolves.toEqual({ a: 1 }); }); @@ -105,3 +148,46 @@ it('rejects a pending read when the stream errors', async () => { const res = createStreamingResponse(erroring); await expect(res.body.getReader().read()).rejects.toThrow('boom'); }); + +it('releases the stream through the iterator return method on cancel', async () => { + const returnSpy = jest.fn(async () => ({ done: true })); + const source: StreamingResponseSource = { + headers: {}, + [Symbol.asyncIterator]: () => ({ + next: async () => ({ done: false, value: utf8('chunk') }), + return: returnSpy, + }), + }; + const res = createStreamingResponse(source); + await res.body.getReader().cancel?.(); + await res.body.cancel?.(); + expect(returnSpy).toHaveBeenCalledTimes(2); +}); + +it('tolerates cancel when the iterator has no return method', async () => { + const res = createStreamingResponse(makeTextSource(['chunk'])); + await expect(Promise.resolve(res.body.cancel?.())).resolves.toBeUndefined(); +}); + +it('accepts a real Node readable stream as the source', async () => { + const readable = Readable.from([Buffer.from('data: '), Buffer.from('hello\n\n')]); + const source: StreamingResponseSource = Object.assign(readable, { + headers: { 'content-type': 'text/event-stream' }, + statusCode: 200, + statusMessage: 'OK', + }); + const res = createStreamingResponse(source); + expect(res.status).toBe(200); + expect(res.headers.get('content-type')).toBe('text/event-stream'); + await expect(res.text()).resolves.toBe('data: hello\n\n'); +}); + +it('destroys a real Node readable stream when the caller cancels mid-stream', async () => { + const readable = Readable.from([Buffer.from('first'), Buffer.from('endless')]); + const source: StreamingResponseSource = Object.assign(readable, { headers: {} }); + const res = createStreamingResponse(source); + const reader = res.body.getReader(); + await reader.read(); + await reader.cancel?.(); + expect(readable.destroyed).toBe(true); +}); diff --git a/packages/shared/common/src/internal/fetch/StreamingResponse.ts b/packages/shared/common/src/internal/fetch/StreamingResponse.ts index 6d316b7d94..b15a2811ae 100644 --- a/packages/shared/common/src/internal/fetch/StreamingResponse.ts +++ b/packages/shared/common/src/internal/fetch/StreamingResponse.ts @@ -1,4 +1,4 @@ -import { Headers, Response, ResponseBody } from '../../api/platform/Requests'; +import type { Headers, Response, ResponseBody } from '../../api/platform/Requests'; // Each platform that constructs a streaming response provides the TextDecoder global. This // package compiles without the global type libraries, so this scoped declaration stands in @@ -22,13 +22,21 @@ export interface ResponseChunkIterator { * the connection drops. */ next(): Promise<{ done?: boolean; value?: Uint8Array }>; + + /** + * Releases the underlying stream. A Node `Readable` iterator destroys the stream when + * this method runs before the stream ends. + */ + return?(): Promise<{ done?: boolean; value?: Uint8Array }>; } /** - * The key of the chunk iterator on a streaming response source. The compiled lib of a - * consumer may not declare `Symbol.asyncIterator`. This conditional resolves to that symbol - * when the lib declares it, and to no key at all otherwise. It keeps these declarations - * valid for every consumer, like `AbortSignalLike` does for the abort signal. + * The key of the chunk iterator on a streaming response source. A consumer's compile-time + * lib may not declare `Symbol.asyncIterator`. This conditional resolves to that symbol when + * the lib declares it, and to no key at all otherwise. That keeps these declarations valid + * for every consumer, like `AbortSignalLike` does for the abort signal. Without the symbol, + * the source type cannot require the iterator, and a source without a stream fails at run + * time instead of compile time. */ type AsyncIteratorKey = typeof Symbol extends { readonly asyncIterator: infer S } ? S extends keyof any @@ -47,51 +55,27 @@ export type StreamingResponseSource = { } & { [K in AsyncIteratorKey]: () => ResponseChunkIterator }; /** - * Wraps the raw headers to match those used by fetch APIs. + * Wraps the raw headers in the fetch `Headers` shape. Lookups are case-insensitive. + * Multi-value headers are joined with ", ", and names without a value are skipped. */ function wrapRawHeaders(headers: RawHeaders): Headers { - function headerVal(name: string): string | null { - const val = headers[name]; - if (val === undefined || val === null) { - return null; - } - if (Array.isArray(val)) { - return val.join(', '); + const normalized = new Map(); + Object.keys(headers).forEach((key) => { + const val = headers[key]; + if (val !== undefined && val !== null) { + normalized.set(key.toLowerCase(), Array.isArray(val) ? val.join(', ') : val); } - return val; - } - - // We want to use generators here for the simplicity of maintaining - // this interface. Also they aren't expected to be high frequency usage. - function* entries(): Iterable<[string, string]> { - for (const key of Object.keys(headers)) { - const val = headerVal(key); - if (val !== null) { - yield [key, val]; - } - } - } - - function* values(): Iterable { - for (const [, val] of entries()) { - yield val; - } - } + }); return { - get: headerVal, - keys: () => Object.keys(headers), - values, - entries, - // The callback receives the value first. The order matches the fetch `Headers.forEach` - // signature. Multi-value headers are joined with a comma, and headers without a value - // are skipped, like `entries`. + get: (name: string) => normalized.get(name.toLowerCase()) ?? null, + keys: () => normalized.keys(), + values: () => normalized.values(), + entries: () => normalized.entries(), forEach: (callback: (value: string, key: string) => void) => { - for (const [key, value] of entries()) { - callback(value, key); - } + normalized.forEach((value, key) => callback(value, key)); }, - has: (name: string) => Object.prototype.hasOwnProperty.call(headers, name), + has: (name: string) => normalized.has(name.toLowerCase()), }; } @@ -105,9 +89,12 @@ export interface StreamingResponse extends Response { } /** - * Creates a response for a streaming request. It does not buffer the body: each read hands - * out the next chunk from the source iterator. The `text` and `json` methods read the - * remainder of the stream, so a caller uses either the reader or those methods, not both. + * Creates a response for a streaming request. The response does not buffer the body. Each + * read hands out the next chunk from the source iterator. The `text` and `json` methods + * read the remainder of the stream, so a caller uses either the reader or those methods, + * not both. The `cancel` methods on the body and the reader call the iterator's `return` + * method when one exists. The body passes through as-is: the caller is responsible for any + * content-encoding decoding before the source reaches this function. * * @param source The raw platform response, for example a Node `IncomingMessage`. A missing * status code maps to 0, and a missing status message maps to an empty string. @@ -136,6 +123,10 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream return new TextDecoder().decode(content); } + const cancel = async (): Promise => { + await iterator.return?.(); + }; + return { headers: wrapRawHeaders(source.headers), status: source.statusCode ?? 0, @@ -149,7 +140,9 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream } return { done: false, value: next.value }; }, + cancel, }), + cancel, }, text, json: async () => JSON.parse(await text()), From 347ed5c77b361e8f536ab1c4f8e47c58b2cf6f5e Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 12 Oct 2026 12:35:47 -0400 Subject: [PATCH 09/16] fix: make streaming response cancel and destroy to harden request release. --- .../internal/fetch/StreamingResponse.test.ts | 21 +++++++++++++++- .../src/internal/fetch/StreamingResponse.ts | 24 ++++++++++++++----- 2 files changed, 38 insertions(+), 7 deletions(-) diff --git a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts index 97f77b31b8..ef9c395658 100644 --- a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts +++ b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts @@ -1,4 +1,4 @@ -import { Readable } from 'node:stream'; +import { PassThrough, Readable } from 'node:stream'; import { createStreamingResponse, @@ -191,3 +191,22 @@ it('destroys a real Node readable stream when the caller cancels mid-stream', as await reader.cancel?.(); expect(readable.destroyed).toBe(true); }); + +it('destroys a real Node readable stream when the caller cancels with a read in progress', async () => { + const stalled = new PassThrough(); + const source: StreamingResponseSource = Object.assign(stalled, { headers: {} }); + const res = createStreamingResponse(source); + const reader = res.body.getReader(); + const pending = reader.read(); + await reader.cancel?.(); + expect(stalled.destroyed).toBe(true); + await expect(pending).rejects.toThrow('Premature close'); +}); + +it('destroys a real Node readable stream when the caller cancels before reading', async () => { + const readable = Readable.from([Buffer.from('never read')]); + const source: StreamingResponseSource = Object.assign(readable, { headers: {} }); + const res = createStreamingResponse(source); + await res.body.cancel?.(); + expect(readable.destroyed).toBe(true); +}); diff --git a/packages/shared/common/src/internal/fetch/StreamingResponse.ts b/packages/shared/common/src/internal/fetch/StreamingResponse.ts index b15a2811ae..d3354740d0 100644 --- a/packages/shared/common/src/internal/fetch/StreamingResponse.ts +++ b/packages/shared/common/src/internal/fetch/StreamingResponse.ts @@ -24,8 +24,8 @@ export interface ResponseChunkIterator { next(): Promise<{ done?: boolean; value?: Uint8Array }>; /** - * Releases the underlying stream. A Node `Readable` iterator destroys the stream when - * this method runs before the stream ends. + * Releases the iterator. A Node `Readable` iterator queues this call behind a pending + * read, so this call alone does not release an idle stream. */ return?(): Promise<{ done?: boolean; value?: Uint8Array }>; } @@ -52,6 +52,13 @@ export type StreamingResponseSource = { headers: RawHeaders; statusCode?: number; statusMessage?: string; + + /** + * Destroys the underlying stream. When the source provides this method, `cancel` uses + * it to release the connection even while a read is in progress. A Node `Readable` + * provides it. + */ + destroy?(): void; } & { [K in AsyncIteratorKey]: () => ResponseChunkIterator }; /** @@ -92,9 +99,9 @@ export interface StreamingResponse extends Response { * Creates a response for a streaming request. The response does not buffer the body. Each * read hands out the next chunk from the source iterator. The `text` and `json` methods * read the remainder of the stream, so a caller uses either the reader or those methods, - * not both. The `cancel` methods on the body and the reader call the iterator's `return` - * method when one exists. The body passes through as-is: the caller is responsible for any - * content-encoding decoding before the source reaches this function. + * not both. The `cancel` methods on the body and the reader destroy the source when it + * provides a `destroy` method, and also call the iterator's `return` method when one + * exists. The body passes through as-is, so the caller decodes any content encoding first. * * @param source The raw platform response, for example a Node `IncomingMessage`. A missing * status code maps to 0, and a missing status message maps to an empty string. @@ -124,7 +131,12 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream } const cancel = async (): Promise => { - await iterator.return?.(); + // Destroy the stream first. That rejects a pending read and unblocks a queued return + // call. A Node readable iterator queues the return call behind a pending read, and on + // a stream that was never read it completes without a destroy, so the return call + // alone cannot release the connection. Do not await it for the same reason. + source.destroy?.(); + iterator.return?.().catch(() => {}); }; return { From 2cb16b2dfc9303a7d4238efd9c5bc0ccea1f32ab Mon Sep 17 00:00:00 2001 From: joker23 <2494686+joker23@users.noreply.github.com> Date: Mon, 12 Oct 2026 17:30:26 -0400 Subject: [PATCH 10/16] Apply suggestion from @tanderson-ld Co-authored-by: Todd Anderson <127344469+tanderson-ld@users.noreply.github.com> --- packages/shared/common/src/api/platform/Requests.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/shared/common/src/api/platform/Requests.ts b/packages/shared/common/src/api/platform/Requests.ts index b394dc7106..bc12b4c153 100644 --- a/packages/shared/common/src/api/platform/Requests.ts +++ b/packages/shared/common/src/api/platform/Requests.ts @@ -86,7 +86,7 @@ export interface ResponseBody { getReader(): BodyReader; /** - * Releases the body without reading it. + * Releases the body. */ cancel?(): Promise | void; } From b9db255eb54ea08361d0d1f25efbde68ce8d9b9a Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 12 Oct 2026 17:16:00 -0400 Subject: [PATCH 11/16] fix: validate the stream content type through header get method --- .../common/src/api/platform/Requests.ts | 4 +- .../__tests__/EventSource.close.test.ts | 2 + .../EventSource.fetchInjection.test.ts | 245 +++++++++--------- .../__tests__/EventSource.throughput.test.ts | 1 + .../eventsource/__tests__/httpHelpers.test.ts | 2 +- .../shared/eventsource/src/EventSource.ts | 21 +- .../shared/eventsource/src/httpHelpers.ts | 20 +- 7 files changed, 157 insertions(+), 138 deletions(-) diff --git a/packages/shared/common/src/api/platform/Requests.ts b/packages/shared/common/src/api/platform/Requests.ts index bc12b4c153..fe89ef726e 100644 --- a/packages/shared/common/src/api/platform/Requests.ts +++ b/packages/shared/common/src/api/platform/Requests.ts @@ -55,8 +55,8 @@ export interface Headers { /** * Executes the callback once for each header, with the value first. Optional so existing * implementations remain valid. A streaming implementation must provide it. The streaming - * client reads headers only through this method, and content-type validation stops working - * without it. + * client forwards response headers to its open and error events only through this method, + * so without it those events carry no headers. * * https://developer.mozilla.org/en-US/docs/Web/API/Headers/forEach */ diff --git a/packages/shared/eventsource/__tests__/EventSource.close.test.ts b/packages/shared/eventsource/__tests__/EventSource.close.test.ts index 101da940c2..4715fbce1c 100644 --- a/packages/shared/eventsource/__tests__/EventSource.close.test.ts +++ b/packages/shared/eventsource/__tests__/EventSource.close.test.ts @@ -129,6 +129,7 @@ it('aborts the request when the error filter declines a retry mid-stream', async status: 200, statusText: 'OK', headers: { + get: () => 'text/event-stream', forEach(callback: (value: string, key: string) => void): void { callback('text/event-stream', 'content-type'); }, @@ -237,6 +238,7 @@ it('closes cleanly and aborts the request when the error filter throws mid-strea status: 200, statusText: 'OK', headers: { + get: () => 'text/event-stream', forEach(callback: (value: string, key: string) => void): void { callback('text/event-stream', 'content-type'); }, diff --git a/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts b/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts index b04b199f64..1fbb7e607e 100644 --- a/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts +++ b/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts @@ -23,6 +23,25 @@ import { writeEvents, } from './helpers'; +/** + * Builds a FetchHeaders over a record, like the SDK platform adapters do: `get` folds case, and + * `forEach` reports the record's own casing. + */ +function recordHeaders(record: Record): FetchHeaders { + return { + get: (name: string) => { + const match = Object.keys(record).find((key) => key.toLowerCase() === name.toLowerCase()); + return match === undefined ? null : record[match]; + }, + forEach: (callback: (value: string, key: string) => void) => { + Object.entries(record).forEach(([key, value]) => callback(value, key)); + }, + }; +} + +/** The headers of an ordinary SSE response. */ +const sseHeaders = (): FetchHeaders => recordHeaders({ 'content-type': 'text/event-stream' }); + /** * A canned response whose body hands out the given chunks and then stays pending forever, like an * idle SSE connection. The shape matches what the Node SDK http/https adapters produce. @@ -33,11 +52,7 @@ function idleStreamResponse(chunks: string[], headers?: FetchHeaders): FetchLike return { status: 200, statusText: 'OK', - headers: headers ?? { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: headers ?? sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -176,7 +191,7 @@ it('reports a non-200 response from an injected fetch as an error', async () => const injected: FetchLike = async () => ({ status: 401, statusText: 'Unauthorized', - headers: { forEach: () => {} }, + headers: recordHeaders({}), body: null, }); const url = `http://localhost:${deliberatelyUnusedPort}/stream`; @@ -192,7 +207,8 @@ it('reports a non-200 response from an injected fetch as an error', async () => }); it('accepts a response whose headers iterate no entries', async () => { - const injected: FetchLike = async () => idleStreamResponse(['data: hello\n\n'], { forEach() {} }); + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], { get: () => null, forEach() {} }); const url = `http://localhost:${deliberatelyUnusedPort}/stream`; const es = createEventSource(url, { fetch: injected }); es.onerror = () => {}; @@ -207,7 +223,42 @@ it('accepts a response whose headers iterate no entries', async () => { it('opens the stream when the response headers expose no forEach', async () => { // A transport without header iteration reports no headers. A 200 with no headers is accepted, // so the stream must open rather than fail as unreadable. - const injected: FetchLike = async () => idleStreamResponse(['data: hello\n\n'], {}); + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], { get: () => null }); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const es = createEventSource(url, { fetch: injected }); + es.onerror = () => {}; + try { + const messages = startMessageQueue(es); + expect((await messages.take()).data).toEqual('hello'); + } finally { + es.close(); + } +}); + +it('rejects a wrong declared content type from a transport without forEach', async () => { + // Content-type validation reads through get, so it works for a transport whose headers + // expose no iteration. + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], { get: () => 'application/json' }); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const errors = new AsyncQueue(); + const es = createEventSource(url, { fetch: injected, initialRetryDelayMillis: 60000 }); + es.onerror = (e) => errors.add(e); + try { + const err = await errors.take(); + expect(err?.message).toContain('unexpected Content-Type'); + } finally { + es.close(); + } +}); + +it('opens the stream when get returns undefined for an absent header', async () => { + // A sloppy transport can implement get as a plain record lookup, which returns undefined + // instead of null. That reads as no declared type, not as a failure. + const sloppy = { get: () => undefined }; + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], sloppy as unknown as FetchHeaders); const url = `http://localhost:${deliberatelyUnusedPort}/stream`; const es = createEventSource(url, { fetch: injected }); es.onerror = () => {}; @@ -222,7 +273,7 @@ it('opens the stream when the response headers expose no forEach', async () => { it('reports an empty message for a non-200 response without a statusText', async () => { const injected: FetchLike = async () => ({ status: 503, - headers: {}, + headers: { get: () => null }, body: null, }); const url = `http://localhost:${deliberatelyUnusedPort}/stream`; @@ -245,7 +296,7 @@ it('aborts the request when a 200 response has no body', async () => { return { status: 200, statusText: 'OK', - headers: { forEach: () => {} }, + headers: recordHeaders({}), body: null, }; }; @@ -266,12 +317,10 @@ it('folds response header names to lowercase for a transport that reports wire c // A standard Headers object reports lowercase names; a raw-header transport reports the wire // case. Consumers look headers up by lowercase name, so both must produce the same shape. const injected: FetchLike = async () => - idleStreamResponse(['data: hello\n\n'], { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'Content-Type'); - callback('abc', 'X-LD-EnvId'); - }, - }); + idleStreamResponse( + ['data: hello\n\n'], + recordHeaders({ 'Content-Type': 'text/event-stream', 'X-LD-EnvId': 'abc' }), + ); const url = `http://localhost:${deliberatelyUnusedPort}/stream`; const es = createEventSource(url, { fetch: injected }); es.onerror = () => {}; @@ -292,11 +341,7 @@ it('cancels the body reader on close for a transport that ignores the abort sign const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => { let delivered = false; @@ -333,11 +378,7 @@ it('yields to timers while a transport resolves reads synchronously', async () = const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -375,11 +416,7 @@ it('treats a stream of empty chunks as dead when the read timeout elapses', asyn const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -420,11 +457,7 @@ it('does not create a reader when onopen closes the stream', async () => { const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader, cancel: bodyCancel }, }); const url = `http://localhost:${deliberatelyUnusedPort}/stream`; @@ -448,17 +481,13 @@ it('releases the unread body of a rejected response', async () => { { status: 503, statusText: 'Service Unavailable', - headers: { forEach: () => {} }, + headers: recordHeaders({}), body: { getReader: () => ({ read: () => new Promise(() => {}) }), cancel: bodyCancel }, }, { status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/html', 'content-type'); - }, - }, + headers: recordHeaders({ 'content-type': 'text/html' }), body: { getReader: () => ({ read: () => new Promise(() => {}) }), cancel: bodyCancel }, }, ]; @@ -498,7 +527,7 @@ it('releases the body of a response that resolves after close', async () => { resolveFetch!({ status: 200, statusText: 'OK', - headers: { forEach: () => {} }, + headers: recordHeaders({}), body: { getReader: () => ({ read: () => new Promise(() => {}) }), cancel: bodyCancel }, }); // The response reaches the stale callback on the microtask queue. The timer hop runs after it. @@ -516,11 +545,7 @@ it('delivers chunks a transport supplies as ArrayBuffer', async () => { const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -564,11 +589,7 @@ it('delivers chunks from an ArrayBuffer created in another realm', async () => { const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -607,11 +628,7 @@ it('reports the fallback message when a read rejection hides its own message', a return { status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -666,11 +683,7 @@ it('delivers chunks a transport supplies as SharedArrayBuffer', async () => { const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -706,11 +719,7 @@ it('delivers an ArrayBuffer chunk that carries its own string tag', async () => const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -742,11 +751,7 @@ it('reports an error for a chunk that forges the buffer brand', async () => { const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => ({ @@ -771,7 +776,7 @@ it('reports an error for a chunk that forges the buffer brand', async () => { it('keeps the status when a response statusText getter throws', async () => { const hostile = { status: 503, - headers: { forEach: () => {} }, + headers: recordHeaders({}), body: null, }; Object.defineProperty(hostile, 'statusText', { @@ -799,7 +804,7 @@ it('releases the request when a hostile response getter escapes the callback', a signal = init.signal; const hostile = { statusText: 'OK', - headers: { forEach: () => {} }, + headers: recordHeaders({}), body: null, }; Object.defineProperty(hostile, 'status', { @@ -824,12 +829,13 @@ it('releases the request when a hostile response getter escapes the callback', a }); it('fails a 200 response whose headers cannot be read', async () => { - // A forEach that delivers a wrong content type and then throws must not open the stream as a - // headerless success with its partial headers discarded. + // A forEach that throws must not open the stream as a headerless success with its partial + // headers discarded, even when get works and declares a valid content type. const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', headers: { + get: () => 'text/event-stream', forEach(callback: (value: string, key: string) => void): void { callback('text/html', 'content-type'); throw new Error('headers boom'); @@ -849,6 +855,33 @@ it('fails a 200 response whose headers cannot be read', async () => { } }); +it('fails a 200 response whose headers get throws', async () => { + // The declared content type gates the stream, so a throwing get must fail and retry. + const injected: FetchLike = async () => ({ + status: 200, + statusText: 'OK', + headers: { + get: () => { + throw new Error('get boom'); + }, + forEach(callback: (value: string, key: string) => void): void { + callback('text/event-stream', 'content-type'); + }, + }, + body: { getReader: () => ({ read: () => new Promise(() => {}) }) }, + }); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const es = createEventSource(url, { fetch: injected, initialRetryDelayMillis: 60000 }); + const events = new AsyncQueue(); + es.addEventListener('open', () => events.add('open')); + es.onerror = (e) => events.add(`error:${e?.message}`); + try { + expect(await events.take()).toEqual('error:stream response headers could not be read'); + } finally { + es.close(); + } +}); + it('cancels the reader when getReader itself closes the stream', async () => { // The body getter and getReader() are the last caller code before the read loop. A close() // from inside either one must not leave an uncancelled reader on the closed stream. @@ -858,11 +891,7 @@ it('cancels the reader when getReader itself closes the stream', async () => { const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => { es?.close(); @@ -896,6 +925,7 @@ it('stays closed when a transport accessor calls close during the response callb status: 200, statusText: 'OK', headers: { + get: () => 'text/event-stream', forEach(): void { es?.close(); }, @@ -928,7 +958,7 @@ it('reads the response status once for the failure report', async () => { let reads = 0; const hostile = { statusText: 'Service Unavailable', - headers: { forEach: () => {} }, + headers: recordHeaders({}), body: null, }; Object.defineProperty(hostile, 'status', { @@ -965,6 +995,7 @@ it('keeps the status when a non-200 response headers forEach throws', async () = status: 401, statusText: 'Unauthorized', headers: { + get: () => null, forEach(): void { throw new Error('headers boom'); }, @@ -1010,11 +1041,7 @@ it('ignores a stale fetch rejection after a newer attempt is streaming', async ( return Promise.resolve({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -1094,11 +1121,7 @@ it('reports an error instead of stalling when a transport supplies a non-buffer const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => ({ @@ -1131,11 +1154,7 @@ it('keeps the read timeout alive across a partial multi-byte chunk', async () => const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -1176,11 +1195,7 @@ it('releases the request when getReader throws an error whose message getter thr return { status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => { const hostile = {}; @@ -1219,11 +1234,7 @@ it('releases the request when getReader throws', async () => { return { status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => { throw new Error('stream locked'); @@ -1253,11 +1264,7 @@ it('close() tolerates a reader cancel that throws synchronously', async () => { const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => { let delivered = false; @@ -1297,11 +1304,7 @@ it('aborts the failed request when a retried read error schedules a reconnect', return { status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { @@ -1341,11 +1344,7 @@ it('recomputes the message origin when urlBuilder picks a new origin for a recon return { status: 200, statusText: 'OK', - headers: { - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, - }, + headers: sseHeaders(), body: { getReader: () => ({ read: async (): Promise<{ done: boolean; value?: Uint8Array }> => { diff --git a/packages/shared/eventsource/__tests__/EventSource.throughput.test.ts b/packages/shared/eventsource/__tests__/EventSource.throughput.test.ts index 4678789f6f..0d20f3f036 100644 --- a/packages/shared/eventsource/__tests__/EventSource.throughput.test.ts +++ b/packages/shared/eventsource/__tests__/EventSource.throughput.test.ts @@ -12,6 +12,7 @@ function singleChunkResponse(chunk: Uint8Array): FetchLikeResponse { status: 200, statusText: 'OK', headers: { + get: () => 'text/event-stream', forEach(callback: (value: string, key: string) => void): void { callback('text/event-stream', 'content-type'); }, diff --git a/packages/shared/eventsource/__tests__/httpHelpers.test.ts b/packages/shared/eventsource/__tests__/httpHelpers.test.ts index 6da1cf2ec6..be8698c260 100644 --- a/packages/shared/eventsource/__tests__/httpHelpers.test.ts +++ b/packages/shared/eventsource/__tests__/httpHelpers.test.ts @@ -40,5 +40,5 @@ it('keeps empty parts', () => { }); it('returns an empty object when the headers expose no forEach', () => { - expect(headersToObject({})).toEqual({}); + expect(headersToObject({ get: () => null })).toEqual({}); }); diff --git a/packages/shared/eventsource/src/EventSource.ts b/packages/shared/eventsource/src/EventSource.ts index c158e04a7b..c6ebe250c2 100644 --- a/packages/shared/eventsource/src/EventSource.ts +++ b/packages/shared/eventsource/src/EventSource.ts @@ -625,9 +625,15 @@ export function createEventSource( // non-200 status would be lost with the throw, and the error filter could not classify // the failure. let responseHeaders: Record; + let contentType: string | undefined; let headersUnreadable = false; try { responseHeaders = headersToObject(res.headers); + const declaredType: unknown = res.headers.get('content-type'); + // A standard get returns a string or null. A sloppy transport can return undefined for + // an absent header, and a hostile one can return anything. Only a string is a declared + // type; every other value reads as no declared type. + contentType = typeof declaredType === 'string' ? declaredType : undefined; } catch { responseHeaders = {}; headersUnreadable = true; @@ -663,9 +669,9 @@ export function createEventSource( } if (headersUnreadable) { - // On a success status the headers decide whether the body is a stream at all. A 200 - // whose headers cannot be read must fail and retry, not open as a headerless stream - // with its partially collected headers discarded. + // On a success status the headers feed the open event and the declared content type + // gates the stream. A 200 whose headers throw during the read must fail and retry, not + // open with its partially collected headers discarded. failOnce({ message: 'stream response headers could not be read' }); releaseBody(res); return; @@ -677,14 +683,13 @@ export function createEventSource( // transport can report no headers. // // The media type must match exactly. Parameters after it, such as a charset, are allowed. - // A standard Headers object comma-joins a duplicated Content-Type header, so each part of - // a joined value must declare the event-stream media type. This is stricter than the - // Fetch algorithm, which keeps only the last parsable part, and an empty or unparsable - // part also fails. + // A standard get comma-joins a duplicated Content-Type header, so each part of a joined + // value must declare the event-stream media type. This is stricter than the Fetch + // algorithm, which keeps only the last parsable part, and an empty or unparsable part + // also fails. // // trim() also strips NBSP, form feed, and vertical tab, which are not HTTP whitespace. // The extra leniency is harmless. - const contentType = responseHeaders['content-type']; if ( contentType !== undefined && !splitHeaderListValue(contentType).every( diff --git a/packages/shared/eventsource/src/httpHelpers.ts b/packages/shared/eventsource/src/httpHelpers.ts index 073bdb0abf..a311972773 100644 --- a/packages/shared/eventsource/src/httpHelpers.ts +++ b/packages/shared/eventsource/src/httpHelpers.ts @@ -11,11 +11,23 @@ */ /** - * The response headers. `forEach` is the only member this package reads. It is optional so a - * transport whose response headers expose no iteration can still satisfy the type. The client - * then treats the response as carrying no headers. + * The response headers. The member set mirrors the subset of the standard `Headers` class that + * this package reads. */ export interface FetchHeaders { + /** + * Returns the value of a header, or `null` when the response carries no such header. The + * lookup is case-insensitive, like the standard `Headers.get`. The client validates the + * declared content type through it. + */ + get(name: string): string | null; + + /** + * Executes the callback once for each header, with the value first. It is optional so a + * transport whose response headers expose no iteration can still satisfy the type. The client + * forwards headers to its open and error events only through this method, so without it those + * events carry no headers. + */ forEach?(callback: (value: string, key: string) => void): void; } @@ -100,7 +112,7 @@ export const bodylessMethods = ['GET', 'HEAD']; export function headersToObject(headers: FetchHeaders): Record { const result: Record = {}; if (!headers.forEach) { - // A transport without header iteration reports no headers. The client treats that the same + // A transport without header iteration forwards no headers. The client treats that the same // as an empty header set. return result; } From d9113ec32177e0b2733fa27b4dde45cc7b8c5dcb Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 12 Oct 2026 17:45:05 -0400 Subject: [PATCH 12/16] fix: Harden the streaming response body reads and release paths --- .../internal/fetch/StreamingResponse.test.ts | 94 +++++++++++++++ .../src/internal/fetch/StreamingResponse.ts | 112 ++++++++++++------ 2 files changed, 170 insertions(+), 36 deletions(-) diff --git a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts index ef9c395658..3ad0655878 100644 --- a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts +++ b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts @@ -90,6 +90,16 @@ it('matches header names without case sensitivity', () => { expect(Array.from(res.headers.keys())).toEqual(['content-type']); }); +it('joins the values of header names that differ only by case', () => { + const res = createStreamingResponse( + makeTextSource([], { + headers: { 'Content-Type': 'text/html', 'content-type': 'text/event-stream' }, + statusCode: 200, + }), + ); + expect(res.headers.get('content-type')).toBe('text/html, text/event-stream'); +}); + it('reports a header without a value as absent', () => { const res = createStreamingResponse( makeTextSource([], { headers: { empty: undefined }, statusCode: 200 }), @@ -136,6 +146,57 @@ it('parses the remaining stream as JSON', async () => { await expect(res.json()).resolves.toEqual({ a: 1 }); }); +it('decodes ArrayBuffer and DataView chunks in text', async () => { + const bytes = utf8('buffer view'); + const asBuffer = bytes.slice(0, 7).buffer as unknown as Uint8Array; + const tail = bytes.slice(7); + const asView = new DataView( + tail.buffer, + tail.byteOffset, + tail.byteLength, + ) as unknown as Uint8Array; + const res = createStreamingResponse(makeSource([asBuffer, asView])); + await expect(res.text()).resolves.toBe('buffer view'); +}); + +it('rejects a string chunk in text instead of corrupting the result', async () => { + const res = createStreamingResponse(makeSource(['data: hi\n\n' as unknown as Uint8Array])); + const err = await res.text().then( + () => undefined, + (e) => e, + ); + expect(err).toBeInstanceOf(TypeError); + expect(String(err)).toContain('BufferSource'); + // The chunk contents must never leak into the error message. + expect(String(err)).not.toContain('data: hi'); +}); + +it('throws a named error when the source has no async iterator', () => { + const source = { headers: {} } as unknown as StreamingResponseSource; + expect(() => createStreamingResponse(source)).toThrow( + 'the streaming response source does not provide an async iterator', + ); +}); + +it('throws when a second reader is requested', () => { + const res = createStreamingResponse(makeTextSource(['chunk'])); + res.body.getReader(); + expect(() => res.body.getReader()).toThrow('already in use'); +}); + +it('rejects text after a reader was created', async () => { + const res = createStreamingResponse(makeTextSource(['chunk'])); + res.body.getReader(); + await expect(res.text()).rejects.toThrow('already in use'); +}); + +it('throws when a reader is requested after text started', async () => { + const res = createStreamingResponse(makeTextSource(['chunk'])); + const pending = res.text(); + expect(() => res.body.getReader()).toThrow('already in use'); + await expect(pending).resolves.toBe('chunk'); +}); + it('rejects a pending read when the stream errors', async () => { const erroring: StreamingResponseSource = { headers: {}, @@ -169,6 +230,39 @@ it('tolerates cancel when the iterator has no return method', async () => { await expect(Promise.resolve(res.body.cancel?.())).resolves.toBeUndefined(); }); +it('still releases the iterator when destroy throws', async () => { + const returnSpy = jest.fn(async () => ({ done: true })); + const source: StreamingResponseSource = { + headers: {}, + destroy: () => { + throw new Error('destroy boom'); + }, + [Symbol.asyncIterator]: () => ({ + next: async () => ({ done: false, value: utf8('chunk') }), + return: returnSpy, + }), + }; + const res = createStreamingResponse(source); + await expect(Promise.resolve(res.body.cancel?.())).resolves.toBeUndefined(); + expect(returnSpy).toHaveBeenCalledTimes(1); +}); + +it('tolerates an iterator whose return resolves synchronously', async () => { + const destroySpy = jest.fn(); + const source: StreamingResponseSource = { + headers: {}, + destroy: destroySpy, + [Symbol.asyncIterator]: () => ({ + next: async () => ({ done: false, value: utf8('chunk') }), + // @ts-ignore deliberately violates the declared Promise-returning shape. + return: () => ({ done: true }), + }), + }; + const res = createStreamingResponse(source); + await expect(Promise.resolve(res.body.cancel?.())).resolves.toBeUndefined(); + expect(destroySpy).toHaveBeenCalledTimes(1); +}); + it('accepts a real Node readable stream as the source', async () => { const readable = Readable.from([Buffer.from('data: '), Buffer.from('hello\n\n')]); const source: StreamingResponseSource = Object.assign(readable, { diff --git a/packages/shared/common/src/internal/fetch/StreamingResponse.ts b/packages/shared/common/src/internal/fetch/StreamingResponse.ts index d3354740d0..6f43e104a2 100644 --- a/packages/shared/common/src/internal/fetch/StreamingResponse.ts +++ b/packages/shared/common/src/internal/fetch/StreamingResponse.ts @@ -3,7 +3,9 @@ import type { Headers, Response, ResponseBody } from '../../api/platform/Request // Each platform that constructs a streaming response provides the TextDecoder global. This // package compiles without the global type libraries, so this scoped declaration stands in // for it. -declare const TextDecoder: new () => { decode(input?: Uint8Array): string }; +declare const TextDecoder: new () => { + decode(input?: Uint8Array, options?: { stream?: boolean }): string; +}; /** * The shape of raw HTTP headers: names mapped to a value, a list of values, or no value. @@ -19,7 +21,8 @@ export type RawHeaders = Record; export interface ResponseChunkIterator { /** * Resolves with the next chunk, or with `done: true` when the stream ends. Rejects when - * the connection drops. + * the connection terminates before the server ends the stream. A chunk delivered together + * with `done: true` is discarded, so the iterator must deliver the last chunk on its own. */ next(): Promise<{ done?: boolean; value?: Uint8Array }>; @@ -54,23 +57,29 @@ export type StreamingResponseSource = { statusMessage?: string; /** - * Destroys the underlying stream. When the source provides this method, `cancel` uses - * it to release the connection even while a read is in progress. A Node `Readable` - * provides it. + * Destroys the underlying stream. `cancel` uses it to release the connection even while + * a read is in progress. A Node `Readable` provides it. A source without it must release + * through the iterator `return` method; a source whose `return` queues behind a pending + * read, like a Node readable iterator, must provide `destroy` or `cancel` cannot release + * it mid-read. */ destroy?(): void; } & { [K in AsyncIteratorKey]: () => ResponseChunkIterator }; /** * Wraps the raw headers in the fetch `Headers` shape. Lookups are case-insensitive. - * Multi-value headers are joined with ", ", and names without a value are skipped. + * Multi-value headers are joined with ", ", names that differ only by case merge into one + * joined value, and names without a value are skipped. */ function wrapRawHeaders(headers: RawHeaders): Headers { const normalized = new Map(); Object.keys(headers).forEach((key) => { const val = headers[key]; if (val !== undefined && val !== null) { - normalized.set(key.toLowerCase(), Array.isArray(val) ? val.join(', ') : val); + const name = key.toLowerCase(); + const joined = Array.isArray(val) ? val.join(', ') : val; + const existing = normalized.get(name); + normalized.set(name, existing === undefined ? joined : `${existing}, ${joined}`); } }); @@ -97,46 +106,74 @@ export interface StreamingResponse extends Response { /** * Creates a response for a streaming request. The response does not buffer the body. Each - * read hands out the next chunk from the source iterator. The `text` and `json` methods - * read the remainder of the stream, so a caller uses either the reader or those methods, - * not both. The `cancel` methods on the body and the reader destroy the source when it - * provides a `destroy` method, and also call the iterator's `return` method when one + * read hands out the next chunk from the source iterator. The body permits one consumer: + * the first `getReader`, `text`, or `json` call claims it, and a later call of any of the + * three throws, like the lock of a standard body. The `cancel` methods on the body and the + * reader call every release function the source provides: they destroy the source when it + * has a `destroy` method, and they also call the iterator's `return` method when one * exists. The body passes through as-is, so the caller decodes any content encoding first. * * @param source The raw platform response, for example a Node `IncomingMessage`. A missing * status code maps to 0, and a missing status message maps to an empty string. */ export function createStreamingResponse(source: StreamingResponseSource): StreamingResponse { + // A consumer compiled without `Symbol.asyncIterator` in its lib can pass a source that + // lacks the iterator without a compile error. The check turns that mistake into a named + // failure instead of a bare member-access TypeError. + if (typeof source[Symbol.asyncIterator] !== 'function') { + throw new TypeError('the streaming response source does not provide an async iterator'); + } const iterator = source[Symbol.asyncIterator](); + + let bodyUsed = false; + const claimBody = (): void => { + if (bodyUsed) { + throw new TypeError('the response body is already in use'); + } + bodyUsed = true; + }; + async function text(): Promise { - const chunks: Uint8Array[] = []; - let length = 0; + claimBody(); + const decoder = new TextDecoder(); + let result = ''; let next = await iterator.next(); while (!next.done) { - if (next.value) { - chunks.push(next.value); - length += next.value.length; + if (next.value !== undefined) { + try { + result += decoder.decode(next.value, { stream: true }); + } catch { + // decode() is the BufferSource validator, like on the reader path in the + // streaming client. The engine's message can quote the chunk contents, and + // stream data must not leak into error messages and logs. + throw new TypeError('the source supplied a chunk that is not a BufferSource'); + } } // The stream provides the chunks in sequence, so the reads cannot run in parallel. // eslint-disable-next-line no-await-in-loop next = await iterator.next(); } - const content = new Uint8Array(length); - let offset = 0; - chunks.forEach((chunk) => { - content.set(chunk, offset); - offset += chunk.length; - }); - return new TextDecoder().decode(content); + // The final decode flushes a trailing partial multi-byte sequence. + return result + decoder.decode(); } const cancel = async (): Promise => { // Destroy the stream first. That rejects a pending read and unblocks a queued return // call. A Node readable iterator queues the return call behind a pending read, and on // a stream that was never read it completes without a destroy, so the return call - // alone cannot release the connection. Do not await it for the same reason. - source.destroy?.(); - iterator.return?.().catch(() => {}); + // alone cannot release the connection. Do not await it for the same reason. Each + // release call is guarded on its own, so a release function that throws, or a return + // that resolves synchronously, does not skip the other release path or reject cancel. + try { + source.destroy?.(); + } catch { + // A throwing destroy has nothing left to release. + } + try { + Promise.resolve(iterator.return?.()).catch(() => {}); + } catch { + // A throwing return has nothing left to release. + } }; return { @@ -144,16 +181,19 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream status: source.statusCode ?? 0, statusText: source.statusMessage ?? '', body: { - getReader: () => ({ - read: async () => { - const next = await iterator.next(); - if (next.done) { - return { done: true }; - } - return { done: false, value: next.value }; - }, - cancel, - }), + getReader: () => { + claimBody(); + return { + read: async () => { + const next = await iterator.next(); + if (next.done) { + return { done: true }; + } + return { done: false, value: next.value }; + }, + cancel, + }; + }, cancel, }, text, From 7d53a2ec1bd9f26e3ef582c45715debc6a0a5de0 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 12 Oct 2026 17:45:05 -0400 Subject: [PATCH 13/16] docs: Simplify the platform streaming type comments --- .../shared/common/src/api/platform/Requests.ts | 14 +++++--------- packages/shared/eventsource/src/httpHelpers.ts | 3 ++- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/packages/shared/common/src/api/platform/Requests.ts b/packages/shared/common/src/api/platform/Requests.ts index fe89ef726e..ac1ba4bfae 100644 --- a/packages/shared/common/src/api/platform/Requests.ts +++ b/packages/shared/common/src/api/platform/Requests.ts @@ -54,9 +54,7 @@ export interface Headers { /** * Executes the callback once for each header, with the value first. Optional so existing - * implementations remain valid. A streaming implementation must provide it. The streaming - * client forwards response headers to its open and error events only through this method, - * so without it those events carry no headers. + * implementations remain valid. * * https://developer.mozilla.org/en-US/docs/Web/API/Headers/forEach */ @@ -69,7 +67,7 @@ export interface Headers { export interface BodyReader { /** * Resolves with the next chunk, or with `done: true` when the server ends the stream. - * Rejects when the connection drops. + * Rejects when the connection terminates before the server ends the stream. */ read(): Promise<{ done: boolean; value?: Uint8Array }>; @@ -166,14 +164,12 @@ export interface Options { signal?: AbortSignalLike; /** - * True for a streaming request. The implementation delivers response chunks with low - * latency. It does not request compressed content, does not buffer the response body, - * and applies no request timeout. The response exposes its body through + * True for a streaming request. The implementation does not buffer the response body, + * and it ignores {@link Options.timeout}. The response exposes its body through * {@link Response.body}. An SDK must use a fetch as a streaming transport only when the * implementation honors this field. A streaming implementation must provide at least one * release path. It honors {@link Options.signal}, or it provides the body cancel. It - * preferably provides both. When `streaming` is true, the implementation ignores - * {@link Options.timeout}. + * preferably provides both. */ streaming?: boolean; } diff --git a/packages/shared/eventsource/src/httpHelpers.ts b/packages/shared/eventsource/src/httpHelpers.ts index a311972773..e37946d1c3 100644 --- a/packages/shared/eventsource/src/httpHelpers.ts +++ b/packages/shared/eventsource/src/httpHelpers.ts @@ -33,7 +33,8 @@ export interface FetchHeaders { /** * A reader over the response body stream. `read` resolves with the next chunk, or with - * `done: true` when the server ends the stream, and rejects when the connection drops. + * `done: true` when the server ends the stream, and rejects when the connection terminates + * before the server ends the stream. */ export interface FetchBodyReader { read(): Promise<{ done: boolean; value?: Uint8Array }>; From 5e1002d568b32eaebbe83744926f681d7a2d482a Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 12 Oct 2026 17:45:05 -0400 Subject: [PATCH 14/16] ci: Tighten the common package size limit to the measured growth --- .github/workflows/common.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/common.yml b/.github/workflows/common.yml index 7c22c2bee2..0d3c1d5820 100644 --- a/.github/workflows/common.yml +++ b/.github/workflows/common.yml @@ -35,4 +35,4 @@ jobs: target_file: 'packages/shared/common/dist/esm/index.mjs' package_name: '@launchdarkly/js-sdk-common' pr_number: ${{ github.event.number }} - size_limit: 35000 + size_limit: 31500 From cd99cba326ef92d03458991c410c2ea53b45f893 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Mon, 12 Oct 2026 18:40:43 -0400 Subject: [PATCH 15/16] fix: keep failure headers and release the body when a streaming read fails --- .../internal/fetch/StreamingResponse.test.ts | 31 ++++++++++++ .../src/internal/fetch/StreamingResponse.ts | 47 ++++++++++--------- .../EventSource.fetchInjection.test.ts | 28 +++++++++++ .../shared/eventsource/src/EventSource.ts | 9 +++- 4 files changed, 91 insertions(+), 24 deletions(-) diff --git a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts index 3ad0655878..5e852890ea 100644 --- a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts +++ b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts @@ -171,6 +171,37 @@ it('rejects a string chunk in text instead of corrupting the result', async () = expect(String(err)).not.toContain('data: hi'); }); +it('destroys the source when text rejects a bad chunk', async () => { + // Readable.from on strings yields string chunks, which the decoder rejects. The + // rejection must release the connection, because text() owns the body. + const readable = Readable.from(['data: hi\n\n']); + const source: StreamingResponseSource = Object.assign(readable, { headers: {} }); + const res = createStreamingResponse(source); + await expect(res.text()).rejects.toThrow('BufferSource'); + expect(readable.destroyed).toBe(true); +}); + +it('keeps the fixed message when the source destroy throws during a bad-chunk release', async () => { + // The release calls inside cancel are guarded. A hostile destroy must not replace the + // fixed BufferSource message, and the iterator return must still run. + const destroySpy = jest.fn(() => { + throw new Error('destroy boom'); + }); + const returnSpy = jest.fn(async () => ({ done: true })); + const source: StreamingResponseSource = { + headers: {}, + destroy: destroySpy, + [Symbol.asyncIterator]: () => ({ + next: async () => ({ done: false, value: 'data: hi\n\n' as unknown as Uint8Array }), + return: returnSpy, + }), + }; + const res = createStreamingResponse(source); + await expect(res.text()).rejects.toThrow('BufferSource'); + expect(destroySpy).toHaveBeenCalledTimes(1); + expect(returnSpy).toHaveBeenCalledTimes(1); +}); + it('throws a named error when the source has no async iterator', () => { const source = { headers: {} } as unknown as StreamingResponseSource; expect(() => createStreamingResponse(source)).toThrow( diff --git a/packages/shared/common/src/internal/fetch/StreamingResponse.ts b/packages/shared/common/src/internal/fetch/StreamingResponse.ts index 6f43e104a2..3debbce835 100644 --- a/packages/shared/common/src/internal/fetch/StreamingResponse.ts +++ b/packages/shared/common/src/internal/fetch/StreamingResponse.ts @@ -133,6 +133,25 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream bodyUsed = true; }; + const cancel = async (): Promise => { + // Destroy the stream first. That rejects a pending read and unblocks a queued return + // call. A Node readable iterator queues the return call behind a pending read, and on + // a stream that was never read it completes without a destroy, so the return call + // alone cannot release the connection. Do not await it for the same reason. Each + // release call is guarded on its own, so a release function that throws, or a return + // that resolves synchronously, does not skip the other release path or reject cancel. + try { + source.destroy?.(); + } catch { + // A throwing destroy has nothing left to release. + } + try { + Promise.resolve(iterator.return?.()).catch(() => {}); + } catch { + // A throwing return has nothing left to release. + } + }; + async function text(): Promise { claimBody(); const decoder = new TextDecoder(); @@ -143,39 +162,21 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream try { result += decoder.decode(next.value, { stream: true }); } catch { - // decode() is the BufferSource validator, like on the reader path in the - // streaming client. The engine's message can quote the chunk contents, and - // stream data must not leak into error messages and logs. + // decode() rejects a chunk that is not a BufferSource. The fixed message keeps + // chunk contents out of error messages. text() owns the body, so a failed read + // must release the source itself. + cancel(); throw new TypeError('the source supplied a chunk that is not a BufferSource'); } } // The stream provides the chunks in sequence, so the reads cannot run in parallel. - // eslint-disable-next-line no-await-in-loop + // oxlint-disable-next-line no-await-in-loop next = await iterator.next(); } // The final decode flushes a trailing partial multi-byte sequence. return result + decoder.decode(); } - const cancel = async (): Promise => { - // Destroy the stream first. That rejects a pending read and unblocks a queued return - // call. A Node readable iterator queues the return call behind a pending read, and on - // a stream that was never read it completes without a destroy, so the return call - // alone cannot release the connection. Do not await it for the same reason. Each - // release call is guarded on its own, so a release function that throws, or a return - // that resolves synchronously, does not skip the other release path or reject cancel. - try { - source.destroy?.(); - } catch { - // A throwing destroy has nothing left to release. - } - try { - Promise.resolve(iterator.return?.()).catch(() => {}); - } catch { - // A throwing return has nothing left to release. - } - }; - return { headers: wrapRawHeaders(source.headers), status: source.statusCode ?? 0, diff --git a/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts b/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts index 1fbb7e607e..a8b74c52c4 100644 --- a/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts +++ b/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts @@ -988,6 +988,34 @@ it('reads the response status once for the failure report', async () => { } }); +it('keeps the headers when a non-200 response has no get', async () => { + // A transport without get fails the content-type lookup. A failure response does not + // need the declared type, so the report must keep the headers that forEach collected, + // including the FDv1 fallback directive. + const headers = { + forEach(callback: (value: string, key: string) => void): void { + callback('true', 'x-ld-fd-fallback'); + }, + } as unknown as FetchHeaders; + const injected: FetchLike = async () => ({ + status: 503, + statusText: 'Service Unavailable', + headers, + body: null, + }); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const errors = new AsyncQueue(); + const es = createEventSource(url, { fetch: injected, errorFilter: () => false }); + es.onerror = (e) => errors.add(e); + try { + const err = await errors.take(); + expect(err?.status).toEqual(503); + expect(err?.headers?.['x-ld-fd-fallback']).toEqual('true'); + } finally { + es.close(); + } +}); + it('keeps the status when a non-200 response headers forEach throws', async () => { // Without the headers fallback, the throw would escape to the fetch catch and the 401 would // be reported with no status, so a permanent auth failure would retry forever. diff --git a/packages/shared/eventsource/src/EventSource.ts b/packages/shared/eventsource/src/EventSource.ts index c6ebe250c2..d5431d4379 100644 --- a/packages/shared/eventsource/src/EventSource.ts +++ b/packages/shared/eventsource/src/EventSource.ts @@ -629,13 +629,20 @@ export function createEventSource( let headersUnreadable = false; try { responseHeaders = headersToObject(res.headers); + } catch { + responseHeaders = {}; + headersUnreadable = true; + } + try { const declaredType: unknown = res.headers.get('content-type'); // A standard get returns a string or null. A sloppy transport can return undefined for // an absent header, and a hostile one can return anything. Only a string is a declared // type; every other value reads as no declared type. contentType = typeof declaredType === 'string' ? declaredType : undefined; } catch { - responseHeaders = {}; + // A missing or throwing get leaves the declared type unknown. Only a success response + // needs the declared type, and the headers read above must survive for the failure + // report. headersUnreadable = true; } From 08ca9995cfe2f6bcd4ab860bcec2209185f92be7 Mon Sep 17 00:00:00 2001 From: Steven Zhang Date: Wed, 14 Oct 2026 11:50:49 -0400 Subject: [PATCH 16/16] fix: Keep header validation working for transports without a usable header get method --- .../internal/fetch/StreamingResponse.test.ts | 34 ++-- .../common/src/api/platform/Requests.ts | 6 +- .../src/internal/fetch/StreamingResponse.ts | 71 ++++---- .../EventSource.fetchInjection.test.ts | 162 +++++++++++++++++- .../eventsource/__tests__/httpHelpers.test.ts | 25 ++- .../shared/eventsource/src/EventSource.ts | 34 +--- .../shared/eventsource/src/httpHelpers.ts | 103 +++++++++-- 7 files changed, 332 insertions(+), 103 deletions(-) diff --git a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts index 5e852890ea..f7dffaf28c 100644 --- a/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts +++ b/packages/shared/common/__tests__/internal/fetch/StreamingResponse.test.ts @@ -209,23 +209,23 @@ it('throws a named error when the source has no async iterator', () => { ); }); -it('throws when a second reader is requested', () => { - const res = createStreamingResponse(makeTextSource(['chunk'])); - res.body.getReader(); - expect(() => res.body.getReader()).toThrow('already in use'); -}); - -it('rejects text after a reader was created', async () => { - const res = createStreamingResponse(makeTextSource(['chunk'])); - res.body.getReader(); - await expect(res.text()).rejects.toThrow('already in use'); -}); - -it('throws when a reader is requested after text started', async () => { - const res = createStreamingResponse(makeTextSource(['chunk'])); - const pending = res.text(); - expect(() => res.body.getReader()).toThrow('already in use'); - await expect(pending).resolves.toBe('chunk'); +it('releases the source when a read fails during text', async () => { + const destroySpy = jest.fn(); + const returnSpy = jest.fn(async () => ({ done: true })); + const source: StreamingResponseSource = { + headers: {}, + destroy: destroySpy, + [Symbol.asyncIterator]: () => ({ + next: async () => { + throw new Error('connection reset'); + }, + return: returnSpy, + }), + }; + const res = createStreamingResponse(source); + await expect(res.text()).rejects.toThrow('connection reset'); + expect(destroySpy).toHaveBeenCalledTimes(1); + expect(returnSpy).toHaveBeenCalledTimes(1); }); it('rejects a pending read when the stream errors', async () => { diff --git a/packages/shared/common/src/api/platform/Requests.ts b/packages/shared/common/src/api/platform/Requests.ts index ac1ba4bfae..13d97cb792 100644 --- a/packages/shared/common/src/api/platform/Requests.ts +++ b/packages/shared/common/src/api/platform/Requests.ts @@ -54,7 +54,8 @@ export interface Headers { /** * Executes the callback once for each header, with the value first. Optional so existing - * implementations remain valid. + * implementations remain valid. The streaming client reads response headers through this + * method, or through {@link Headers.entries} when this method is absent. * * https://developer.mozilla.org/en-US/docs/Web/API/Headers/forEach */ @@ -165,7 +166,8 @@ export interface Options { /** * True for a streaming request. The implementation does not buffer the response body, - * and it ignores {@link Options.timeout}. The response exposes its body through + * it does not request compressed content, and it ignores {@link Options.timeout}. The + * response exposes its body through * {@link Response.body}. An SDK must use a fetch as a streaming transport only when the * implementation honors this field. A streaming implementation must provide at least one * release path. It honors {@link Options.signal}, or it provides the body cancel. It diff --git a/packages/shared/common/src/internal/fetch/StreamingResponse.ts b/packages/shared/common/src/internal/fetch/StreamingResponse.ts index 3debbce835..8bbd4f3ec3 100644 --- a/packages/shared/common/src/internal/fetch/StreamingResponse.ts +++ b/packages/shared/common/src/internal/fetch/StreamingResponse.ts @@ -106,9 +106,9 @@ export interface StreamingResponse extends Response { /** * Creates a response for a streaming request. The response does not buffer the body. Each - * read hands out the next chunk from the source iterator. The body permits one consumer: - * the first `getReader`, `text`, or `json` call claims it, and a later call of any of the - * three throws, like the lock of a standard body. The `cancel` methods on the body and the + * read hands out the next chunk from the source iterator. The body does not enforce a + * consumer lock: every `getReader`, `text`, and `json` call reads from the same iterator, + * so the caller uses one consumer per response. The `cancel` methods on the body and the * reader call every release function the source provides: they destroy the source when it * has a `destroy` method, and they also call the iterator's `return` method when one * exists. The body passes through as-is, so the caller decodes any content encoding first. @@ -125,14 +125,6 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream } const iterator = source[Symbol.asyncIterator](); - let bodyUsed = false; - const claimBody = (): void => { - if (bodyUsed) { - throw new TypeError('the response body is already in use'); - } - bodyUsed = true; - }; - const cancel = async (): Promise => { // Destroy the stream first. That rejects a pending read and unblocks a queued return // call. A Node readable iterator queues the return call behind a pending read, and on @@ -153,25 +145,29 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream }; async function text(): Promise { - claimBody(); const decoder = new TextDecoder(); let result = ''; - let next = await iterator.next(); - while (!next.done) { - if (next.value !== undefined) { - try { - result += decoder.decode(next.value, { stream: true }); - } catch { - // decode() rejects a chunk that is not a BufferSource. The fixed message keeps - // chunk contents out of error messages. text() owns the body, so a failed read - // must release the source itself. - cancel(); - throw new TypeError('the source supplied a chunk that is not a BufferSource'); + try { + let next = await iterator.next(); + while (!next.done) { + if (next.value !== undefined) { + try { + result += decoder.decode(next.value, { stream: true }); + } catch { + // decode() rejects a chunk that is not a BufferSource. The fixed message keeps + // chunk contents out of error messages. + throw new TypeError('the source supplied a chunk that is not a BufferSource'); + } } + // The stream provides the chunks in sequence, so the reads cannot run in parallel. + // oxlint-disable-next-line no-await-in-loop + next = await iterator.next(); } - // The stream provides the chunks in sequence, so the reads cannot run in parallel. - // oxlint-disable-next-line no-await-in-loop - next = await iterator.next(); + } catch (err) { + // text() owns the body, so any failed read must release the source itself. This + // covers a rejecting next() as well as a bad chunk. + cancel(); + throw err; } // The final decode flushes a trailing partial multi-byte sequence. return result + decoder.decode(); @@ -182,19 +178,16 @@ export function createStreamingResponse(source: StreamingResponseSource): Stream status: source.statusCode ?? 0, statusText: source.statusMessage ?? '', body: { - getReader: () => { - claimBody(); - return { - read: async () => { - const next = await iterator.next(); - if (next.done) { - return { done: true }; - } - return { done: false, value: next.value }; - }, - cancel, - }; - }, + getReader: () => ({ + read: async () => { + const next = await iterator.next(); + if (next.done) { + return { done: true }; + } + return { done: false, value: next.value }; + }, + cancel, + }), cancel, }, text, diff --git a/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts b/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts index a8b74c52c4..094be9774f 100644 --- a/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts +++ b/packages/shared/eventsource/__tests__/EventSource.fetchInjection.test.ts @@ -270,6 +270,92 @@ it('opens the stream when get returns undefined for an absent header', async () } }); +it('rejects a wrong content type that only the iterated headers reveal', async () => { + // A get that does not fold header-name case returns null for a present header. The + // iterated record closes that gate bypass. + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], { + get: () => null, + forEach(callback: (value: string, key: string) => void): void { + callback('text/html', 'Content-Type'); + }, + }); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const errors = new AsyncQueue(); + const es = createEventSource(url, { fetch: injected, initialRetryDelayMillis: 60000 }); + es.onerror = (e) => errors.add(e); + try { + const err = await errors.take(); + expect(err?.message).toContain('unexpected Content-Type'); + } finally { + es.close(); + } +}); + +it('rejects a wrong content type when get returns a non-string value', async () => { + // Only a string from get is a declared type. A hostile non-string result defers to the + // iterated record instead of skipping the gate. + const hostile = { + get: () => 12345, + forEach(callback: (value: string, key: string) => void): void { + callback('text/html', 'content-type'); + }, + }; + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], hostile as unknown as FetchHeaders); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const errors = new AsyncQueue(); + const es = createEventSource(url, { fetch: injected, initialRetryDelayMillis: 60000 }); + es.onerror = (e) => errors.add(e); + try { + const err = await errors.take(); + expect(err?.message).toContain('unexpected Content-Type'); + } finally { + es.close(); + } +}); + +it('opens the stream for a transport whose headers only expose forEach', async () => { + // The only legal headers shape before get was added. It must keep working through the + // iterated record. + const legacy = { + forEach(callback: (value: string, key: string) => void): void { + callback('text/event-stream', 'content-type'); + }, + }; + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], legacy as unknown as FetchHeaders); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const es = createEventSource(url, { fetch: injected }); + es.onerror = () => {}; + try { + const messages = startMessageQueue(es); + expect((await messages.take()).data).toEqual('hello'); + } finally { + es.close(); + } +}); + +it('rejects a wrong content type for a transport whose headers only expose forEach', async () => { + const legacy = { + forEach(callback: (value: string, key: string) => void): void { + callback('text/html', 'content-type'); + }, + }; + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], legacy as unknown as FetchHeaders); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const errors = new AsyncQueue(); + const es = createEventSource(url, { fetch: injected, initialRetryDelayMillis: 60000 }); + es.onerror = (e) => errors.add(e); + try { + const err = await errors.take(); + expect(err?.message).toContain('unexpected Content-Type'); + } finally { + es.close(); + } +}); + it('reports an empty message for a non-200 response without a statusText', async () => { const injected: FetchLike = async () => ({ status: 503, @@ -855,8 +941,31 @@ it('fails a 200 response whose headers cannot be read', async () => { } }); -it('fails a 200 response whose headers get throws', async () => { - // The declared content type gates the stream, so a throwing get must fail and retry. +it('opens through the iterated headers when get throws', async () => { + // A throwing get leaves the iterated record as the only content-type source. The stream + // must open through it instead of failing as unreadable. + const injected: FetchLike = async () => + idleStreamResponse(['data: hello\n\n'], { + get: () => { + throw new Error('get boom'); + }, + forEach(callback: (value: string, key: string) => void): void { + callback('text/event-stream', 'content-type'); + }, + }); + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const es = createEventSource(url, { fetch: injected }); + es.onerror = () => {}; + try { + const messages = startMessageQueue(es); + expect((await messages.take()).data).toEqual('hello'); + } finally { + es.close(); + } +}); + +it('fails a 200 response whose headers expose no access method', async () => { + // With no usable get and no iteration, the content type cannot be validated at all. const injected: FetchLike = async () => ({ status: 200, statusText: 'OK', @@ -864,9 +973,6 @@ it('fails a 200 response whose headers get throws', async () => { get: () => { throw new Error('get boom'); }, - forEach(callback: (value: string, key: string) => void): void { - callback('text/event-stream', 'content-type'); - }, }, body: { getReader: () => ({ read: () => new Promise(() => {}) }) }, }); @@ -1042,6 +1148,52 @@ it('keeps the status when a non-200 response headers forEach throws', async () = } }); +it('keeps the status when the response headers getter throws', async () => { + // The headers read happens once, inside the parser's guard. A throwing getter must not + // erase the status from the failure report, or a permanent auth failure would retry forever. + const hostile = { status: 401, statusText: 'Unauthorized', body: null }; + Object.defineProperty(hostile, 'headers', { + get() { + throw new Error('headers boom'); + }, + }); + const injected: FetchLike = async () => hostile as unknown as FetchLikeResponse; + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const errors = new AsyncQueue(); + const es = createEventSource(url, { fetch: injected, errorFilter: () => false }); + es.onerror = (e) => errors.add(e); + try { + const err = await errors.take(); + expect(err?.status).toEqual(401); + } finally { + es.close(); + } +}); + +it('fails a 200 response whose headers getter throws', async () => { + const hostile = { + status: 200, + statusText: 'OK', + body: { getReader: () => ({ read: () => new Promise(() => {}) }) }, + }; + Object.defineProperty(hostile, 'headers', { + get() { + throw new Error('headers boom'); + }, + }); + const injected: FetchLike = async () => hostile as unknown as FetchLikeResponse; + const url = `http://localhost:${deliberatelyUnusedPort}/stream`; + const es = createEventSource(url, { fetch: injected, initialRetryDelayMillis: 60000 }); + const events = new AsyncQueue(); + es.addEventListener('open', () => events.add('open')); + es.onerror = (e) => events.add(`error:${e?.message}`); + try { + expect(await events.take()).toEqual('error:stream response headers could not be read'); + } finally { + es.close(); + } +}); + it('ignores a stale fetch rejection after a newer attempt is streaming', async () => { // A slow transport can reject the first attempt's promise long after the read timeout has // already failed over to a second attempt. The stale rejection must not release the live diff --git a/packages/shared/eventsource/__tests__/httpHelpers.test.ts b/packages/shared/eventsource/__tests__/httpHelpers.test.ts index be8698c260..b4dd0844b2 100644 --- a/packages/shared/eventsource/__tests__/httpHelpers.test.ts +++ b/packages/shared/eventsource/__tests__/httpHelpers.test.ts @@ -39,6 +39,27 @@ it('keeps empty parts', () => { expect(splitHeaderListValue('a,,b,')).toEqual(['a', '', 'b', '']); }); -it('returns an empty object when the headers expose no forEach', () => { - expect(headersToObject({ get: () => null })).toEqual({}); +it('returns null when the headers expose no iteration method', () => { + expect(headersToObject({ get: () => null })).toBeNull(); +}); + +it('collects headers through forEach and folds the names to lowercase', () => { + expect( + headersToObject({ + get: () => null, + forEach: (callback) => { + callback('text/event-stream', 'Content-Type'); + }, + }), + ).toEqual({ 'content-type': 'text/event-stream' }); +}); + +it('collects headers through entries when forEach is absent', () => { + const pairs: [string, string][] = [['Content-Type', 'text/event-stream']]; + expect( + headersToObject({ + get: () => null, + entries: () => pairs, + }), + ).toEqual({ 'content-type': 'text/event-stream' }); }); diff --git a/packages/shared/eventsource/src/EventSource.ts b/packages/shared/eventsource/src/EventSource.ts index d5431d4379..5c6331bf9b 100644 --- a/packages/shared/eventsource/src/EventSource.ts +++ b/packages/shared/eventsource/src/EventSource.ts @@ -16,8 +16,8 @@ import { import { bodylessMethods, defaultFetch, - headersToObject, INVALID_HEADER_VALUE_CHAR, + parseResponseHeaders, splitHeaderListValue, } from './httpHelpers'; import { createDefaultEventRegistry } from './listenerRegistry'; @@ -621,30 +621,14 @@ export function createEventSource( return; } - // A hostile headers object must not break the failure report. Without the fallback, a - // non-200 status would be lost with the throw, and the error filter could not classify - // the failure. - let responseHeaders: Record; - let contentType: string | undefined; - let headersUnreadable = false; - try { - responseHeaders = headersToObject(res.headers); - } catch { - responseHeaders = {}; - headersUnreadable = true; - } - try { - const declaredType: unknown = res.headers.get('content-type'); - // A standard get returns a string or null. A sloppy transport can return undefined for - // an absent header, and a hostile one can return anything. Only a string is a declared - // type; every other value reads as no declared type. - contentType = typeof declaredType === 'string' ? declaredType : undefined; - } catch { - // A missing or throwing get leaves the declared type unknown. Only a success response - // needs the declared type, and the headers read above must survive for the failure - // report. - headersUnreadable = true; - } + // Every header access is guarded inside the parser, so a hostile headers object cannot + // break the failure report. Without that guard, a non-200 status would be lost with the + // throw, and the error filter could not classify the failure. + const { + headers: responseHeaders, + contentType, + unreadable: headersUnreadable, + } = parseResponseHeaders(res); // The status is read once. A getter that changes its answer between reads would skew the // classification, and a throw on a later read would skip the release below. diff --git a/packages/shared/eventsource/src/httpHelpers.ts b/packages/shared/eventsource/src/httpHelpers.ts index e37946d1c3..f31bcd8ff6 100644 --- a/packages/shared/eventsource/src/httpHelpers.ts +++ b/packages/shared/eventsource/src/httpHelpers.ts @@ -24,11 +24,15 @@ export interface FetchHeaders { /** * Executes the callback once for each header, with the value first. It is optional so a - * transport whose response headers expose no iteration can still satisfy the type. The client - * forwards headers to its open and error events only through this method, so without it those - * events carry no headers. + * transport whose response headers expose no iteration can still satisfy the type. */ forEach?(callback: (value: string, key: string) => void): void; + + /** + * Returns an iterator over the header name-value pairs, like the standard + * `Headers.entries`. It is the iteration fallback when `forEach` is absent. + */ + entries?(): Iterable<[string, string]>; } /** @@ -110,19 +114,92 @@ export const defaultFetch: FetchLike = (url, init) => globalThis.fetch(url, init /** The methods for which a `fetch()` request cannot have a body. */ export const bodylessMethods = ['GET', 'HEAD']; -export function headersToObject(headers: FetchHeaders): Record { +/** + * Collects the headers into a plain record. Returns `null` when the headers object provides + * no iteration method at all, so the caller can tell an unreadable header set from an empty + * one. A standard Headers object reports lowercase names, so names fold to lowercase. The + * fold gives a transport that reports wire-cased names the same shape. + */ +export function headersToObject(headers: FetchHeaders): Record | null { const result: Record = {}; - if (!headers.forEach) { - // A transport without header iteration forwards no headers. The client treats that the same - // as an empty header set. + if (headers.forEach) { + headers.forEach((value, key) => { + result[key.toLowerCase()] = value; + }); + return result; + } + if (headers.entries) { + Array.from(headers.entries()).forEach(([key, value]) => { + result[key.toLowerCase()] = value; + }); return result; } - headers.forEach((value, key) => { - // A standard Headers object reports lowercase names. The fold gives a transport that reports - // wire-cased names the same shape, so a consumer can look a header up by its lowercase name. - result[key.toLowerCase()] = value; - }); - return result; + return null; +} + +/** + * The response headers in the shape the client consumes. `contentType` is the declared + * content type, or `undefined` when the response declares none. `unreadable` is true when + * no header access works at all. + */ +export interface ParsedResponseHeaders { + headers: Record; + contentType: string | undefined; + unreadable: boolean; +} + +/** + * Reads and validates the headers of a response. Every access to the headers object is + * guarded, so a hostile response cannot break the failure report with a throw. The declared + * content type prefers the `get` lookup and falls back to the iterated record. + */ +export function parseResponseHeaders(res: FetchLikeResponse): ParsedResponseHeaders { + let resHeaders: FetchHeaders | undefined; + try { + ({ headers: resHeaders } = res); + } catch { + resHeaders = undefined; + } + if (resHeaders === undefined || resHeaders === null) { + return { headers: {}, contentType: undefined, unreadable: true }; + } + + let headers: Record = {}; + let iterated = false; + let unreadable = false; + try { + const collected = headersToObject(resHeaders); + iterated = collected !== null; + headers = collected ?? {}; + } catch { + unreadable = true; + } + + // `getResult` holds what `get` returned. `getAnsweredAbsent` is true only when a working + // `get` reported the header as absent, which is the one case where a missing record is + // not a failure. + let getResult: unknown; + let getAnsweredAbsent = false; + try { + if (typeof resHeaders.get === 'function') { + getResult = resHeaders.get('content-type'); + getAnsweredAbsent = getResult === null || getResult === undefined; + } + } catch { + // A throwing get leaves the declared type to the iterated record. + getResult = undefined; + } + + let contentType: string | undefined; + if (typeof getResult === 'string') { + contentType = getResult; + } else if (iterated) { + contentType = headers['content-type']; + } else if (!getAnsweredAbsent) { + unreadable = true; + } + + return { headers, contentType, unreadable }; } /**