From 63d179f7b7bbec163141f414ce119acf1f34bcff Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 00:24:55 -0400 Subject: [PATCH 01/18] feat(sdk): allow overriding the HTTP implementation via options.fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The SDK called the global `fetch` directly, so consumers had no way to influence how requests are made — ClientOptions exposed only maxRetries. `options.fetch` accepts any fetch-compatible function and is used for every request, retries included. The motivating case: undici 8.8.0-8.9.0 stalls for seconds before writing a request onto a socket idle for a few seconds (nodejs/undici#5600, fixed in 8.10.0), and Node 26 bundles 8.9.0. A consumer polling every few seconds sees multi-second latency on calls that should take milliseconds, with no recourse inside the SDK. With this they can supply an undici dispatcher with a tuned keepAliveTimeout and carry on. The same hook covers the ordinary reasons SDKs grow one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional through to HttpContext rather than being defaulted at construction, so the default path still calls the global directly. That keeps it late-bound — replacing globalThis.fetch after a client exists still works, which is what vi.stubGlobal does — and avoids invoking a detached `fetch` reference, which is not universally safe. Both properties are pinned by tests. Streaming is unaffected: .stream()/.liveQuery() go through EventSource, which this does not replace. Documented as such. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014EJvon2pyU4zaSHTGJj8fE --- clients/ts/src/client.test.ts | 53 +++++++++++++++++++++++++++++ clients/ts/src/client.ts | 1 + clients/ts/src/http.ts | 9 +++-- clients/ts/src/types.ts | 34 +++++++++++++++++- docs/src/content/docs/sdk/index.mdx | 26 ++++++++++++++ 5 files changed, 120 insertions(+), 3 deletions(-) diff --git a/clients/ts/src/client.test.ts b/clients/ts/src/client.test.ts index 6b201d24..5abd487a 100644 --- a/clients/ts/src/client.test.ts +++ b/clients/ts/src/client.test.ts @@ -143,3 +143,56 @@ describe("WaveHouseClient.sql()", () => { expect(callWithLegacyParams).toThrow(/client\.sql\(sql, params\) was removed/); }); }); + +describe("options.fetch", () => { + it("routes requests through a supplied fetch instead of the global", async () => { + const custom = vi.fn().mockResolvedValue(new Response(JSON.stringify([]), { status: 200 })); + const client = createClient({ + baseURL: "http://localhost:8080", + options: { fetch: custom as unknown as typeof fetch }, + }); + + await client.from("clicks").select("*").limit(1); + + expect(custom).toHaveBeenCalledTimes(1); + expect(fetchSpy).not.toHaveBeenCalled(); + const [url, init] = custom.mock.calls[0]; + expect(String(url)).toContain("http://localhost:8080"); + expect(init).toMatchObject({ method: expect.any(String) }); + }); + + it("falls back to the global fetch when no override is given", async () => { + const client = createClient({ baseURL: "http://localhost:8080" }); + await client.from("clicks").select("*").limit(1); + expect(fetchSpy).toHaveBeenCalledTimes(1); + }); + + it("keeps the global late-bound so it can be swapped after construction", async () => { + const client = createClient({ baseURL: "http://localhost:8080" }); + const replacement = vi + .fn() + .mockResolvedValue(new Response(JSON.stringify([]), { status: 200 })); + vi.stubGlobal("fetch", replacement); + + await client.from("clicks").select("*").limit(1); + + expect(replacement).toHaveBeenCalledTimes(1); + expect(fetchSpy).not.toHaveBeenCalled(); + }); + + it("applies the override to retries too, not just the first attempt", async () => { + const custom = vi + .fn() + .mockResolvedValueOnce(new Response("boom", { status: 500 })) + .mockResolvedValueOnce(new Response(JSON.stringify([]), { status: 200 })); + const client = createClient({ + baseURL: "http://localhost:8080", + options: { fetch: custom as unknown as typeof fetch, maxRetries: 1 }, + }); + + await client.from("clicks").select("*").limit(1); + + expect(custom).toHaveBeenCalledTimes(2); + expect(fetchSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/clients/ts/src/client.ts b/clients/ts/src/client.ts index eb443c42..8147e1ca 100644 --- a/clients/ts/src/client.ts +++ b/clients/ts/src/client.ts @@ -37,6 +37,7 @@ export class WaveHouseClient { auth: config.auth, options: { maxRetries: config.options?.maxRetries ?? 2, + fetch: config.options?.fetch, }, }; diff --git a/clients/ts/src/http.ts b/clients/ts/src/http.ts index 39215cb0..44e47cb9 100644 --- a/clients/ts/src/http.ts +++ b/clients/ts/src/http.ts @@ -56,12 +56,17 @@ export async function request(ctx: HttpContext, opts: RequestOptions): Promis for (let attempt = 0; attempt < maxAttempts; attempt++) { try { - const res = await fetch(url, { + const init: RequestInit = { method: opts.method, headers, body: requestBody, signal: opts.signal, - }); + }; + // Call the global directly when no override is configured, rather than + // capturing it — a detached `fetch` reference is not universally safe to + // invoke, and late binding is what lets tests swap `globalThis.fetch`. + const doFetch = ctx.options.fetch; + const res = doFetch ? await doFetch(url, init) : await fetch(url, init); if (res.ok) { const text = await res.text(); diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index fd33a5ad..ffc6c1f0 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -80,9 +80,37 @@ export interface ClientConfig<_DB extends Database = Database> { options?: ClientOptions; } +/** + * A `fetch`-compatible function. Matches the global `fetch` signature, so + * undici's export, `node-fetch`, or a thin wrapper around either satisfies it + * without casting. + */ +export type FetchLike = typeof fetch; + export interface ClientOptions { /** Maximum retry attempts for failed requests. Default: 2. */ maxRetries?: number; + /** + * HTTP implementation used for every request. Defaults to the global `fetch`. + * + * Provide one to route through a proxy, attach client certificates, add + * middleware (logging, tracing, circuit breaking), or work around transport + * behavior your runtime gets wrong — e.g. an undici dispatcher with a tuned + * `keepAliveTimeout`: + * + * ```ts + * import { Agent, fetch as undiciFetch } from "undici"; + * const dispatcher = new Agent({ keepAliveTimeout: 1_000 }); + * createClient({ + * baseURL, + * options: { fetch: (url, init) => undiciFetch(url, { ...init, dispatcher }) }, + * }); + * ``` + * + * Only the request path is affected — streaming (`.stream()`, `.liveQuery()`) + * goes through `EventSource`, which this does not replace. + */ + fetch?: FetchLike; } // --- Structured query AST (matches backend wire format) --- @@ -276,5 +304,9 @@ export interface StreamOptions { export interface HttpContext { baseURL: string; auth?: () => Promise | string; - options: { maxRetries: number }; + // `fetch` stays optional here rather than being defaulted at construction: + // resolving it per call keeps the global late-bound, so replacing + // `globalThis.fetch` after a client exists still works (vi.stubGlobal does + // exactly that). + options: { maxRetries: number; fetch?: FetchLike }; } diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 38fbb38b..2a45d981 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -326,6 +326,32 @@ const wh = createClient({ | `baseURL` | `string` | — | WaveHouse server URL, optionally including a path prefix (required) | | `auth` | `() => Promise \| string` | — | Token provider. Omit for public access | | `options.maxRetries` | `number` | `2` | Retry attempts for failed/5xx requests | +| `options.fetch` | `typeof fetch` | global `fetch` | HTTP implementation used for every request | + +### Supplying your own `fetch` + +`options.fetch` replaces the HTTP implementation the SDK uses. Reach for it to +route through a proxy, attach client certificates, wrap requests in your own +middleware (logging, tracing, circuit breaking), or to work around transport +behavior of the runtime you happen to be on: + +```ts +import { Agent, fetch as undiciFetch } from 'undici'; + +// e.g. retire idle sockets sooner than the runtime's default +const dispatcher = new Agent({ keepAliveTimeout: 1_000 }); + +const wh = createClient({ + baseURL: 'https://wavehouse.example.com', + options: { + fetch: (url, init) => undiciFetch(url, { ...init, dispatcher }), + }, +}); +``` + +Retries go through the same function, so middleware sees every attempt. Only the +request path is affected — streaming (`.stream()`, `.liveQuery()`) uses +`EventSource`, which this does not replace. :::note[How the token is transmitted] The SDK attaches your `auth` token as an `Authorization: Bearer` header on REST calls, and — because the browser `EventSource` API can't set headers — as a `?token=` query parameter on streaming connections. When both are present the server prefers the header, and it strips `?token=` from the request URL either way — so an unused credential can't survive into a later handler's log line, and the token stays out of **WaveHouse's own** logs. From 154cd4516b0e174e6f1c0fcdeb7da4800674e29b Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 00:34:30 -0400 Subject: [PATCH 02/18] docs: changelog entry for options.fetch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Documentation sync for the preceding commit — a new public ClientOptions field on a published package, which AGENTS.md §Documentation Sync requires under [Unreleased]. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_014EJvon2pyU4zaSHTGJj8fE --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a397980..bce224ca 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls for seconds before writing a request onto a socket idle for a few seconds ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so a consumer polling every few seconds sees multi-second latency on calls that should take milliseconds, with no recourse inside the SDK. With this they can pass an undici `Agent` with a tuned `keepAliveTimeout` and carry on. The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Streaming is unaffected — `.stream()`/`.liveQuery()` go through `EventSource`, which this does not replace. + - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. - **"Durability & Storage" operations guide** (`docs/src/content/docs/durability.md` (new), `docs/src/config/sidebar.ts`, `docs/src/content/docs/reverse-proxy.mdx`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/deployment.md`): documents #84. A new Operations page making the embedded-JetStream durability contract explicit before the docs site publishes: a `200` from `POST /v1/ingest` means the event has been `fsync`'d to disk on the node (the server runs with `SyncAlways: true` in `internal/mq/embedded.go`), which makes the storage substrate's `fsync` tail the ingest latency floor. Covers the contract (and how it differs from JetStream's default page-cache-then-periodic-sync mode), why a slow `fsync` tail manifests as `create stream: ... context deadline exceeded` and `503` backpressure, a where-it's-cheap-vs-expensive substrate table (managed cloud block storage and PLP NVMe vs. ZFS-without-SLOG / qcow2-on-`ext4` / spinning disks), an `fio` recipe + verdict bands to measure your own storage (with the macOS `F_FULLFSYNC` honesty caveat), and the symptom checklist. Forward-references the configurable group-commit interval (`mq.sync_interval`, [#139](https://github.com/Wave-RF/WaveHouse/issues/139)) and the planned `wavehouse storage-check` preflight ([#84](https://github.com/Wave-RF/WaveHouse/issues/84)) without claiming either exists yet. Cross-linked from Configuration (Message Queue), Deployment (Persistent Storage), and the Ingest Pipeline's worker-side ack section; no code changes. From 7ee58614b655f643e304339ce6c08c94acb75237 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 09:24:23 -0400 Subject: [PATCH 03/18] fix(sdk): export FetchLike and correct the options.fetch docs Pre-push review found the advertised type surface didn't match what shipped, in two ways. FetchLike was declared and referenced by ClientOptions but never listed in index.ts's export block, so `import type { FetchLike } from "@wavehouse/sdk"` did not resolve -- while the CHANGELOG claimed it was exported. It's now in the export list; verified in dist/index.d.ts. The documented undici example did not compile. Reproduced with the repo's own tsc against undici 7.28: under the default lib (which includes DOM, and is what this package's own tsconfig uses) the wrapper fails with TS2322 on Response -- undici declares its own Request and Response, separate from the ones behind the global fetch -- plus TS2345 on the input arg. Assigning undici's fetch export directly fails under Node-only libs too. So the "satisfies it without casting" claim was wrong in general, not just at the edges. The example now carries the casts that make it typecheck (verified clean under both lib configs against the built package), and the JSDoc says plainly that such implementations need a cast, and why. Also corrected: liveQuery's initial backfill is an ordinary request and does go through the supplied fetch -- only the SSE connection is exempt. The docs, JSDoc and CHANGELOG all said otherwise, and streaming.md already described the backfill correctly, so the pages disagreed. The docs section had been inserted between the ClientConfig table and that section's trailing content, orphaning the auth note and the baseURL path-prefix subsection under an h3 about fetch; it now sits after them. Adds the install note for the undici import, states the contract an implementer must satisfy (string URL, plain RequestInit, only .ok/.status/.headers/.text() read back, rejection -> retried NETWORK_ERROR, AbortError -> ABORTED), and leads with a middleware example that compiles anywhere. The override test asserted only that init.method was some string; it now pins the full init -- string URL, POST, Content-Type, Authorization, serialized body -- which is what proxy and middleware consumers rely on. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- clients/ts/src/client.test.ts | 11 +++- clients/ts/src/index.ts | 2 + clients/ts/src/types.ts | 32 +++++++---- docs/src/content/docs/sdk/index.mdx | 84 +++++++++++++++++++++-------- 5 files changed, 97 insertions(+), 34 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bce224ca..6ad1f7c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls for seconds before writing a request onto a socket idle for a few seconds ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so a consumer polling every few seconds sees multi-second latency on calls that should take milliseconds, with no recourse inside the SDK. With this they can pass an undici `Agent` with a tuned `keepAliveTimeout` and carry on. The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Streaming is unaffected — `.stream()`/`.liveQuery()` go through `EventSource`, which this does not replace. +- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls for seconds before writing a request onto a socket idle for a few seconds ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so a consumer polling every few seconds sees multi-second latency on calls that should take milliseconds, with no recourse inside the SDK. With this they can pass an undici `Agent` with a tuned `keepAliveTimeout` and carry on. The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Note that implementations shipping their own request/response declarations (undici, `node-fetch`) need a cast, since those types are separate from the ones behind the global `fetch`; the documented contract is deliberately narrow (string URL, plain `RequestInit`, and only `.ok`/`.status`/`.headers`/`.text()` read back) so the cast is safe. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. diff --git a/clients/ts/src/client.test.ts b/clients/ts/src/client.test.ts index 5abd487a..96a29375 100644 --- a/clients/ts/src/client.test.ts +++ b/clients/ts/src/client.test.ts @@ -149,6 +149,7 @@ describe("options.fetch", () => { const custom = vi.fn().mockResolvedValue(new Response(JSON.stringify([]), { status: 200 })); const client = createClient({ baseURL: "http://localhost:8080", + auth: () => "test-token", options: { fetch: custom as unknown as typeof fetch }, }); @@ -156,9 +157,17 @@ describe("options.fetch", () => { expect(custom).toHaveBeenCalledTimes(1); expect(fetchSpy).not.toHaveBeenCalled(); + // The documented contract: a string URL and a complete RequestInit — + // proxy/middleware consumers rely on the whole request reaching them. const [url, init] = custom.mock.calls[0]; + expect(typeof url).toBe("string"); expect(String(url)).toContain("http://localhost:8080"); - expect(init).toMatchObject({ method: expect.any(String) }); + expect(init.method).toBe("POST"); + expect(init.headers).toMatchObject({ + "Content-Type": "application/json", + Authorization: "Bearer test-token", + }); + expect(typeof init.body).toBe("string"); }); it("falls back to the global fetch when no override is given", async () => { diff --git a/clients/ts/src/index.ts b/clients/ts/src/index.ts index 27db5667..2b305fcf 100644 --- a/clients/ts/src/index.ts +++ b/clients/ts/src/index.ts @@ -28,6 +28,8 @@ export type { Database, // DLQ DLQStats, + // HTTP + FetchLike, FetchOptions, // Query FilterOp, diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index ffc6c1f0..e8d3abb9 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -81,9 +81,17 @@ export interface ClientConfig<_DB extends Database = Database> { } /** - * A `fetch`-compatible function. Matches the global `fetch` signature, so - * undici's export, `node-fetch`, or a thin wrapper around either satisfies it - * without casting. + * A `fetch`-compatible function, matching the global `fetch` signature. + * + * Implementations that ship their own request/response types (undici, + * `node-fetch`) generally need a cast: their declarations are separate from the + * ones behind your global `fetch`, so the two are not structurally assignable. + * + * The SDK only ever calls it with a string URL and a plain `RequestInit`, and + * only reads `.ok`, `.status`, `.headers`, and `.text()` off the response — so + * a cast here is safe in practice. A rejection is surfaced as a + * `NETWORK_ERROR` result and retried with backoff; an `AbortError` becomes + * `ABORTED` without a retry. */ export type FetchLike = typeof fetch; @@ -99,16 +107,21 @@ export interface ClientOptions { * `keepAliveTimeout`: * * ```ts - * import { Agent, fetch as undiciFetch } from "undici"; + * import { Agent, fetch as undiciFetch } from "undici"; // npm install undici * const dispatcher = new Agent({ keepAliveTimeout: 1_000 }); * createClient({ * baseURL, - * options: { fetch: (url, init) => undiciFetch(url, { ...init, dispatcher }) }, + * options: { + * fetch: ((url: string, init?: RequestInit) => + * undiciFetch(url, { ...init, dispatcher } as never)) as unknown as FetchLike, + * }, * }); * ``` * - * Only the request path is affected — streaming (`.stream()`, `.liveQuery()`) - * goes through `EventSource`, which this does not replace. + * Only the SSE transport is exempt: the live connection behind `.stream()` + * and `.liveQuery()` uses `EventSource`, which this does not replace. + * `.liveQuery()`'s initial backfill is an ordinary request and does go + * through your function. */ fetch?: FetchLike; } @@ -304,9 +317,6 @@ export interface StreamOptions { export interface HttpContext { baseURL: string; auth?: () => Promise | string; - // `fetch` stays optional here rather than being defaulted at construction: - // resolving it per call keeps the global late-bound, so replacing - // `globalThis.fetch` after a client exists still works (vi.stubGlobal does - // exactly that). + // Stays optional rather than defaulted here, to keep the global late-bound. options: { maxRetries: number; fetch?: FetchLike }; } diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 2a45d981..2b14d361 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -326,7 +326,27 @@ const wh = createClient({ | `baseURL` | `string` | — | WaveHouse server URL, optionally including a path prefix (required) | | `auth` | `() => Promise \| string` | — | Token provider. Omit for public access | | `options.maxRetries` | `number` | `2` | Retry attempts for failed/5xx requests | -| `options.fetch` | `typeof fetch` | global `fetch` | HTTP implementation used for every request | +| `options.fetch` | `FetchLike` | global `fetch` | HTTP implementation used for every request ([details](#supplying-your-own-fetch)) | + +:::note[How the token is transmitted] +The SDK attaches your `auth` token as an `Authorization: Bearer` header on REST calls, and — because the browser `EventSource` API can't set headers — as a `?token=` query parameter on streaming connections. When both are present the server prefers the header, and it strips `?token=` from the request URL either way — so an unused credential can't survive into a later handler's log line, and the token stays out of **WaveHouse's own** logs. + +That strip can't reach anything upstream, though: the token is in the request URI on the wire, so any proxy, CDN, or load balancer in front of WaveHouse records it in access logs unless you redact query strings there. +::: + +#### Serving under a path prefix + +`baseURL` may carry a path prefix, for a WaveHouse reachable somewhere other than the root of an origin — behind a backend-for-frontend (BFF), an app-server route, or a path-routed ingress. Every request path is appended to it, on both transports: + +```ts +const wh = createClient({ baseURL: 'https://app.example.com/api/warehouse' }); +// queries → https://app.example.com/api/warehouse/v1/query +// streams → https://app.example.com/api/warehouse/v1/stream +``` + +Trailing slashes on `baseURL` are optional, but it must be **absolute** — scheme and host included. A same-origin relative path like `/api/warehouse` makes every REST call reject with a `TypeError` instead of returning a `Result`, and surfaces on a stream as an `SSE_CONNECT_ERROR` to the subscriber's `error` callback — so build an absolute one: `` `${location.origin}/api/warehouse` ``. + +The proxy in front must **strip** the prefix before forwarding, since WaveHouse itself always serves at `/v1/…` — see [Behind a reverse proxy → Path prefixes](/reverse-proxy#path-prefixes). ### Supplying your own `fetch` @@ -336,42 +356,64 @@ middleware (logging, tracing, circuit breaking), or to work around transport behavior of the runtime you happen to be on: ```ts -import { Agent, fetch as undiciFetch } from 'undici'; - -// e.g. retire idle sockets sooner than the runtime's default -const dispatcher = new Agent({ keepAliveTimeout: 1_000 }); - const wh = createClient({ baseURL: 'https://wavehouse.example.com', options: { - fetch: (url, init) => undiciFetch(url, { ...init, dispatcher }), + fetch: async (url, init) => { + const started = performance.now(); + const res = await fetch(url, init); + console.log(res.status, url, `${Math.round(performance.now() - started)}ms`); + return res; + }, }, }); ``` -Retries go through the same function, so middleware sees every attempt. Only the -request path is affected — streaming (`.stream()`, `.liveQuery()`) uses -`EventSource`, which this does not replace. +Retries go through the same function, so middleware sees every attempt. -:::note[How the token is transmitted] -The SDK attaches your `auth` token as an `Authorization: Bearer` header on REST calls, and — because the browser `EventSource` API can't set headers — as a `?token=` query parameter on streaming connections. When both are present the server prefers the header, and it strips `?token=` from the request URL either way — so an unused credential can't survive into a later handler's log line, and the token stays out of **WaveHouse's own** logs. +The SDK always calls your function with a **string** URL and a plain +`RequestInit`, and reads only `.ok`, `.status`, `.headers`, and `.text()` off +the response. If it rejects, the request becomes a `NETWORK_ERROR` result and is +retried with backoff; an `AbortError` becomes `ABORTED` and is not retried. The +option's type is exported as `FetchLike`. -That strip can't reach anything upstream, though: the token is in the request URI on the wire, so any proxy, CDN, or load balancer in front of WaveHouse records it in access logs unless you redact query strings there. -::: +Only the SSE transport is exempt: the live connection behind `.stream()` and +`.liveQuery()` uses `EventSource`, which this does not replace. `.liveQuery()`'s +[initial backfill](/sdk/streaming#live-queries) is an ordinary request and *does* +go through your function. -#### Serving under a path prefix +#### Swapping in undici -`baseURL` may carry a path prefix, for a WaveHouse reachable somewhere other than the root of an origin — behind a backend-for-frontend (BFF), an app-server route, or a path-routed ingress. Every request path is appended to it, on both transports: +The motivating case is a transport bug you can't fix from inside the SDK — for +example [undici #5600](https://github.com/nodejs/undici/issues/5600), where +8.8.0–8.9.0 (bundled in Node 26) stalls for seconds before writing a request +onto a briefly idle socket. A dispatcher with a tuned `keepAliveTimeout` works +around it: ```ts -const wh = createClient({ baseURL: 'https://app.example.com/api/warehouse' }); -// queries → https://app.example.com/api/warehouse/v1/query -// streams → https://app.example.com/api/warehouse/v1/stream +import { Agent, fetch as undiciFetch } from 'undici'; // npm install undici + +// Retire idle sockets sooner than the runtime's default +const dispatcher = new Agent({ keepAliveTimeout: 1_000 }); + +const wh = createClient({ + baseURL: 'https://wavehouse.example.com', + options: { + fetch: ((url: string, init?: RequestInit) => + undiciFetch(url, { ...init, dispatcher } as never)) as unknown as FetchLike, + }, +}); ``` -Trailing slashes on `baseURL` are optional, but it must be **absolute** — scheme and host included. A same-origin relative path like `/api/warehouse` makes every REST call reject with a `TypeError` instead of returning a `Result`, and surfaces on a stream as an `SSE_CONNECT_ERROR` to the subscriber's `error` callback — so build an absolute one: `` `${location.origin}/api/warehouse` ``. +The casts are load-bearing: undici declares its own `Request`/`Response` types, +separate from the ones behind your global `fetch`, so the two aren't +structurally assignable and TypeScript rejects the direct form. They're safe +because of the narrow contract above. `undici` is a dependency you add — the SDK +itself stays dependency-free. -The proxy in front must **strip** the prefix before forwarding, since WaveHouse itself always serves at `/v1/…` — see [Behind a reverse proxy → Path prefixes](/reverse-proxy#path-prefixes). +If you only need the dispatcher (and not middleware), `setGlobalDispatcher(new +Agent({ keepAliveTimeout: 1_000 }))` achieves the same thing process-wide +without `options.fetch`. ### Type-Safe Tables From 3eb2d4f6b46ed4421d08c3c474871a7d50ba88ec Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 09:44:12 -0400 Subject: [PATCH 04/18] fix(sdk): narrow FetchLike and correct the documented fetch contract Second pre-push review round. The contract sentence added in the last commit was itself wrong, and the type didn't accept the shape it described. The contract claimed the SDK reads "only .ok/.status/.headers/.text()" off the response. The error path also calls res.json() and reads res.statusText (errors.ts:7,17). A response object built to the documented spec doesn't crash -- parseErrorResponse swallows the TypeError -- it silently degrades, yielding a WaveHouseError whose message is typed string but is undefined at runtime, on every error response. Contract now lists all six members and says which are read when. FetchLike was typeof fetch, which rejects the very middleware signature the docs tell you to write: (url: string, init?: RequestInit) => Promise is not assignable to a parameter accepting RequestInfo | URL, so it fails on contravariance with an error that has nothing to do with the cause the docs give. Narrowed to the signature the SDK actually calls -- a string URL is all it ever passes. That's strictly more permissive: the global fetch still assigns, and declared middleware now assigns with no cast. Verified against the built package under both a DOM-inclusive and a Node-only lib config. Abort handling is now stated: ABORTED requires a DOMException named AbortError, so an implementation that signals abort another way -- node-fetch throws its own AbortError class -- is retried as NETWORK_ERROR instead. That was worth saying out loud given the JSDoc names node-fetch as a supported implementation. Corrected the severity attributed to nodejs/undici#5600. It was described as stalling "for seconds" on a socket idle for seconds; the issue is titled "stalls up to ~500ms on an idle event loop" and the reported measurements are ~450-465ms against a 10ms server, with the repro making back-to-back requests. The trigger is an idle event loop, not a seconds-idle socket. Also says plainly that 8.10.0 fixes it and upgrading is the real answer -- the page previously steered readers toward a permanent code change for a bug already fixed upstream. The undici snippet used FetchLike and createClient with no SDK import, so it didn't compile as pasted; it now imports what it uses. Config table notes SSE streams are excluded, since that carve-out was three sections below where most readers stop. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- clients/ts/src/client.test.ts | 5 +++-- clients/ts/src/types.ts | 35 ++++++++++++++++++----------- docs/src/content/docs/sdk/index.mdx | 35 ++++++++++++++++++++--------- 4 files changed, 51 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ad1f7c1..aca15c71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls for seconds before writing a request onto a socket idle for a few seconds ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so a consumer polling every few seconds sees multi-second latency on calls that should take milliseconds, with no recourse inside the SDK. With this they can pass an undici `Agent` with a tuned `keepAliveTimeout` and carry on. The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Note that implementations shipping their own request/response declarations (undici, `node-fetch`) need a cast, since those types are separate from the ones behind the global `fetch`; the documented contract is deliberately narrow (string URL, plain `RequestInit`, and only `.ok`/`.status`/`.headers`/`.text()` read back) so the cast is safe. +- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls roughly 500 ms per request when a keep-alive socket is reused on an idle event loop ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600) — the reporter measured ~450–465 ms against a 10 ms server; fixed in 8.10.0), and Node 26 bundles 8.9.0 — so a consumer on an affected runtime pays that on calls that should take milliseconds, with no recourse inside the SDK. Upgrading undici is the real fix; `options.fetch` is what you reach for while pinned. With this they can pass an undici `Agent` with a tuned `keepAliveTimeout` and carry on. The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. `FetchLike` is deliberately narrower than `typeof fetch` — it takes a `string` URL, which is all the SDK ever passes — so consumer middleware declared as `(url: string, init?: RequestInit) => Promise` assigns without a cast, where `typeof fetch` would reject it on parameter contravariance. Implementations shipping their own request/response declarations (undici, `node-fetch`) do still need a cast, since those types are separate from the ones behind the global `fetch`; the narrow documented contract is what makes it safe — a string URL and plain `RequestInit` in, and only `.ok`/`.status`/`.headers` plus `.text()` on success and `.json()`/`.statusText` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. diff --git a/clients/ts/src/client.test.ts b/clients/ts/src/client.test.ts index 96a29375..1553654a 100644 --- a/clients/ts/src/client.test.ts +++ b/clients/ts/src/client.test.ts @@ -6,6 +6,7 @@ import { PolicyNamespace } from "./policy.js"; import { SchemaNamespace } from "./schema.js"; import { SysNamespace } from "./sys.js"; import { TableRef } from "./table.js"; +import type { FetchLike } from "./types.js"; let fetchSpy: ReturnType; @@ -150,7 +151,7 @@ describe("options.fetch", () => { const client = createClient({ baseURL: "http://localhost:8080", auth: () => "test-token", - options: { fetch: custom as unknown as typeof fetch }, + options: { fetch: custom as unknown as FetchLike }, }); await client.from("clicks").select("*").limit(1); @@ -196,7 +197,7 @@ describe("options.fetch", () => { .mockResolvedValueOnce(new Response(JSON.stringify([]), { status: 200 })); const client = createClient({ baseURL: "http://localhost:8080", - options: { fetch: custom as unknown as typeof fetch, maxRetries: 1 }, + options: { fetch: custom as unknown as FetchLike, maxRetries: 1 }, }); await client.from("clicks").select("*").limit(1); diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index e8d3abb9..2f5029c6 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -81,25 +81,34 @@ export interface ClientConfig<_DB extends Database = Database> { } /** - * A `fetch`-compatible function, matching the global `fetch` signature. + * A `fetch`-compatible function. * - * Implementations that ship their own request/response types (undici, - * `node-fetch`) generally need a cast: their declarations are separate from the - * ones behind your global `fetch`, so the two are not structurally assignable. + * Deliberately narrower than `typeof fetch`: the SDK only ever calls it with a + * string URL, so this is what it actually requires. The global `fetch` still + * assigns to it, and so does middleware you declare as + * `(url: string, init?: RequestInit) => Promise` — which `typeof + * fetch` would reject, since a parameter accepting only `string` is not + * assignable to one accepting `RequestInfo | URL`. * - * The SDK only ever calls it with a string URL and a plain `RequestInit`, and - * only reads `.ok`, `.status`, `.headers`, and `.text()` off the response — so - * a cast here is safe in practice. A rejection is surfaced as a - * `NETWORK_ERROR` result and retried with backoff; an `AbortError` becomes - * `ABORTED` without a retry. + * The response is read for `.ok`, `.status` and `.headers` always, `.text()` on + * success, and `.json()` plus `.statusText` when it is not `ok`. A rejection + * becomes a `NETWORK_ERROR` result and is retried with backoff — except a + * `DOMException` named `AbortError` (what the platform `fetch` and undici throw + * on abort), which becomes `ABORTED` and is not retried. Implementations that + * reject with some other abort error, such as `node-fetch`, are retried instead. + * + * Implementations shipping their own request/response declarations (undici, + * `node-fetch`) still need a cast, since those types are separate from the ones + * behind your global `fetch`; the narrow contract above is what makes it safe. */ -export type FetchLike = typeof fetch; +export type FetchLike = (url: string, init?: RequestInit) => Promise; export interface ClientOptions { /** Maximum retry attempts for failed requests. Default: 2. */ maxRetries?: number; /** - * HTTP implementation used for every request. Defaults to the global `fetch`. + * HTTP implementation used for every REST request (SSE streams excluded). + * Defaults to the global `fetch`. * * Provide one to route through a proxy, attach client certificates, add * middleware (logging, tracing, circuit breaking), or work around transport @@ -112,8 +121,8 @@ export interface ClientOptions { * createClient({ * baseURL, * options: { - * fetch: ((url: string, init?: RequestInit) => - * undiciFetch(url, { ...init, dispatcher } as never)) as unknown as FetchLike, + * fetch: (url, init) => + * undiciFetch(url, { ...init, dispatcher } as never) as unknown as Promise, * }, * }); * ``` diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 2b14d361..ba262c5d 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -326,7 +326,7 @@ const wh = createClient({ | `baseURL` | `string` | — | WaveHouse server URL, optionally including a path prefix (required) | | `auth` | `() => Promise \| string` | — | Token provider. Omit for public access | | `options.maxRetries` | `number` | `2` | Retry attempts for failed/5xx requests | -| `options.fetch` | `FetchLike` | global `fetch` | HTTP implementation used for every request ([details](#supplying-your-own-fetch)) | +| `options.fetch` | `FetchLike` | global `fetch` | HTTP implementation for every REST request; SSE streams excluded ([details](#supplying-your-own-fetch)) | :::note[How the token is transmitted] The SDK attaches your `auth` token as an `Authorization: Bearer` header on REST calls, and — because the browser `EventSource` API can't set headers — as a `?token=` query parameter on streaming connections. When both are present the server prefers the header, and it strips `?token=` from the request URL either way — so an unused credential can't survive into a later handler's log line, and the token stays out of **WaveHouse's own** logs. @@ -372,10 +372,19 @@ const wh = createClient({ Retries go through the same function, so middleware sees every attempt. The SDK always calls your function with a **string** URL and a plain -`RequestInit`, and reads only `.ok`, `.status`, `.headers`, and `.text()` off -the response. If it rejects, the request becomes a `NETWORK_ERROR` result and is -retried with backoff; an `AbortError` becomes `ABORTED` and is not retried. The -option's type is exported as `FetchLike`. +`RequestInit`. Off the response it reads `.ok`, `.status` and `.headers` +always, `.text()` on success, and `.json()` plus `.statusText` when the response +is not `ok` — so a hand-rolled response object needs all six. If your function +rejects, the request becomes a `NETWORK_ERROR` result and is retried with +backoff; the one exception is a `DOMException` named `AbortError` — what the +platform `fetch` and undici throw on abort — which becomes `ABORTED` and is not +retried. An implementation that signals abort some other way (`node-fetch` +throws its own `AbortError` class) gets retried instead. + +The option's type is exported as `FetchLike`, and is deliberately narrower than +`typeof fetch` — it takes a `string` URL, so your own +`(url: string, init?: RequestInit) => Promise` middleware assigns to it +without a cast. Only the SSE transport is exempt: the live connection behind `.stream()` and `.liveQuery()` uses `EventSource`, which this does not replace. `.liveQuery()`'s @@ -386,11 +395,17 @@ go through your function. The motivating case is a transport bug you can't fix from inside the SDK — for example [undici #5600](https://github.com/nodejs/undici/issues/5600), where -8.8.0–8.9.0 (bundled in Node 26) stalls for seconds before writing a request -onto a briefly idle socket. A dispatcher with a tuned `keepAliveTimeout` works -around it: +reusing a keep-alive socket on an idle event loop stalls roughly 500 ms per +request (the reporter measured ~450–465 ms against a 10 ms server). It affects +undici 8.8.0–8.9.0, and Node 26 bundles 8.9.0. + +**Upgrade first if you can:** this was fixed in undici 8.10.0, so a runtime with +8.10.0 or newer needs no workaround. Reach for `options.fetch` when you're +pinned to an affected version — a dispatcher with a tuned `keepAliveTimeout` +sidesteps it: ```ts +import { createClient } from '@wavehouse/sdk'; import { Agent, fetch as undiciFetch } from 'undici'; // npm install undici // Retire idle sockets sooner than the runtime's default @@ -399,8 +414,8 @@ const dispatcher = new Agent({ keepAliveTimeout: 1_000 }); const wh = createClient({ baseURL: 'https://wavehouse.example.com', options: { - fetch: ((url: string, init?: RequestInit) => - undiciFetch(url, { ...init, dispatcher } as never)) as unknown as FetchLike, + fetch: (url, init) => + undiciFetch(url, { ...init, dispatcher } as never) as unknown as Promise, }, }); ``` From 9524ec6250fdd1bf214d3eaa2d14ce45c2a08a84 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 10:09:37 -0400 Subject: [PATCH 05/18] =?UTF-8?q?fix(sdk):=20correct=20the=20undici=20reme?= =?UTF-8?q?dy=20=E2=80=94=20the=20keepAliveTimeout=20tuning=20is=20inert?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third pre-push review round. The docs prescribed a workaround for nodejs/undici#5600 that does not work, which I confirmed by measuring it rather than reasoning about it. Against a 10ms local server on undici 8.9.0 (Node 22.23.2), with 1.5s idle gaps, per-request latency: default Agent() 22 1513 1493 584 Agent({ keepAliveTimeout: 1_000 }) 13 1500 1496 1513 Agent({ keepAliveTimeout: 1 }) 15 1484 1511 1500 Agent({ pipelining: 0 }) 13 13 12 12 same script on undici 8.10.0 22 15 13 13 So the tuning the page recommended changes nothing: the socket retirement timer is starved by the same idle event loop that triggers the bug, so the socket is still pooled and still reused. What actually made the documented snippet fast is that `npm install undici` resolves to 8.10.0 -- a fixed implementation -- not the dispatcher argument. Reframed the remedy accordingly: options.fetch is how you route through an undici you install yourself, so the copy bundled with your Node never handles the request. Dropped the keepAliveTimeout argument and its "retire idle sockets sooner" comment, and said plainly that the knob does not help, since it is the obvious thing for a reader to try. For anyone genuinely pinned to an affected undici, pipelining: 0 is documented instead -- measured effective, at a connection per request. The setGlobalDispatcher fallback is now attributed correctly: the mechanism works, but only helps when the installed undici is 8.10.0+. Also corrected the severity in the other direction from last commit. The upstream issue title says ~500ms and I took it at face value; the stall is actually erratic and idle-gap dependent, measuring ~100ms to ~2.5s across gaps. The docs now give the upstream number and our own range rather than a single figure. Contract precision: .status is only read on the failure path (http.ts:80, errors.ts:19-22) -- the success branch touches only .text() and .headers. Listing it as always-read was conservative but wrong for a paragraph that is an explicit spec. Verified the rewritten snippet by running it, not just typechecking it: against a live server it returns data on the success path and, on a 404, a WaveHouseError whose message is "Not Found" -- i.e. the .statusText fallback the contract documents. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- clients/ts/src/types.ts | 17 +++++----- docs/src/content/docs/sdk/index.mdx | 49 ++++++++++++++++------------- 3 files changed, 37 insertions(+), 31 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aca15c71..3e72d8b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls roughly 500 ms per request when a keep-alive socket is reused on an idle event loop ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600) — the reporter measured ~450–465 ms against a 10 ms server; fixed in 8.10.0), and Node 26 bundles 8.9.0 — so a consumer on an affected runtime pays that on calls that should take milliseconds, with no recourse inside the SDK. Upgrading undici is the real fix; `options.fetch` is what you reach for while pinned. With this they can pass an undici `Agent` with a tuned `keepAliveTimeout` and carry on. The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. `FetchLike` is deliberately narrower than `typeof fetch` — it takes a `string` URL, which is all the SDK ever passes — so consumer middleware declared as `(url: string, init?: RequestInit) => Promise` assigns without a cast, where `typeof fetch` would reject it on parameter contravariance. Implementations shipping their own request/response declarations (undici, `node-fetch`) do still need a cast, since those types are separate from the ones behind the global `fetch`; the narrow documented contract is what makes it safe — a string URL and plain `RequestInit` in, and only `.ok`/`.status`/`.headers` plus `.text()` on success and `.json()`/`.statusText` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. +- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. The upstream report measured ~450–465 ms; measuring 8.9.0 while writing this, it ranged erratically from ~100 ms to ~2.5 s depending on the idle gap. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, so the bundled copy never handles the request. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. `FetchLike` is deliberately narrower than `typeof fetch` — it takes a `string` URL, which is all the SDK ever passes — so consumer middleware declared as `(url: string, init?: RequestInit) => Promise` assigns without a cast, where `typeof fetch` would reject it on parameter contravariance. Implementations shipping their own request/response declarations (undici, `node-fetch`) do still need a cast, since those types are separate from the ones behind the global `fetch`; the narrow documented contract is what makes it safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index 2f5029c6..51aeaa23 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -90,8 +90,8 @@ export interface ClientConfig<_DB extends Database = Database> { * fetch` would reject, since a parameter accepting only `string` is not * assignable to one accepting `RequestInfo | URL`. * - * The response is read for `.ok`, `.status` and `.headers` always, `.text()` on - * success, and `.json()` plus `.statusText` when it is not `ok`. A rejection + * The response is read for `.ok` and `.headers` always, `.text()` on success, + * and `.status`, `.statusText` plus `.json()` when it is not `ok`. A rejection * becomes a `NETWORK_ERROR` result and is retried with backoff — except a * `DOMException` named `AbortError` (what the platform `fetch` and undici throw * on abort), which becomes `ABORTED` and is not retried. Implementations that @@ -111,18 +111,17 @@ export interface ClientOptions { * Defaults to the global `fetch`. * * Provide one to route through a proxy, attach client certificates, add - * middleware (logging, tracing, circuit breaking), or work around transport - * behavior your runtime gets wrong — e.g. an undici dispatcher with a tuned - * `keepAliveTimeout`: + * middleware (logging, tracing, circuit breaking), or bypass a transport bug + * in the runtime's bundled HTTP stack by routing through an implementation + * you install yourself — e.g. an undici new enough to be free of + * {@link https://github.com/nodejs/undici/issues/5600 | undici #5600}: * * ```ts - * import { Agent, fetch as undiciFetch } from "undici"; // npm install undici - * const dispatcher = new Agent({ keepAliveTimeout: 1_000 }); + * import { fetch as undiciFetch } from "undici"; // npm install undici — 8.10.0+ * createClient({ * baseURL, * options: { - * fetch: (url, init) => - * undiciFetch(url, { ...init, dispatcher } as never) as unknown as Promise, + * fetch: (url, init) => undiciFetch(url, init as never) as unknown as Promise, * }, * }); * ``` diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index ba262c5d..0f1c97be 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -372,9 +372,9 @@ const wh = createClient({ Retries go through the same function, so middleware sees every attempt. The SDK always calls your function with a **string** URL and a plain -`RequestInit`. Off the response it reads `.ok`, `.status` and `.headers` -always, `.text()` on success, and `.json()` plus `.statusText` when the response -is not `ok` — so a hand-rolled response object needs all six. If your function +`RequestInit`. Off the response it reads `.ok` and `.headers` always, `.text()` +on success, and `.status`, `.statusText` plus `.json()` when the response is not +`ok` — so a hand-rolled response object needs all six. If your function rejects, the request becomes a `NETWORK_ERROR` result and is retried with backoff; the one exception is a `DOMException` named `AbortError` — what the platform `fetch` and undici throw on abort — which becomes `ABORTED` and is not @@ -393,29 +393,28 @@ go through your function. #### Swapping in undici -The motivating case is a transport bug you can't fix from inside the SDK — for -example [undici #5600](https://github.com/nodejs/undici/issues/5600), where -reusing a keep-alive socket on an idle event loop stalls roughly 500 ms per -request (the reporter measured ~450–465 ms against a 10 ms server). It affects -undici 8.8.0–8.9.0, and Node 26 bundles 8.9.0. +The motivating case is a transport bug in the runtime's own HTTP stack, which +you can't fix from inside the SDK — for example +[undici #5600](https://github.com/nodejs/undici/issues/5600): reusing a +keep-alive socket while the event loop is idle stalls the request before it +goes out. The upstream report measured ~450–465 ms against a 10 ms server; our +own measurements on 8.9.0 saw it swing erratically from ~100 ms to ~2.5 s +depending on the idle gap. It affects undici 8.8.0–8.9.0, and Node 26 bundles +8.9.0. -**Upgrade first if you can:** this was fixed in undici 8.10.0, so a runtime with -8.10.0 or newer needs no workaround. Reach for `options.fetch` when you're -pinned to an affected version — a dispatcher with a tuned `keepAliveTimeout` -sidesteps it: +**Upgrading undici is the actual fix** — it landed in 8.10.0. What +`options.fetch` buys you is a way to do that without waiting for a new runtime: +install undici yourself and route requests through it, so the version bundled +with your Node never handles them. ```ts import { createClient } from '@wavehouse/sdk'; -import { Agent, fetch as undiciFetch } from 'undici'; // npm install undici - -// Retire idle sockets sooner than the runtime's default -const dispatcher = new Agent({ keepAliveTimeout: 1_000 }); +import { fetch as undiciFetch } from 'undici'; // npm install undici — 8.10.0+ const wh = createClient({ baseURL: 'https://wavehouse.example.com', options: { - fetch: (url, init) => - undiciFetch(url, { ...init, dispatcher } as never) as unknown as Promise, + fetch: (url, init) => undiciFetch(url, init as never) as unknown as Promise, }, }); ``` @@ -426,9 +425,17 @@ structurally assignable and TypeScript rejects the direct form. They're safe because of the narrow contract above. `undici` is a dependency you add — the SDK itself stays dependency-free. -If you only need the dispatcher (and not middleware), `setGlobalDispatcher(new -Agent({ keepAliveTimeout: 1_000 }))` achieves the same thing process-wide -without `options.fetch`. +If you're genuinely pinned to an affected undici, a dispatcher that never reuses +a keep-alive socket avoids the stall — `new Agent({ pipelining: 0 })`, passed as +`dispatcher` in the `RequestInit`. It costs you a fresh connection per request, +so prefer upgrading. Note that tuning `keepAliveTimeout` does **not** help: the +retirement timer is starved by the same idle event loop that triggers the bug, +so the socket is still there to be reused. + +To change the transport process-wide instead of per-client, an installed undici's +`setGlobalDispatcher(new Agent())` is picked up by Node's built-in global +`fetch`, no `options.fetch` needed — but that only helps if the *installed* +undici is 8.10.0+, since it's the bundled implementation doing the stalling. ### Type-Safe Tables From 186b09a194daf0ae1715cc4abba94d7bd85df2b6 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 10:29:27 -0400 Subject: [PATCH 06/18] docs(sdk): fix the setGlobalDispatcher rationale, note test stubbing Fourth pre-push review round. The setGlobalDispatcher caveat ended "since it's the bundled implementation doing the stalling", which argues for the opposite conclusion: if the bundled copy is what stalls, a reader infers that swapping the global dispatcher cannot help -- which would also undercut the options.fetch remedy above it. The mechanism is that setGlobalDispatcher hands connection handling to the installed copy, so that copy is the one that has to carry the fix. Reworded to say so. Also adds stubbing HTTP in your own tests to the list of reasons to reach for the option. It is probably the most common one, and the CHANGELOG entry already called it out while the page did not. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- docs/src/content/docs/sdk/index.mdx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index f7622b77..d9246c59 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -353,8 +353,9 @@ The proxy in front must **strip** the prefix before forwarding, since WaveHouse `options.fetch` replaces the HTTP implementation the SDK uses. Reach for it to route through a proxy, attach client certificates, wrap requests in your own -middleware (logging, tracing, circuit breaking), or to work around transport -behavior of the runtime you happen to be on: +middleware (logging, tracing, circuit breaking), stub HTTP in your own tests +without monkey-patching a global, or to work around transport behavior of the +runtime you happen to be on: ```ts const wh = createClient({ @@ -435,8 +436,9 @@ so the socket is still there to be reused. To change the transport process-wide instead of per-client, an installed undici's `setGlobalDispatcher(new Agent())` is picked up by Node's built-in global -`fetch`, no `options.fetch` needed — but that only helps if the *installed* -undici is 8.10.0+, since it's the bundled implementation doing the stalling. +`fetch`, no `options.fetch` needed — but only if the *installed* undici is +8.10.0+: `setGlobalDispatcher` hands connection handling to that copy, so it's +the one that has to carry the fix. ### Type-Safe Tables From 2f1b16144ee7c2300f096d4f69bad9f8c6a48723 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 10:53:21 -0400 Subject: [PATCH 07/18] docs(sdk): drop the too-narrow ceiling on the undici stall range The text claimed our measurements ranged "~100 ms to ~2.5 s". That ceiling is an artifact of one setup: npm-installed undici 8.9.0 on Node 22, where gaps of 4s+ show no stall at all because the default 4s keepAliveTimeout has already retired the socket, so there is nothing to reuse. Measuring against Node 26's bundled 8.9.0 through the SDK tells a different story -- 6s gaps against a stub server answering instantly produced [12, 23990, 2203, 2100, 2099] ms. I can't reproduce that from Node 22 with an npm-installed undici, so rather than assert a bound I can't defend in both directions, the docs now say severity varies with runtime and idle gap, quote the upstream ~450-465ms figure, and give our own range as ~100ms to tens of seconds. This also keeps the entry consistent with #455's account of the same bug, which reported multi-second stalls from the e2e harness's multi-second lingers. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- docs/src/content/docs/sdk/index.mdx | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e99ebfd9..9c3974ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. The upstream report measured ~450–465 ms; measuring 8.9.0 while writing this, it ranged erratically from ~100 ms to ~2.5 s depending on the idle gap. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, so the bundled copy never handles the request. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. `FetchLike` is deliberately narrower than `typeof fetch` — it takes a `string` URL, which is all the SDK ever passes — so consumer middleware declared as `(url: string, init?: RequestInit) => Promise` assigns without a cast, where `typeof fetch` would reject it on parameter contravariance. Implementations shipping their own request/response declarations (undici, `node-fetch`) do still need a cast, since those types are separate from the ones behind the global `fetch`; the narrow documented contract is what makes it safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. +- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, so the bundled copy never handles the request. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. `FetchLike` is deliberately narrower than `typeof fetch` — it takes a `string` URL, which is all the SDK ever passes — so consumer middleware declared as `(url: string, init?: RequestInit) => Promise` assigns without a cast, where `typeof fetch` would reject it on parameter contravariance. Implementations shipping their own request/response declarations (undici, `node-fetch`) do still need a cast, since those types are separate from the ones behind the global `fetch`; the narrow documented contract is what makes it safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index d9246c59..2fb661d6 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -399,10 +399,10 @@ The motivating case is a transport bug in the runtime's own HTTP stack, which you can't fix from inside the SDK — for example [undici #5600](https://github.com/nodejs/undici/issues/5600): reusing a keep-alive socket while the event loop is idle stalls the request before it -goes out. The upstream report measured ~450–465 ms against a 10 ms server; our -own measurements on 8.9.0 saw it swing erratically from ~100 ms to ~2.5 s -depending on the idle gap. It affects undici 8.8.0–8.9.0, and Node 26 bundles -8.9.0. +goes out. How bad it gets varies with the runtime and the idle gap: the +upstream report measured ~450–465 ms against a 10 ms server, and we have +measured anything from ~100 ms to tens of seconds on a server answering +instantly. It affects undici 8.8.0–8.9.0, and Node 26 bundles 8.9.0. **Upgrading undici is the actual fix** — it landed in 8.10.0. What `options.fetch` buys you is a way to do that without waiting for a new runtime: From 896d85ccdfd173517e80bbc490627eb5c5033e5e Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 11:17:14 -0400 Subject: [PATCH 08/18] test(sdk): drop the FetchLike casts, show the pipelining:0 wrapper Both from CodeRabbit on #456. The override tests still carried `as unknown as FetchLike` from when FetchLike was typeof fetch and a bare vi.fn() couldn't satisfy it. Since the narrowing that is no longer true, so the casts were vestigial -- and a cast in the test undercuts the claim that a consumer needs none. Now `vi.fn(...)`, which also types the recorded call, so the init assertions are checked against RequestInit instead of any. The pinned-to-an-affected-undici escape was described in prose but not shown, and it is the one case where you do have to merge a dispatcher into the init the SDK hands you -- the main example no longer does, since the remedy there is to bring your own fixed undici. Added the snippet; typechecked under both lib configs. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- clients/ts/src/client.test.ts | 18 ++++++++++-------- docs/src/content/docs/sdk/index.mdx | 25 ++++++++++++++++++++----- 2 files changed, 30 insertions(+), 13 deletions(-) diff --git a/clients/ts/src/client.test.ts b/clients/ts/src/client.test.ts index 1553654a..a4786aa5 100644 --- a/clients/ts/src/client.test.ts +++ b/clients/ts/src/client.test.ts @@ -147,11 +147,13 @@ describe("WaveHouseClient.sql()", () => { describe("options.fetch", () => { it("routes requests through a supplied fetch instead of the global", async () => { - const custom = vi.fn().mockResolvedValue(new Response(JSON.stringify([]), { status: 200 })); + // Typed as FetchLike rather than cast: a consumer's override needs no cast, + // so the test shouldn't need one either. + const custom = vi.fn(async () => new Response(JSON.stringify([]), { status: 200 })); const client = createClient({ baseURL: "http://localhost:8080", auth: () => "test-token", - options: { fetch: custom as unknown as FetchLike }, + options: { fetch: custom }, }); await client.from("clicks").select("*").limit(1); @@ -162,13 +164,13 @@ describe("options.fetch", () => { // proxy/middleware consumers rely on the whole request reaching them. const [url, init] = custom.mock.calls[0]; expect(typeof url).toBe("string"); - expect(String(url)).toContain("http://localhost:8080"); - expect(init.method).toBe("POST"); - expect(init.headers).toMatchObject({ + expect(url).toContain("http://localhost:8080"); + expect(init?.method).toBe("POST"); + expect(init?.headers).toMatchObject({ "Content-Type": "application/json", Authorization: "Bearer test-token", }); - expect(typeof init.body).toBe("string"); + expect(typeof init?.body).toBe("string"); }); it("falls back to the global fetch when no override is given", async () => { @@ -192,12 +194,12 @@ describe("options.fetch", () => { it("applies the override to retries too, not just the first attempt", async () => { const custom = vi - .fn() + .fn() .mockResolvedValueOnce(new Response("boom", { status: 500 })) .mockResolvedValueOnce(new Response(JSON.stringify([]), { status: 200 })); const client = createClient({ baseURL: "http://localhost:8080", - options: { fetch: custom as unknown as FetchLike, maxRetries: 1 }, + options: { fetch: custom, maxRetries: 1 }, }); await client.from("clicks").select("*").limit(1); diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 2fb661d6..d90559b8 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -428,11 +428,26 @@ because of the narrow contract above. `undici` is a dependency you add — the S itself stays dependency-free. If you're genuinely pinned to an affected undici, a dispatcher that never reuses -a keep-alive socket avoids the stall — `new Agent({ pipelining: 0 })`, passed as -`dispatcher` in the `RequestInit`. It costs you a fresh connection per request, -so prefer upgrading. Note that tuning `keepAliveTimeout` does **not** help: the -retirement timer is starved by the same idle event loop that triggers the bug, -so the socket is still there to be reused. +a keep-alive socket avoids the stall. Merge it into the `init` the SDK hands +you, as undici's `dispatcher` option: + +```ts +import { Agent, fetch as undiciFetch } from 'undici'; + +const dispatcher = new Agent({ pipelining: 0 }); // no keep-alive reuse + +const wh = createClient({ + baseURL: 'https://wavehouse.example.com', + options: { + fetch: (url, init) => + undiciFetch(url, { ...init, dispatcher } as never) as unknown as Promise, + }, +}); +``` + +That costs a fresh connection per request, so prefer upgrading. Note that tuning +`keepAliveTimeout` does **not** help: the retirement timer is starved by the same +idle event loop that triggers the bug, so the socket is still there to be reused. To change the transport process-wide instead of per-client, an installed undici's `setGlobalDispatcher(new Agent())` is picked up by Node's built-in global From 6e7158b5b60773b4fa9111f8e21b3fa03f73a1ba Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 14:30:54 -0400 Subject: [PATCH 09/18] feat(sdk): add options.headers and options.fetchOptions, widen FetchLike Completes #269's REST-side scope. Three knobs now, shaped after the conventions in Supabase's, OpenAI's and Anthropic's clients rather than invented here. options.headers adds static headers to every REST request -- the motivating case being a Cloudflare Access service token in front of WaveHouse. Names match case-insensitively, as HTTP requires, and the merge is deliberately asymmetric: configured headers sit underneath the SDK's own, and a collision drops the configured one rather than joining the two. auth keeps Authorization; a request's Content-Type and Accept can't be displaced by a global. Both rules are lifted from bugs other SDKs shipped -- a global Content-Type joined with an upload's own produced `application/json, image/png` and 415s, and a case-sensitive Authorization check let a lowercase spelling ride alongside the canonical one. options.fetchOptions merges extra RequestInit fields, for what isn't a header and doesn't warrant replacing the transport: credentials: "include" for a cookie-authenticated origin, plus mode/cache and runtime extensions like Next.js's next: { tags }. The fields the SDK controls -- method, headers, body, signal -- are applied after the spread, so this can't corrupt the request. fetchOptions.headers is ignored rather than merged; options.headers is the header channel, and merging both would give two precedence stories for one concept. FetchLike widens to the standard fetch signature. It was narrowed earlier in this branch on the theory that it accepted strictly more, which was only true in one direction: a value typed as the narrow form is rejected by every other SDK's wide option, so a shared wrapper couldn't be handed to both. Every client surveyed uses the wide shape and none carry complaints about it. Still written out rather than `typeof fetch`, which resolves differently with and without DOM in lib. REST-only, stated plainly in the docs rather than left implicit: SSE takes neither headers nor a fetch, so a header-gated deployment can query but not stream until #203. The same gap in Supabase's realtime client was found by a user whose RLS policies silently stopped matching, which is the failure mode worth pre-empting. Verified on the wire against a live server, not just against a mocked fetch: the CF-Access header is sent, auth beats a lowercase `authorization` impostor, and Content-Type stays application/json. Per-call overrides and dynamic header callbacks are deferred to #459. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 4 +- clients/ts/src/client.test.ts | 105 ++++++++++++++++++++++++++++ clients/ts/src/client.ts | 2 + clients/ts/src/http.ts | 34 ++++++++- clients/ts/src/index.ts | 2 +- clients/ts/src/pipes.ts | 4 +- clients/ts/src/query-builder.ts | 6 +- clients/ts/src/table.ts | 4 +- clients/ts/src/types.ts | 67 +++++++++++++----- docs/src/content/docs/sdk/index.mdx | 77 ++++++++++++++++++-- 10 files changed, 270 insertions(+), 35 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c3974ea..ed96d577 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **`options.fetch` — supply your own HTTP implementation to the SDK** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): the SDK called the global `fetch` directly and `ClientOptions` exposed only `maxRetries`, so a consumer had no way to influence how requests are made. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, so the bundled copy never handles the request. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. `FetchLike` is deliberately narrower than `typeof fetch` — it takes a `string` URL, which is all the SDK ever passes — so consumer middleware declared as `(url: string, init?: RequestInit) => Promise` assigns without a cast, where `typeof fetch` would reject it on parameter contravariance. Implementations shipping their own request/response declarations (undici, `node-fetch`) do still need a cast, since those types are separate from the ones behind the global `fetch`; the narrow documented contract is what makes it safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. +- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, so the bundled copy never handles the request. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. `FetchLike` is deliberately narrower than `typeof fetch` — it takes a `string` URL, which is all the SDK ever passes — so consumer middleware declared as `(url: string, init?: RequestInit) => Promise` assigns without a cast, where `typeof fetch` would reject it on parameter contravariance. Implementations shipping their own request/response declarations (undici, `node-fetch`) do still need a cast, since those types are separate from the ones behind the global `fetch`; the narrow documented contract is what makes it safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. @@ -25,6 +25,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- **BREAKING (SDK): `FetchOptions` is renamed `RequestOptions`** (`clients/ts/src/types.ts`, `clients/ts/src/index.ts`, `clients/ts/src/query-builder.ts`, `clients/ts/src/table.ts`, `clients/ts/src/pipes.ts`): the per-call options type accepted by `.fetch()`. The old name collided conceptually with the new `options.fetchOptions` — which, following OpenAI, Anthropic, and the wider ecosystem, means "extra `RequestInit` fields", not "options for our `.fetch()` method". Shipping both would have left `FetchOptions` and `fetchOptions` in the same SDK one capital letter apart, meaning unrelated things. `RequestOptions` is what Anthropic's SDK calls the identical concept. No deprecated alias: the type is unreferenced by anything consuming the pre-1.0 package, and keeping it would preserve exactly the ambiguity the rename removes. Renaming the import is the whole migration. The module-private `RequestOptions` in `http.ts` — the internal request descriptor — becomes `RequestSpec` to free the name. + - **`@wavehouse/sdk` `engines.node` floor back to `>=22`, matching the only line we test** (`clients/ts/package.json`, `clients/ts/README.md`, `docs/src/content/docs/sdk/index.mdx`, `docs/src/content/docs/sdk/queries.md`, `pnpm-workspace.yaml`): the floor was relaxed to `>=18` when the browser-first distribution landed (see the entry below), on the reasoning that the runtime needs only `fetch`. Nothing ever tested 18, though — `.nvmrc` pins 22 and `.github/actions/setup-env` consumes it via `node-version-file`, so 22 is the single version CI exercises — and Node 18 and 20 have both since reached upstream end-of-life. Declaring a floor we neither test nor is supported upstream promises more than it can back, so it returns to `>=22`. **Consumer impact:** installing on Node < 22 now warns with `EBADENGINE` under npm, and fails outright under pnpm with `engine-strict` enabled. The SDK README and the docs' Runtime support section state the requirement, which they previously either omitted or quoted as 18. - **Live SSE events are now projected and serialized once per role instead of once per subscriber** (`internal/stream/hub.go` (new), `internal/stream/{subscriber,bucket,heartbeat,metrics,doc}.go`, `internal/api/stream.go`, `internal/api/hub.go` + `internal/api/transform.go` (both removed — the broadcast hub moves to `internal/stream`, and the orphaned test-only `transformForClient` is dropped), `cmd/wavehouse/main.go`, `docs/src/content/docs/architecture.md`, `AGENTS.md`, plus tests in `internal/stream/{hub,filter,subscriber,bucket,heartbeat}_test.go` and `internal/api/{stream,transform,router,errors}_test.go`): the first PR of the SSE delivery-path throughput epic ([#294](https://github.com/Wave-RF/WaveHouse/issues/294)), building on the `internal/stream` primitives from #346. The broadcast hub moves into `internal/stream` as `Hub`: subscribers register under `(topic, role)`, and `Broadcast` decodes each event **once**, applies each subscribed role's column policy **once**, builds one SSE frame per role, and fans it to every member of that role's `Bucket`. Previously every connection independently ran `json.Unmarshal → policy.Evaluate → filterEventColumns → json.Marshal` (plus a second unmarshal just to read the `id:` timestamp) on the *same* event in its own read loop — byte-identical work repeated N times. For a single-role audience (the public dashboard, every viewer `public`) that collapses N re-projections to 1, moving the measured ~2 270 deliveries/s ceiling toward an events/s ceiling. The `(topic, role)` key is sufficient and claims-independent: column visibility derives only from the role+table policy entry, and the stream path applies no row-level filter (a documented invariant — if row-level filtering is ever added to streaming, the key must take claims into account). The handler's two `select` cases (keepalive vs. per-subscriber event) collapse into one byte-pump over a single `Subscriber.Frames()` queue carrying typed `Frame`s; the subscriber queue grows from cap 1 (keepalive-only) to 64 so live events buffer while the handler is mid-write. Gap-fill replay and `Last-Event-ID`/`?since=` resumption are unchanged (replay stays per-connection via the shared `stream.ReplayFrame`; live frames carry the same `id: `). Slow-consumer drops, silent before, now increment `wavehouse_sse_dropped_frames_total`; an inert `Subscriber.Evicted()` seam is wired for the eviction follow-up. The per-delivery OpenTelemetry span (another #294 item) was already removed in #346. **Deferred to follow-ups:** active slow-consumer eviction (#94) and right-sizing the subscriber buffer + broadcast lock cost (#152). diff --git a/clients/ts/src/client.test.ts b/clients/ts/src/client.test.ts index a4786aa5..85001aee 100644 --- a/clients/ts/src/client.test.ts +++ b/clients/ts/src/client.test.ts @@ -208,3 +208,108 @@ describe("options.fetch", () => { expect(fetchSpy).not.toHaveBeenCalled(); }); }); + +describe("options.headers", () => { + const headersOf = (call: number) => + fetchSpy.mock.calls[call][1].headers as Record; + + it("adds configured headers to every request", async () => { + const client = createClient({ + baseURL: "http://localhost:8080", + options: { headers: { "CF-Access-Client-Id": "abc.access", "X-Tenant": "acme" } }, + }); + + await client.from("clicks").select("*").limit(1); + await client.from("clicks").select("*").limit(1); + + for (const call of [0, 1]) { + expect(headersOf(call)).toMatchObject({ + "CF-Access-Client-Id": "abc.access", + "X-Tenant": "acme", + }); + } + }); + + it("cannot override the Content-Type a request needs", async () => { + // A global Content-Type outranking the request's own is a documented way to + // break uploads — it must lose, not merge. + const client = createClient({ + baseURL: "http://localhost:8080", + options: { headers: { "Content-Type": "text/plain" } }, + }); + + await client.from("clicks").select("*").limit(1); + + expect(headersOf(0)["Content-Type"]).toBe("application/json"); + expect(Object.values(headersOf(0))).not.toContain("text/plain"); + }); + + it("cannot displace the auth token, even under different casing", async () => { + const client = createClient({ + baseURL: "http://localhost:8080", + auth: () => "real-token", + options: { headers: { authorization: "Bearer impostor" } }, + }); + + await client.from("clicks").select("*").limit(1); + + const sent = headersOf(0); + expect(sent.Authorization).toBe("Bearer real-token"); + // The lowercase spelling must not ride along beside the canonical one. + expect(sent.authorization).toBeUndefined(); + }); + + it("still applies when no auth is configured", async () => { + const client = createClient({ + baseURL: "http://localhost:8080", + options: { headers: { "X-Tenant": "acme" } }, + }); + + await client.from("clicks").select("*").limit(1); + + expect(headersOf(0)["X-Tenant"]).toBe("acme"); + expect(headersOf(0).Authorization).toBeUndefined(); + }); +}); + +describe("options.fetchOptions", () => { + it("merges configured RequestInit fields into every request", async () => { + const client = createClient({ + baseURL: "http://localhost:8080", + options: { fetchOptions: { credentials: "include", cache: "no-store" } }, + }); + + await client.from("clicks").select("*").limit(1); + + const init = fetchSpy.mock.calls[0][1]; + expect(init.credentials).toBe("include"); + expect(init.cache).toBe("no-store"); + }); + + it("cannot corrupt the fields the SDK controls", async () => { + const client = createClient({ + baseURL: "http://localhost:8080", + auth: () => "test-token", + options: { + headers: { "X-Tenant": "acme" }, + fetchOptions: { + method: "DELETE", + body: "hijacked", + headers: { "X-Tenant": "overridden", Authorization: "Bearer impostor" }, + } as RequestInit, + }, + }); + + await client.from("clicks").select("*").limit(1); + + const init = fetchSpy.mock.calls[0][1]; + expect(init.method).toBe("POST"); + expect(init.body).not.toBe("hijacked"); + // options.headers is the header channel; fetchOptions.headers is discarded + // wholesale rather than merged, so it can't smuggle an Authorization past auth. + expect(init.headers).toMatchObject({ + "X-Tenant": "acme", + Authorization: "Bearer test-token", + }); + }); +}); diff --git a/clients/ts/src/client.ts b/clients/ts/src/client.ts index 8147e1ca..bb3987a9 100644 --- a/clients/ts/src/client.ts +++ b/clients/ts/src/client.ts @@ -38,6 +38,8 @@ export class WaveHouseClient { options: { maxRetries: config.options?.maxRetries ?? 2, fetch: config.options?.fetch, + headers: config.options?.headers, + fetchOptions: config.options?.fetchOptions, }, }; diff --git a/clients/ts/src/http.ts b/clients/ts/src/http.ts index 44e47cb9..aef3056b 100644 --- a/clients/ts/src/http.ts +++ b/clients/ts/src/http.ts @@ -2,7 +2,7 @@ import { networkError, parseErrorResponse } from "./errors.js"; import type { HttpContext, WaveHouseError } from "./types.js"; import { resolveURL } from "./url.js"; -interface RequestOptions { +interface RequestSpec { method: string; path: string; body?: unknown; @@ -25,11 +25,32 @@ export interface HttpResult { headers: Headers; } +/** + * Merge configured headers underneath the SDK's own, matching names + * case-insensitively so a caller's `authorization` can't sit alongside our + * `Authorization` and let the server pick. Canonical casing wins, and values + * replace rather than append — a header joined instead of replaced is a known + * way to produce `Content-Type: application/json, image/png`. + */ +function mergeHeaders( + base: Record, + extra: Record | undefined, +): Record { + if (!extra) return base; + const taken = new Map(Object.keys(base).map((k) => [k.toLowerCase(), k])); + const merged = { ...base }; + for (const [name, value] of Object.entries(extra)) { + // Skip rather than overwrite: `base` is the SDK's own, which outranks. + if (!taken.has(name.toLowerCase())) merged[name] = value; + } + return merged; +} + /** * Internal fetch wrapper with auth injection, retry, backoff, and Retry-After. * @internal */ -export async function request(ctx: HttpContext, opts: RequestOptions): Promise> { +export async function request(ctx: HttpContext, opts: RequestSpec): Promise> { const url = resolveURL(ctx.baseURL, opts.path, opts.params).toString(); const headers: Record = { "Content-Type": opts.contentType ?? "application/json", @@ -51,14 +72,21 @@ export async function request(ctx: HttpContext, opts: RequestOptions): Promis } } + // After auth, so `auth` keeps ownership of Authorization, and after the + // Content-Type/Accept this request needs. + const finalHeaders = mergeHeaders(headers, ctx.options.headers); + let lastError: WaveHouseError | null = null; const maxAttempts = ctx.options.maxRetries + 1; for (let attempt = 0; attempt < maxAttempts; attempt++) { try { + // Configured RequestInit first, so the fields the SDK controls overwrite + // it — a supplied `method` or `body` would otherwise corrupt the request. const init: RequestInit = { + ...ctx.options.fetchOptions, method: opts.method, - headers, + headers: finalHeaders, body: requestBody, signal: opts.signal, }; diff --git a/clients/ts/src/index.ts b/clients/ts/src/index.ts index 2b305fcf..2756cf29 100644 --- a/clients/ts/src/index.ts +++ b/clients/ts/src/index.ts @@ -30,7 +30,6 @@ export type { DLQStats, // HTTP FetchLike, - FetchOptions, // Query FilterOp, // Ingest @@ -44,6 +43,7 @@ export type { Policy, PolicyFilter, QueryFilter, + RequestOptions, Result, RolePermissions, Schemas, diff --git a/clients/ts/src/pipes.ts b/clients/ts/src/pipes.ts index 38835594..81c2d855 100644 --- a/clients/ts/src/pipes.ts +++ b/clients/ts/src/pipes.ts @@ -1,7 +1,7 @@ import { err, ok } from "./errors.js"; import { request } from "./http.js"; import type { StreamController } from "./stream/controller.js"; -import type { FetchOptions, HttpContext, Pipe, Result, StreamOptions } from "./types.js"; +import type { HttpContext, Pipe, RequestOptions, Result, StreamOptions } from "./types.js"; type CreateStreamFn = (table: string, opts?: StreamOptions) => StreamController; @@ -25,7 +25,7 @@ export class PipeRef> implements PromiseLike> { + async fetch(opts?: RequestOptions): Promise> { const { data, error } = await request(this._ctx, { method: "POST", path: `/v1/pipes/${encodeURIComponent(this._name)}`, diff --git a/clients/ts/src/query-builder.ts b/clients/ts/src/query-builder.ts index b77b9192..e5ef1af1 100644 --- a/clients/ts/src/query-builder.ts +++ b/clients/ts/src/query-builder.ts @@ -5,11 +5,11 @@ import { StreamController } from "./stream/controller.js"; import { LiveQuery } from "./stream/live-query.js"; import type { Aggregation, - FetchOptions, FilterOp, HttpContext, OrderClause, QueryFilter, + RequestOptions, Result, StreamOptions, StreamSubscriber, @@ -137,7 +137,7 @@ export class QueryBuilder> implements PromiseLike< /** Default row limit when none is specified — deliberately tighter than the backend's DefaultMaxRows (10000) safety cap. */ static readonly DEFAULT_LIMIT = 1000; - async fetch(opts?: FetchOptions): Promise> { + async fetch(opts?: RequestOptions): Promise> { const effectiveLimit = opts?.limit ?? this._state.limit ?? QueryBuilder.DEFAULT_LIMIT; const ast = this._buildAST(effectiveLimit); @@ -239,7 +239,7 @@ export class QueryBuilder> implements PromiseLike< private async _fetchNext( prevRows: Row[], _limit: number, - opts?: FetchOptions, + opts?: RequestOptions, ): Promise> { // No explicit order → no keyset cursor to build; nothing to page by. const cursor = this._state.orderBy[0]; diff --git a/clients/ts/src/table.ts b/clients/ts/src/table.ts index f44c05e3..18c53146 100644 --- a/clients/ts/src/table.ts +++ b/clients/ts/src/table.ts @@ -3,10 +3,10 @@ import { request } from "./http.js"; import { QueryBuilder } from "./query-builder.js"; import type { StreamController } from "./stream/controller.js"; import type { - FetchOptions, HttpContext, InsertRecordResult, InsertResult, + RequestOptions, Result, StreamOptions, TableSchema, @@ -58,7 +58,7 @@ export class TableRef> { } /** SELECT * shortcut — fetches rows with optional pagination. */ - async fetch(opts?: FetchOptions): Promise> { + async fetch(opts?: RequestOptions): Promise> { return this.select() .limit(opts?.limit ?? 1000) .fetch(opts); diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index 51aeaa23..c4befdbe 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -83,25 +83,23 @@ export interface ClientConfig<_DB extends Database = Database> { /** * A `fetch`-compatible function. * - * Deliberately narrower than `typeof fetch`: the SDK only ever calls it with a - * string URL, so this is what it actually requires. The global `fetch` still - * assigns to it, and so does middleware you declare as - * `(url: string, init?: RequestInit) => Promise` — which `typeof - * fetch` would reject, since a parameter accepting only `string` is not - * assignable to one accepting `RequestInfo | URL`. + * Spelled out rather than written `typeof fetch`, which resolves differently + * depending on whether the consumer's TypeScript `lib` includes DOM — the same + * signature either way, but stable across configurations. * - * The response is read for `.ok` and `.headers` always, `.text()` on success, - * and `.status`, `.statusText` plus `.json()` when it is not `ok`. A rejection - * becomes a `NETWORK_ERROR` result and is retried with backoff — except a - * `DOMException` named `AbortError` (what the platform `fetch` and undici throw - * on abort), which becomes `ABORTED` and is not retried. Implementations that - * reject with some other abort error, such as `node-fetch`, are retried instead. + * In practice the SDK only ever calls it with a string URL, and reads `.ok` and + * `.headers` always, `.text()` on success, and `.status`, `.statusText` plus + * `.json()` when the response is not `ok`. A rejection becomes a + * `NETWORK_ERROR` result and is retried with backoff — except a `DOMException` + * named `AbortError` (what the platform `fetch` and undici throw on abort), + * which becomes `ABORTED` and is not retried. Implementations that reject with + * some other abort error, such as `node-fetch`, are retried instead. * * Implementations shipping their own request/response declarations (undici, - * `node-fetch`) still need a cast, since those types are separate from the ones + * `node-fetch`) need a cast, since those types are separate from the ones * behind your global `fetch`; the narrow contract above is what makes it safe. */ -export type FetchLike = (url: string, init?: RequestInit) => Promise; +export type FetchLike = (input: string | URL | Request, init?: RequestInit) => Promise; export interface ClientOptions { /** Maximum retry attempts for failed requests. Default: 2. */ @@ -132,6 +130,28 @@ export interface ClientOptions { * through your function. */ fetch?: FetchLike; + /** + * Headers added to every REST request (SSE streams excluded) — for a + * header-gated proxy in front of WaveHouse, such as a Cloudflare Access + * service token. + * + * Matched case-insensitively, as HTTP headers are. Applied underneath the + * SDK's own headers: `auth` still owns `Authorization`, and the + * `Content-Type` and `Accept` a given request needs are not overridable + * here — a global `Content-Type` that outranked the request's own is a + * documented way to break uploads. + */ + headers?: Record; + /** + * Extra `RequestInit` fields merged into every REST request — `credentials` + * for a cookie-authenticated origin, `mode`, `cache`, `keepalive`, or a + * runtime-specific extension such as Next.js's `next: { tags }`. + * + * Fields the SDK controls (`method`, `headers`, `body`, `signal`) always + * win, so this cannot corrupt the request itself. Non-standard fields may + * need a cast, since `RequestInit` only declares the standard ones. + */ + fetchOptions?: RequestInit; } // --- Structured query AST (matches backend wire format) --- @@ -305,9 +325,15 @@ export interface ValidationResult { valid: boolean; } -// --- Fetch options --- +// --- Per-call request options --- -export interface FetchOptions { +/** + * Options for a single call, passed to `.fetch()`. + * + * Note that `await`ing a builder directly (`await wh.from("t").select("*")`) + * takes no options — use the explicit `.fetch({ … })` form for those. + */ +export interface RequestOptions { signal?: AbortSignal; limit?: number; } @@ -325,6 +351,11 @@ export interface StreamOptions { export interface HttpContext { baseURL: string; auth?: () => Promise | string; - // Stays optional rather than defaulted here, to keep the global late-bound. - options: { maxRetries: number; fetch?: FetchLike }; + // `fetch` stays optional rather than defaulted here, to keep the global late-bound. + options: { + maxRetries: number; + fetch?: FetchLike; + headers?: Record; + fetchOptions?: RequestInit; + }; } diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index d90559b8..3d507a9c 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -327,7 +327,13 @@ const wh = createClient({ | `baseURL` | `string` | — | WaveHouse server URL, optionally including a path prefix (required) | | `auth` | `() => Promise \| string` | — | Token provider. Omit for public access | | `options.maxRetries` | `number` | `2` | Retry attempts for failed/5xx requests | -| `options.fetch` | `FetchLike` | global `fetch` | HTTP implementation for every REST request; SSE streams excluded ([details](#supplying-your-own-fetch)) | +| `options.headers` | `Record` | — | Headers added to every REST request ([details](#custom-headers)) | +| `options.fetchOptions` | `RequestInit` | — | Extra `RequestInit` fields merged into every REST request ([details](#extra-requestinit-fields)) | +| `options.fetch` | `FetchLike` | global `fetch` | HTTP implementation for every REST request ([details](#supplying-your-own-fetch)) | + +:::caution[These three apply to REST only] +`headers`, `fetchOptions`, and `fetch` do **not** reach the streaming transport. `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither custom headers nor a `fetch` — which is why the token rides in `?token=` there. If WaveHouse sits behind a header-gated proxy, REST works and streaming does not; follow [#203](https://github.com/Wave-RF/WaveHouse/issues/203) for that. (`.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered.) +::: :::note[How the token is transmitted] The SDK attaches your `auth` token as an `Authorization: Bearer` header on REST calls, and — because the browser `EventSource` API can't set headers — as a `?token=` query parameter on streaming connections. When both are present the server prefers the header, and it strips `?token=` from the request URL either way — so an unused credential can't survive into a later handler's log line, and the token stays out of **WaveHouse's own** logs. @@ -349,6 +355,66 @@ Trailing slashes on `baseURL` are optional, but it must be **absolute** — sche The proxy in front must **strip** the prefix before forwarding, since WaveHouse itself always serves at `/v1/…` — see [Behind a reverse proxy → Path prefixes](/reverse-proxy#path-prefixes). +### Custom headers + +`options.headers` adds headers to every REST request — the usual reason being a +header-gated proxy in front of WaveHouse, such as a Cloudflare Access service +token: + +```ts +const wh = createClient({ + baseURL: 'https://wavehouse.example.com', + options: { + headers: { + 'CF-Access-Client-Id': process.env.CF_ACCESS_CLIENT_ID!, + 'CF-Access-Client-Secret': process.env.CF_ACCESS_CLIENT_SECRET!, + }, + }, +}); +``` + +Header names are matched **case-insensitively**, as HTTP requires, so +`authorization` and `Authorization` are the same header rather than two. + +Your headers are applied *underneath* the SDK's own, and a collision means yours +is dropped rather than merged: + +- **`auth` keeps `Authorization`.** Setting it here won't displace your token + provider. Use `auth` for credentials. +- **`Content-Type` and `Accept` belong to the request.** The SDK knows what it's + sending; a global `Content-Type` that outranked it would break requests whose + body isn't JSON. + +Values replace, never append — a header joined rather than replaced is how you +end up sending `Content-Type: application/json, image/png`. + +Headers are static. For a credential that rotates per request, wrap the +transport with [`options.fetch`](#supplying-your-own-fetch); a callback form is +tracked in [#459](https://github.com/Wave-RF/WaveHouse/issues/459). + +### Extra `RequestInit` fields + +`options.fetchOptions` is merged into the `RequestInit` of every REST request — +for settings that aren't headers and don't warrant replacing the whole +transport. The common case is a cookie-authenticated origin, where the default +`credentials: 'same-origin'` means a cross-origin browser request sends no +cookie at all: + +```ts +const wh = createClient({ + baseURL: 'https://wavehouse.example.com', + options: { fetchOptions: { credentials: 'include' } }, +}); +``` + +It also carries `mode`, `cache`, `keepalive`, and runtime-specific extensions +such as Next.js's `next: { tags }` — those aren't declared on the standard +`RequestInit`, so they may need a cast. + +The fields the SDK controls — `method`, `headers`, `body`, and `signal` — always +win, so this can't corrupt the request itself. In particular `headers` here is +ignored; use `options.headers`, which merges properly. + ### Supplying your own `fetch` `options.fetch` replaces the HTTP implementation the SDK uses. Reach for it to @@ -383,10 +449,11 @@ platform `fetch` and undici throw on abort — which becomes `ABORTED` and is no retried. An implementation that signals abort some other way (`node-fetch` throws its own `AbortError` class) gets retried instead. -The option's type is exported as `FetchLike`, and is deliberately narrower than -`typeof fetch` — it takes a `string` URL, so your own -`(url: string, init?: RequestInit) => Promise` middleware assigns to it -without a cast. +The option's type is exported as `FetchLike`. It matches the standard `fetch` +signature — written out rather than as `typeof fetch`, which resolves +differently depending on whether your TypeScript `lib` includes DOM. Let +TypeScript infer your parameters (`(url, init) => …`) and it fits without +annotation. Only the SSE transport is exempt: the live connection behind `.stream()` and `.liveQuery()` uses `EventSource`, which this does not replace. `.liveQuery()`'s From adb228b7f60744954ccf40f9c7537d624995217e Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 14:49:02 -0400 Subject: [PATCH 10/18] fix(sdk): collapse same-header casings, repair undici snippets after widening Review round on the expanded scope. Three real defects, two of them caused by the FetchLike widening in the previous commit. mergeHeaders matched case-insensitively against the SDK's headers but not within the configured set, so `{ "x-tenant": "a", "X-Tenant": "b" }` survived as two keys and the Headers constructor comma-joined them at fetch time -- `x-tenant: a, b`. Verified on Node. That is the exact corruption the function's own docblock cites as its reason to exist, so it was worth closing rather than documenting. Configured names now replace each other, last spelling wins, with a test. Widening FetchLike broke both undici snippets and I did not re-check them -- I verified the new headers/fetchOptions examples and assumed the existing ones were unaffected. They were not: `url` is now `string | URL | Request`, which is not assignable to undici's RequestInfo whenever DOM is in lib, i.e. the TypeScript default. Reproduced as TS2345 against undici 7.28 and undici-types 8.3. Both snippets and the matching JSDoc now cast the URL, and the "casts are load-bearing" paragraph explains why there are two. The CHANGELOG entry still carried the pre-widening rationale and contradicted itself a few sentences apart -- claiming FetchLike both matches the standard signature and is deliberately narrower than it. The narrow claim is now false against the shipped type. CHANGELOG.md sits outside scripts/docs-prose.sh, so no gate would have caught it. Also corrected in the docs: - credentials: "include" was the headline fetchOptions example, and it cannot work against a stock WaveHouse. corsMiddleware deliberately never emits Access-Control-Allow-Credentials (it is a Bearer-token API, asserted by a test in router_test.go), and the default cors_allowed_origins "*" makes include a hard browser failure anyway. Now led by cache/keepalive, with credentials kept as a qualified note pointing at the proxy-owns-CORS case. - Custom headers from a browser must pass CORS preflight, and Access-Control-Allow-Headers is a fixed list with no config knob, so a CF-Access header only works server-side or behind a proxy that owns CORS. Said outright rather than left for someone to discover. - "Values replace, never append" described nothing observable, since a collision drops the configured value rather than replacing the SDK's. Reworded to what actually happens. - mode/cache/keepalive were lumped in with "not declared on the standard RequestInit". They are standard; only the runtime extensions need a cast. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- clients/ts/src/client.test.ts | 16 +++++++++ clients/ts/src/http.ts | 13 +++++-- clients/ts/src/types.ts | 2 +- docs/src/content/docs/sdk/index.mdx | 53 ++++++++++++++++++++--------- 5 files changed, 66 insertions(+), 20 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ed96d577..5fdf2c44 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, so the bundled copy never handles the request. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. `FetchLike` is deliberately narrower than `typeof fetch` — it takes a `string` URL, which is all the SDK ever passes — so consumer middleware declared as `(url: string, init?: RequestInit) => Promise` assigns without a cast, where `typeof fetch` would reject it on parameter contravariance. Implementations shipping their own request/response declarations (undici, `node-fetch`) do still need a cast, since those types are separate from the ones behind the global `fetch`; the narrow documented contract is what makes it safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. +- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, so the bundled copy never handles the request. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Implementations shipping their own request/response declarations (undici, `node-fetch`) need casts on both the URL argument and the return value, since those types are separate from the ones behind the global `fetch`; the narrow documented runtime contract is what makes them safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. diff --git a/clients/ts/src/client.test.ts b/clients/ts/src/client.test.ts index 85001aee..50d6dae7 100644 --- a/clients/ts/src/client.test.ts +++ b/clients/ts/src/client.test.ts @@ -259,6 +259,22 @@ describe("options.headers", () => { expect(sent.authorization).toBeUndefined(); }); + it("collapses two configured spellings of one header instead of sending both", async () => { + // Left as separate keys, the Headers constructor comma-joins them at fetch + // time — `x-tenant: acme, beta` — which is the corruption this guards. + const client = createClient({ + baseURL: "http://localhost:8080", + options: { headers: { "x-tenant": "acme", "X-Tenant": "beta" } }, + }); + + await client.from("clicks").select("*").limit(1); + + const sent = headersOf(0); + const spellings = Object.keys(sent).filter((k) => k.toLowerCase() === "x-tenant"); + expect(spellings).toHaveLength(1); + expect(new Headers(sent).get("x-tenant")).toBe("beta"); + }); + it("still applies when no auth is configured", async () => { const client = createClient({ baseURL: "http://localhost:8080", diff --git a/clients/ts/src/http.ts b/clients/ts/src/http.ts index aef3056b..905a8556 100644 --- a/clients/ts/src/http.ts +++ b/clients/ts/src/http.ts @@ -37,11 +37,20 @@ function mergeHeaders( extra: Record | undefined, ): Record { if (!extra) return base; - const taken = new Map(Object.keys(base).map((k) => [k.toLowerCase(), k])); + const sdkNames = new Set(Object.keys(base).map((k) => k.toLowerCase())); const merged = { ...base }; + // Spelling each configured name was last stored under, so two entries + // differing only in case replace each other here rather than surviving as + // separate keys for `Headers` to comma-join at fetch time. + const configured = new Map(); for (const [name, value] of Object.entries(extra)) { + const lower = name.toLowerCase(); // Skip rather than overwrite: `base` is the SDK's own, which outranks. - if (!taken.has(name.toLowerCase())) merged[name] = value; + if (sdkNames.has(lower)) continue; + const prior = configured.get(lower); + if (prior !== undefined) delete merged[prior]; + merged[name] = value; + configured.set(lower, name); } return merged; } diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index c4befdbe..9fb79add 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -119,7 +119,7 @@ export interface ClientOptions { * createClient({ * baseURL, * options: { - * fetch: (url, init) => undiciFetch(url, init as never) as unknown as Promise, + * fetch: (url, init) => undiciFetch(url as string, init as never) as unknown as Promise, * }, * }); * ``` diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 3d507a9c..9cddb0ce 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -385,8 +385,16 @@ is dropped rather than merged: sending; a global `Content-Type` that outranked it would break requests whose body isn't JSON. -Values replace, never append — a header joined rather than replaced is how you -end up sending `Content-Type: application/json, image/png`. +Nothing is ever comma-joined: on a collision the SDK's value stands alone, and +two of your own entries differing only in case collapse to the last one — a +header joined rather than replaced is how you end up sending +`Content-Type: application/json, image/png`. + +From a **browser**, a cross-origin custom header must also survive CORS +preflight, and WaveHouse allow-lists a fixed set (`Accept`, `Authorization`, +`Content-Type`, `Last-Event-ID`, `X-Request-ID`) with no config knob. So custom +headers work server-side, or from a browser when the proxy in front owns CORS — +which is the same proxy the header is usually for. Headers are static. For a credential that rotates per request, wrap the transport with [`options.fetch`](#supplying-your-own-fetch); a callback form is @@ -396,20 +404,30 @@ tracked in [#459](https://github.com/Wave-RF/WaveHouse/issues/459). `options.fetchOptions` is merged into the `RequestInit` of every REST request — for settings that aren't headers and don't warrant replacing the whole -transport. The common case is a cookie-authenticated origin, where the default -`credentials: 'same-origin'` means a cross-origin browser request sends no -cookie at all: +transport: ```ts const wh = createClient({ baseURL: 'https://wavehouse.example.com', - options: { fetchOptions: { credentials: 'include' } }, + options: { fetchOptions: { cache: 'no-store', keepalive: true } }, }); ``` -It also carries `mode`, `cache`, `keepalive`, and runtime-specific extensions -such as Next.js's `next: { tags }` — those aren't declared on the standard -`RequestInit`, so they may need a cast. +It carries any `RequestInit` field — `mode`, `cache`, `keepalive`, +`credentials`, `redirect` — plus runtime-specific extensions such as Next.js's +`next: { tags }` for per-deployment cache control. Only those runtime +extensions need a cast; the standard fields are declared on `RequestInit` +already. + +:::note[`credentials: 'include'` needs a proxy that owns CORS] +Sending cookies cross-origin is a common reason to reach for this, but it can't +work against WaveHouse's own CORS: it deliberately never emits +`Access-Control-Allow-Credentials` (it's a Bearer-token API), and the default +`cors_allowed_origins: "*"` makes `include` a hard browser failure regardless. +It applies where a fronting proxy answers CORS itself — see +[Behind a reverse proxy](/reverse-proxy#header-and-auth-forwarding). Same-origin +deployments already send cookies without it. +::: The fields the SDK controls — `method`, `headers`, `body`, and `signal` — always win, so this can't corrupt the request itself. In particular `headers` here is @@ -483,16 +501,19 @@ import { fetch as undiciFetch } from 'undici'; // npm install undici — 8.10.0+ const wh = createClient({ baseURL: 'https://wavehouse.example.com', options: { - fetch: (url, init) => undiciFetch(url, init as never) as unknown as Promise, + fetch: (url, init) => undiciFetch(url as string, init as never) as unknown as Promise, }, }); ``` -The casts are load-bearing: undici declares its own `Request`/`Response` types, -separate from the ones behind your global `fetch`, so the two aren't -structurally assignable and TypeScript rejects the direct form. They're safe -because of the narrow contract above. `undici` is a dependency you add — the SDK -itself stays dependency-free. +Both casts are load-bearing, for the same underlying reason: undici declares its +own `Request`/`Response` types, separate from the ones behind your global +`fetch`, so the two aren't structurally assignable. The return cast handles the +`Response` mismatch; `url as string` is needed because `FetchLike` accepts the +full `fetch` input union while undici's `RequestInfo` names its *own* `Request`. +They're safe because of the narrow runtime contract above — the SDK only ever +passes a string. `undici` is a dependency you add; the SDK itself stays +dependency-free. If you're genuinely pinned to an affected undici, a dispatcher that never reuses a keep-alive socket avoids the stall. Merge it into the `init` the SDK hands @@ -507,7 +528,7 @@ const wh = createClient({ baseURL: 'https://wavehouse.example.com', options: { fetch: (url, init) => - undiciFetch(url, { ...init, dispatcher } as never) as unknown as Promise, + undiciFetch(url as string, { ...init, dispatcher } as never) as unknown as Promise, }, }); ``` From 5e9b1cbac1842cbb321007e3f7cf23a2b2116ba9 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 15:09:04 -0400 Subject: [PATCH 11/18] fix(docs): the undici remedy needs an explicit dispatcher to work at all Docs review found that the documented workaround does not do what it claims, and measurement confirms it. Third time the undici guidance in this branch has been wrong in a way only measurement caught. The section said: install undici 8.10.0 yourself and route through it, "so the version bundled with your Node never handles them". That is not what happens. undici keeps its connection pool on a shared globalThis symbol -- Symbol(undici.globalDispatcher.2) -- and the first copy loaded claims it, which on an affected runtime is the bundled one. fetch() resolves that global when no dispatcher is passed, so an installed 8.10.0's fetch dispatches through the bundled 8.9.0's pool. Reproduced with both copies loaded side by side. new.getGlobalDispatcher() returns an instance of old.Agent, and the timings follow, 1.5s idle gaps against a 10ms server: no explicit dispatcher 21 1514 1495 583 explicit new Agent() 17 14 12 13 So the snippet now passes the dispatcher explicitly, which never consults the shared symbol, and a caution block explains why with those numbers. Verified end to end through the SDK with 8.9.0 loaded first: 24, 15, 13, 14 ms. This also collapses the section, since the pinned-to-an-affected-version case was already passing an explicit dispatcher -- it is now the same snippet with different Agent options rather than a second one. The setGlobalDispatcher note gains the reason it works: an explicit write wins the same race. Also from the same review: - "Both casts are load-bearing" undercounted the snippet, which has three. init as never was left unexplained, which is the one that looks most alarming. All three are now named, with what each bridges. - Next.js next: { tags } was described as per-deployment cache control. It tags a cache entry for on-demand revalidation via revalidateTag(). Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- clients/ts/src/types.ts | 14 ++++-- docs/src/content/docs/sdk/index.mdx | 73 ++++++++++++++++------------- 3 files changed, 52 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5fdf2c44..7e5ceea6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, so the bundled copy never handles the request. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Implementations shipping their own request/response declarations (undici, `node-fetch`) need casts on both the URL argument and the return value, since those types are separate from the ones behind the global `fetch`; the narrow documented runtime contract is what makes them safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. +- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, passing its dispatcher **explicitly**. That last part is load-bearing and not obvious — undici keeps its connection pool on a shared `globalThis` symbol claimed by whichever copy loads first, so calling an installed 8.10.0's `fetch` without a `dispatcher` still resolves the bundled 8.9.0's pool and still stalls. Measured with both copies loaded, 1.5 s idle gaps against a 10 ms server: `21, 1514, 1495, 583 ms` with an implied dispatcher versus `17, 14, 12, 13 ms` with an explicit `new Agent()`. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Implementations shipping their own request/response declarations (undici, `node-fetch`) need casts on both the URL argument and the return value, since those types are separate from the ones behind the global `fetch`; the narrow documented runtime contract is what makes them safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index 9fb79add..109ad04b 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -96,8 +96,9 @@ export interface ClientConfig<_DB extends Database = Database> { * some other abort error, such as `node-fetch`, are retried instead. * * Implementations shipping their own request/response declarations (undici, - * `node-fetch`) need a cast, since those types are separate from the ones - * behind your global `fetch`; the narrow contract above is what makes it safe. + * `node-fetch`) need casts on the URL, the init and the return value, since + * those types are separate from the ones behind your global `fetch`; the + * narrow runtime contract above is what makes them safe. */ export type FetchLike = (input: string | URL | Request, init?: RequestInit) => Promise; @@ -115,11 +116,16 @@ export interface ClientOptions { * {@link https://github.com/nodejs/undici/issues/5600 | undici #5600}: * * ```ts - * import { fetch as undiciFetch } from "undici"; // npm install undici — 8.10.0+ + * import { Agent, fetch as undiciFetch } from "undici"; // npm install undici — 8.10.0+ + * // Pass the dispatcher explicitly: undici's pool lives on a shared + * // globalThis symbol claimed by whichever copy loaded first, so an implied + * // dispatcher can still be the runtime's affected one. + * const dispatcher = new Agent(); * createClient({ * baseURL, * options: { - * fetch: (url, init) => undiciFetch(url as string, init as never) as unknown as Promise, + * fetch: (url, init) => + * undiciFetch(url as string, { ...init, dispatcher } as never) as unknown as Promise, * }, * }); * ``` diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 9cddb0ce..521a5b4c 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -415,7 +415,7 @@ const wh = createClient({ It carries any `RequestInit` field — `mode`, `cache`, `keepalive`, `credentials`, `redirect` — plus runtime-specific extensions such as Next.js's -`next: { tags }` for per-deployment cache control. Only those runtime +`next: { tags }`, which tags the cached response for on-demand revalidation. Only those runtime extensions need a cast; the standard fields are declared on `RequestInit` already. @@ -490,58 +490,67 @@ measured anything from ~100 ms to tens of seconds on a server answering instantly. It affects undici 8.8.0–8.9.0, and Node 26 bundles 8.9.0. **Upgrading undici is the actual fix** — it landed in 8.10.0. What -`options.fetch` buys you is a way to do that without waiting for a new runtime: -install undici yourself and route requests through it, so the version bundled -with your Node never handles them. +`options.fetch` buys you is a way to get there without waiting for a new +runtime: install undici yourself and route requests through it. The one +non-obvious part is that you must pass its dispatcher **explicitly**. ```ts import { createClient } from '@wavehouse/sdk'; -import { fetch as undiciFetch } from 'undici'; // npm install undici — 8.10.0+ +import { Agent, fetch as undiciFetch } from 'undici'; // npm install undici — 8.10.0+ + +// Explicit, not implied — see below. +const dispatcher = new Agent(); const wh = createClient({ baseURL: 'https://wavehouse.example.com', options: { - fetch: (url, init) => undiciFetch(url as string, init as never) as unknown as Promise, + fetch: (url, init) => + undiciFetch(url as string, { ...init, dispatcher } as never) as unknown as Promise, }, }); ``` -Both casts are load-bearing, for the same underlying reason: undici declares its -own `Request`/`Response` types, separate from the ones behind your global -`fetch`, so the two aren't structurally assignable. The return cast handles the -`Response` mismatch; `url as string` is needed because `FetchLike` accepts the -full `fetch` input union while undici's `RequestInfo` names its *own* `Request`. -They're safe because of the narrow runtime contract above — the SDK only ever -passes a string. `undici` is a dependency you add; the SDK itself stays -dependency-free. - -If you're genuinely pinned to an affected undici, a dispatcher that never reuses -a keep-alive socket avoids the stall. Merge it into the `init` the SDK hands -you, as undici's `dispatcher` option: +:::caution[Importing a fixed undici is not enough on its own] +undici keeps its connection pool on a shared `globalThis` symbol, and the first +copy loaded claims it — which on an affected runtime is the *bundled* one. Call +`undiciFetch` without a `dispatcher` and it resolves the global one, so requests +can still go through the buggy pool even though you imported a fixed undici. -```ts -import { Agent, fetch as undiciFetch } from 'undici'; - -const dispatcher = new Agent({ pipelining: 0 }); // no keep-alive reuse +Measured with 8.9.0 loaded first and 8.10.0's `fetch` doing the request, 1.5 s +idle gaps against a 10 ms server: -const wh = createClient({ - baseURL: 'https://wavehouse.example.com', - options: { - fetch: (url, init) => - undiciFetch(url as string, { ...init, dispatcher } as never) as unknown as Promise, - }, -}); ``` +no explicit dispatcher 21 1514 1495 583 ← still stalling +explicit new Agent() 17 14 12 13 +``` + +An explicit `dispatcher` (or an explicit `setGlobalDispatcher` call, below) +always wins, because it never consults the shared symbol. +::: +All three casts are load-bearing, for one underlying reason: undici declares its +own request/response types, separate from the ones behind your global `fetch`, +so the two aren't structurally assignable. `url as string` is needed because +`FetchLike` accepts the full `fetch` input union while undici's `RequestInfo` +names its *own* `Request`; `init as never` covers the same split on +`RequestInit`, which differs on `body` and `headers` (`never` is assignable to +either spelling, so one snippet compiles whether or not your `lib` includes +DOM); and the return cast handles the `Response` mismatch. They're safe because +of the narrow runtime contract above — a string URL and a plain `RequestInit` +in, six members read back. `undici` is a dependency you add; the SDK itself +stays dependency-free. + +If you're genuinely pinned to an affected undici, the same snippet fixes it with +a dispatcher that never reuses a keep-alive socket — `new Agent({ pipelining: 0 })`. That costs a fresh connection per request, so prefer upgrading. Note that tuning `keepAliveTimeout` does **not** help: the retirement timer is starved by the same idle event loop that triggers the bug, so the socket is still there to be reused. To change the transport process-wide instead of per-client, an installed undici's `setGlobalDispatcher(new Agent())` is picked up by Node's built-in global -`fetch`, no `options.fetch` needed — but only if the *installed* undici is -8.10.0+: `setGlobalDispatcher` hands connection handling to that copy, so it's -the one that has to carry the fix. +`fetch`, no `options.fetch` needed. That's an explicit write, so it wins the +race described above — but only if the *installed* undici is 8.10.0+, since it +hands connection handling to that copy. ### Type-Safe Tables From 7b47a9d9eecbf002f2e092454b81eef9a6eaa128 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 15:20:51 -0400 Subject: [PATCH 12/18] docs(sdk): split the two reasons an explicit dispatcher wins Review round on the previous fix. The caution said an explicit dispatcher and an explicit setGlobalDispatcher both win "because it never consults the shared symbol". That is only true of the dispatcher. setGlobalDispatcher wins the opposite way -- it overwrites the symbol after the first copy claimed it -- which is what the paragraph further down already said, so the page contradicted itself and left no way to understand why a call on the installed copy affects the bundled fetch at all. Reasons are now split. The casts paragraph still spelled the init cast `init as never`, which was the pre-dispatcher snippet's text. The snippet now casts the merged object. Beyond not matching, copying the cast as the prose spelled it would drop `dispatcher` and land back on the stalling global pool that the caution three lines above exists to prevent. Also took the reviewer's suggestion on the pinned-version paragraph: it reuses a snippet whose comment reads "8.10.0+", which is the opposite of that reader's situation, so it now says to use your pinned version with pipelining: 0. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- docs/src/content/docs/sdk/index.mdx | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 521a5b4c..266adf2d 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -524,25 +524,27 @@ no explicit dispatcher 21 1514 1495 583 ← still stalling explicit new Agent() 17 14 12 13 ``` -An explicit `dispatcher` (or an explicit `setGlobalDispatcher` call, below) -always wins, because it never consults the shared symbol. +An explicit `dispatcher` wins because the request never consults the shared +symbol at all. An explicit `setGlobalDispatcher` call (below) wins the other +way round — it overwrites the symbol after the first copy claimed it. ::: All three casts are load-bearing, for one underlying reason: undici declares its own request/response types, separate from the ones behind your global `fetch`, so the two aren't structurally assignable. `url as string` is needed because `FetchLike` accepts the full `fetch` input union while undici's `RequestInfo` -names its *own* `Request`; `init as never` covers the same split on -`RequestInit`, which differs on `body` and `headers` (`never` is assignable to -either spelling, so one snippet compiles whether or not your `lib` includes -DOM); and the return cast handles the `Response` mismatch. They're safe because +names its *own* `Request`; `{ ...init, dispatcher } as never` covers the same +split on `RequestInit`, which differs on `body` and `headers` (`never` is +assignable to either spelling, so one snippet compiles whether or not your `lib` +includes DOM); and the return cast handles the `Response` mismatch. They're safe because of the narrow runtime contract above — a string URL and a plain `RequestInit` in, six members read back. `undici` is a dependency you add; the SDK itself stays dependency-free. -If you're genuinely pinned to an affected undici, the same snippet fixes it with -a dispatcher that never reuses a keep-alive socket — `new Agent({ pipelining: 0 })`. -That costs a fresh connection per request, so prefer upgrading. Note that tuning +If you're genuinely pinned to an affected undici, the same snippet fixes it — +your pinned version, with a dispatcher that never reuses a keep-alive socket: +`new Agent({ pipelining: 0 })`. That costs a fresh connection per request, so +prefer upgrading. Note that tuning `keepAliveTimeout` does **not** help: the retirement timer is starved by the same idle event loop that triggers the bug, so the socket is still there to be reused. From 755c753ca02e72e10a0bdb49a66cc7392f23c9f4 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 15:36:38 -0400 Subject: [PATCH 13/18] docs(sdk): don't claim the bundled undici always wins the pool The caution said the shared globalThis symbol is claimed by "the first copy loaded -- which on an affected runtime is the bundled one". The second half is wrong, and wrong in the ordering the snippet above it creates. Node does not claim the symbol at startup; it loads its bundled undici on the first call to the built-in fetch. Verified locally: no undici symbol exists on globalThis at startup, and Symbol(undici.globalDispatcher.1) appears only after a fetch() call. Since ESM imports evaluate before any user code runs, `import ... from "undici"` normally makes the INSTALLED copy the claimant. My earlier measurement forced the bundled-first ordering with an explicit `import "old"`, which is why it looked deterministic. The measurement sentence already says "with 8.9.0 loaded first", so it was honest; the surrounding claim was not. Reworded to what holds: whichever copy loads first claims it, Node claims for the bundled copy on the first built-in fetch rather than at startup, so ownership comes down to a load order you don't really control -- which is a better argument for passing the dispatcher explicitly than the deterministic version was, since it removes the dependency instead of betting on it. Same softening in the CHANGELOG; the JSDoc already hedged correctly. Also labels the measurement block's units. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- docs/src/content/docs/sdk/index.mdx | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7e5ceea6..c3dbdc93 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, passing its dispatcher **explicitly**. That last part is load-bearing and not obvious — undici keeps its connection pool on a shared `globalThis` symbol claimed by whichever copy loads first, so calling an installed 8.10.0's `fetch` without a `dispatcher` still resolves the bundled 8.9.0's pool and still stalls. Measured with both copies loaded, 1.5 s idle gaps against a 10 ms server: `21, 1514, 1495, 583 ms` with an implied dispatcher versus `17, 14, 12, 13 ms` with an explicit `new Agent()`. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Implementations shipping their own request/response declarations (undici, `node-fetch`) need casts on both the URL argument and the return value, since those types are separate from the ones behind the global `fetch`; the narrow documented runtime contract is what makes them safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. +- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, passing its dispatcher **explicitly**. That last part is load-bearing and not obvious — undici keeps its connection pool on a shared `globalThis` symbol claimed by whichever copy loads first (Node claims it for the bundled copy on the first built-in `fetch` call, not at startup), so calling an installed 8.10.0's `fetch` without a `dispatcher` resolves whatever is on that symbol and can still stall through the bundled 8.9.0's pool. Measured with both copies loaded, 1.5 s idle gaps against a 10 ms server: `21, 1514, 1495, 583 ms` with an implied dispatcher versus `17, 14, 12, 13 ms` with an explicit `new Agent()`. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Implementations shipping their own request/response declarations (undici, `node-fetch`) need casts on both the URL argument and the return value, since those types are separate from the ones behind the global `fetch`; the narrow documented runtime contract is what makes them safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 266adf2d..808fb486 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -511,15 +511,18 @@ const wh = createClient({ ``` :::caution[Importing a fixed undici is not enough on its own] -undici keeps its connection pool on a shared `globalThis` symbol, and the first -copy loaded claims it — which on an affected runtime is the *bundled* one. Call -`undiciFetch` without a `dispatcher` and it resolves the global one, so requests -can still go through the buggy pool even though you imported a fixed undici. +undici keeps its connection pool on a shared `globalThis` symbol, and whichever +copy loads first claims it. Node claims it for the *bundled* copy the first time +anything calls the built-in `fetch` — not at startup — so which undici owns the +pool comes down to a load order you don't really control. Call `undiciFetch` +without a `dispatcher` and it resolves whatever is on that symbol, so requests +can go through the buggy pool even though you imported a fixed undici. Measured with 8.9.0 loaded first and 8.10.0's `fetch` doing the request, 1.5 s idle gaps against a 10 ms server: ``` + per-request ms no explicit dispatcher 21 1514 1495 583 ← still stalling explicit new Agent() 17 14 12 13 ``` From d1132644d60b50c919619ff1ed2c17c462c8e05b Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 15:53:15 -0400 Subject: [PATCH 14/18] docs(sdk): widen what claims undici's pool, fix the cast count Two leftovers from the previous round. The caution named a built-in fetch() call as what makes Node claim the shared dispatcher symbol. Too narrow. Verified on Node 22: new Headers(), new Response(), new Request(), new FormData() and even reading globalThis.WebSocket all claim it; only reading globalThis.fetch without calling does not. That matters because the caution exists to stop a reader reasoning about the ordering -- someone who audits "I never call the built-in fetch, everything goes through undiciFetch" gets a false all-clear. Worth saying outright that this SDK is one such claimant: http.ts constructs a Headers on the abort and retry-exhausted paths. The CHANGELOG still said undici needs casts on "both the URL argument and the return value". There are three -- the init cast is unavoidable regardless of the DOM-lib split, since dispatcher isn't a RequestInit field at all. Same undercount already fixed in the docs and already correct in the JSDoc, so the PR was contradicting itself across its own artifacts. CHANGELOG.md is excluded from scripts/docs-prose.sh by name, which is why this is the second factual drift to survive there. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- docs/src/content/docs/sdk/index.mdx | 12 ++++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3dbdc93..d52ff27a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,7 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added -- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, passing its dispatcher **explicitly**. That last part is load-bearing and not obvious — undici keeps its connection pool on a shared `globalThis` symbol claimed by whichever copy loads first (Node claims it for the bundled copy on the first built-in `fetch` call, not at startup), so calling an installed 8.10.0's `fetch` without a `dispatcher` resolves whatever is on that symbol and can still stall through the bundled 8.9.0's pool. Measured with both copies loaded, 1.5 s idle gaps against a 10 ms server: `21, 1514, 1495, 583 ms` with an implied dispatcher versus `17, 14, 12, 13 ms` with an explicit `new Agent()`. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Implementations shipping their own request/response declarations (undici, `node-fetch`) need casts on both the URL argument and the return value, since those types are separate from the ones behind the global `fetch`; the narrow documented runtime contract is what makes them safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. +- **HTTP customization for the SDK — `options.headers`, `options.fetchOptions`, and `options.fetch`** (`clients/ts/src/types.ts`, `clients/ts/src/client.ts`, `clients/ts/src/http.ts`, `clients/ts/src/index.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/index.mdx`): closes #269. `ClientOptions` exposed only `maxRetries`, so a WaveHouse behind a header-gated proxy (Cloudflare Access, an mTLS sidecar, an auth gateway) or a cookie-authenticated origin was unreachable from the SDK — a defense-in-depth gate forced consumers off the client entirely. Three knobs land together, shaped after the conventions in Supabase's, OpenAI's, and Anthropic's clients rather than invented here. **`headers`** adds static headers to every REST request; names match case-insensitively as HTTP requires, and they apply *underneath* the SDK's own — `auth` keeps `Authorization`, and a request's `Content-Type`/`Accept` can't be displaced by a global one, since a header joined rather than replaced is how you ship `Content-Type: application/json, image/png`. **`fetchOptions`** merges extra `RequestInit` fields (`credentials: "include"` for the cookie case, `mode`, `cache`, or a runtime extension like Next.js's `next: { tags }`); the fields the SDK controls — `method`, `headers`, `body`, `signal` — always win, so it cannot corrupt the request. **`fetch`** replaces the HTTP implementation outright. All three are REST-only: `.stream()` and `.liveQuery()`'s live connection go through `EventSource`, which accepts neither headers nor a `fetch`, so a header-gated deployment can query but not stream until #203 changes that transport — stated plainly in the docs rather than left implicit, since the equivalent gap in Supabase's realtime client was found the hard way by a user whose RLS policies silently stopped matching. `.liveQuery()`'s initial backfill is an ordinary REST call and *is* covered. Per-call overrides and dynamic header callbacks are deliberately deferred to #459; the per-call slot (`.fetch(opts)`) already exists, so adding them later is additive. The exported `FetchLike` matches the standard `fetch` signature, written out rather than as `typeof fetch` because that resolves differently depending on whether the consumer's TypeScript `lib` includes DOM — the same fragility behind Supabase's long tail of `node-fetch` resolution issues. `options.fetch` accepts any `fetch`-compatible function (exported as the `FetchLike` type) and is used for every request, retries included, so middleware sees each attempt. The motivating case is a runtime bug consumers can't fix themselves: undici 8.8.0–8.9.0 stalls a request before it goes out when a keep-alive socket is reused while the event loop is idle ([nodejs/undici#5600](https://github.com/nodejs/undici/issues/5600), fixed in 8.10.0), and Node 26 bundles 8.9.0 — so calls that should take milliseconds don't, with no recourse inside the SDK. Severity varies with the runtime and the idle gap: the upstream report measured ~450–465 ms, and our own runs against an instant-answering server have ranged from ~100 ms to tens of seconds. Upgrading undici is the real fix, and `options.fetch` is how you get it without waiting for a new runtime: install undici yourself and route through it, passing its dispatcher **explicitly**. That last part is load-bearing and not obvious — undici keeps its connection pool on a shared `globalThis` symbol claimed by whichever copy loads first (Node claims it for the bundled copy on the first built-in `fetch` call, not at startup), so calling an installed 8.10.0's `fetch` without a `dispatcher` resolves whatever is on that symbol and can still stall through the bundled 8.9.0's pool. Measured with both copies loaded, 1.5 s idle gaps against a 10 ms server: `21, 1514, 1495, 583 ms` with an implied dispatcher versus `17, 14, 12, 13 ms` with an explicit `new Agent()`. (Tuning `keepAliveTimeout` is *not* a workaround — measured, it changes nothing, because the retirement timer is starved by the same idle event loop; `new Agent({ pipelining: 0 })` does work for anyone pinned to an affected version, at a connection per request.) The same hook covers the ordinary reasons an SDK grows one: proxies, client certificates, tracing or circuit-breaker middleware, and mocking HTTP in a consumer's own tests without monkey-patching a global. `fetch` stays optional all the way through to the internal `HttpContext` rather than being defaulted at construction, so the default path still calls the global directly — that keeps it late-bound (replacing `globalThis.fetch` after a client exists still works, which is what `vi.stubGlobal` does) and avoids invoking a detached `fetch` reference, which is not universally safe; both properties are pinned by tests. Only the SSE transport is exempt: the live connection behind `.stream()`/`.liveQuery()` goes through `EventSource`, which this does not replace — but `.liveQuery()`'s initial backfill is an ordinary request and does go through the supplied function. Implementations shipping their own request/response declarations (undici, `node-fetch`) need casts on the URL argument, the init, and the return value, since those types are separate from the ones behind the global `fetch`; the narrow documented runtime contract is what makes them safe — a string URL and plain `RequestInit` in, and only `.ok`/`.headers` plus `.text()` on success and `.status`/`.statusText`/`.json()` on a non-`ok` response read back. Abort handling is part of that contract: `ABORTED` requires a `DOMException` named `AbortError` (platform `fetch` and undici), and any other rejection is retried as `NETWORK_ERROR`. - **Non-JWT operator key for full-access admin + break-glass recovery** (`internal/auth/auth.go`, `internal/auth/context.go`, `internal/auth/auth_test.go`, `internal/api/router.go`, `internal/api/router_test.go`, `internal/config/config.go`, `internal/config/config_test.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `tests/e2e/fixtures/config.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/access-control.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/deployment.md`, `docs/src/content/docs/development.md`, `docs/src/content/docs/architecture.md`, `docs/src/content/docs/reverse-proxy.mdx`, `AGENTS.md`, `SECURITY.md`): closes #240; partially advances the auth-hardening epic [#228](https://github.com/Wave-RF/WaveHouse/issues/228) and the management/data-plane split [#359](https://github.com/Wave-RF/WaveHouse/issues/359). Adds an optional `auth.operator_key` (`WH_AUTH_OPERATOR_KEY`): a request presenting it — via an `Authorization: Operator ` header (forwarded verbatim by proxies, no collision with Bearer JWTs) or the `X-Operator-Key` alias — is authorized as a full-access platform operator — the whole data plane *and* the `/v1/admin/*` management surface — without minting a JWT and independently of the token verifier, giving the person running the deployment a role-free credential for bootstrap and break-glass. The middleware checks it before the Bearer token with a constant-time comparison (`crypto/subtle`) and stamps two things into the request context: the live `admin_role` (so the policy evaluator's admin bypass grants unrestricted data-plane access while a policy exists) and a platform-operator bit that `RequireAdmin` honors **even when the policy is `nil`/deleted** — the one HTTP path that can restore a wiped policy, which previously required SSH access and a reboot. Empty (the default) disables it, so existing deployments are unchanged; treat it as an admin secret (load from a secret store, serve only over TLS). A successful operator authentication is audit-logged at `INFO`; a request presenting a *non-matching* operator key is logged at `WARN` and counted by a new `wavehouse_auth_operator_key_failures_total` counter (a probing/brute-force signal on the most privileged credential in the system) before falling through to the normal token/default path — the middleware still never rejects. The `Authorization` auth-scheme is matched case-insensitively (RFC 7235) via a shared `authScheme` helper, which also makes the existing `Bearer` JWT scheme case-insensitive (previously it required the canonical `Bearer` casing). Explicitly out of scope, tracked in #359: scoping the operator credential to the management surface only, and capability-scoped admin permissions. - **Missing-dedupe-id observability + optional strict mode** (`internal/api/ingest.go`, `internal/api/ingest_test.go`, `internal/config/config.go`, `cmd/wavehouse/main.go`, `config.yaml`, `deployments/compose/standalone.yaml`, `docs/src/content/docs/configuration.mdx`, `docs/src/content/docs/api.md`, `docs/src/content/docs/architecture.md`): closes #219. With dedupe enabled, a row missing the configured `id_field` can't be deduped — previously it was published with idempotency silently disabled and *no* log or metric, so a producer bug that dropped the id turned off the guarantee for those rows unnoticed. Now every such row is logged at `WARN` and counted by a new `wavehouse_ingest_dedupe_missing_id_total` counter (labeled by `table`), making the loss observable server-side. A new opt-in `dedupe.require_id` (`WH_DEDUPE_REQUIRE_ID`, default `false`) turns that signal into enforcement: a row missing the id is rejected (`400` for a single insert; a per-record failure in a batch) instead of published — a tripwire for producers that must guarantee the id (complements the client-side [#202](https://github.com/Wave-RF/WaveHouse/issues/202)). Default behavior is unchanged. diff --git a/docs/src/content/docs/sdk/index.mdx b/docs/src/content/docs/sdk/index.mdx index 808fb486..78ccd1d8 100644 --- a/docs/src/content/docs/sdk/index.mdx +++ b/docs/src/content/docs/sdk/index.mdx @@ -513,10 +513,14 @@ const wh = createClient({ :::caution[Importing a fixed undici is not enough on its own] undici keeps its connection pool on a shared `globalThis` symbol, and whichever copy loads first claims it. Node claims it for the *bundled* copy the first time -anything calls the built-in `fetch` — not at startup — so which undici owns the -pool comes down to a load order you don't really control. Call `undiciFetch` -without a `dispatcher` and it resolves whatever is on that symbol, so requests -can go through the buggy pool even though you imported a fixed undici. +anything touches one of its web globals — a `fetch()` call, but equally a +`new Headers()` or `new Response()` — not at startup. So which undici owns the +pool comes down to a load order you don't really control, and auditing your own +code for `fetch` calls won't tell you: any dependency can claim it first, and so +can this SDK, which constructs a `Headers` on its abort and retry-exhausted +paths. Call `undiciFetch` without a `dispatcher` and it resolves whatever is on +that symbol, so requests can go through the buggy pool even though you imported +a fixed undici. Measured with 8.9.0 loaded first and 8.10.0's `fetch` doing the request, 1.5 s idle gaps against a 10 ms server: From 5337c63fced745aa6bd0a06f6cf0326dfcb908fe Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 16:32:19 -0400 Subject: [PATCH 15/18] fix(sdk): PipeRef.fetch no longer accepts a limit it silently ignores Closes #464. From CodeRabbit on #456. PipeRef.fetch took the same RequestOptions as the query builder, which carries `limit`, but forwarded only `signal` -- so wh.pipe('x').fetch({ limit: 10 }) compiled, ran, and quietly did nothing. QueryBuilder.fetch and TableRef.fetch both honour it, so the inconsistency sat inside one shared type. There is nothing to forward: internal/api/pipes.go binds the request body as the pipe's parameters via pipes.BindParams, so a row cap only exists if the pipe's own SQL declares one. The fix is the surface, not the plumbing -- fetch now takes Pick, and the JSDoc points at wh.pipe(name, { limit }) as the real route. A @ts-expect-error test pins the rejection so it can't quietly widen back. Pre-existing rather than introduced here, and I first filed it to keep this diff focused. That was the wrong call: this PR renames that exact type and gives it a JSDoc describing it as the options for `.fetch()`, which makes the false advertisement more prominent, not less. The new test also exposed a flaw in the suite's own fixture. fetchSpy resolved a single shared Response, and a Response body can only be read once -- so any test making two requests had the second fail and retry, and an assertion on mock.calls[1] was reading that retry rather than a second request. "adds configured headers to every request" was passing for exactly that wrong reason. The mock now builds a fresh Response per call, and that test asserts a call count of 2 so the distinction is pinned rather than assumed. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- clients/ts/src/client.test.ts | 20 +++++++++++++++++++- clients/ts/src/pipes.ts | 11 +++++++++-- 2 files changed, 28 insertions(+), 3 deletions(-) diff --git a/clients/ts/src/client.test.ts b/clients/ts/src/client.test.ts index 50d6dae7..f704ce7a 100644 --- a/clients/ts/src/client.test.ts +++ b/clients/ts/src/client.test.ts @@ -11,7 +11,12 @@ import type { FetchLike } from "./types.js"; let fetchSpy: ReturnType; beforeEach(() => { - fetchSpy = vi.fn().mockResolvedValue(new Response(JSON.stringify([]), { status: 200 })); + // A fresh Response per call: a Response body can only be read once, so a + // shared instance makes the second request fail and retry, and a test + // asserting on `mock.calls[1]` would silently be reading that retry. + fetchSpy = vi + .fn() + .mockImplementation(() => Promise.resolve(new Response(JSON.stringify([]), { status: 200 }))); vi.stubGlobal("fetch", fetchSpy); }); @@ -114,6 +119,17 @@ describe("WaveHouseClient.pipe()", () => { const pipe = client.pipe("top_pages", { limit: 10 }); expect(typeof pipe.then).toBe("function"); }); + + it("rejects a per-call limit, which the pipes endpoint cannot honour", async () => { + const client = createClient({ baseURL: "http://localhost:8080" }); + // Compile-time assertion: accepting `limit` here would silently drop it, + // since the endpoint binds the body as the pipe's parameters. A row cap + // belongs in the pipe's own SQL, supplied via `wh.pipe(name, { limit })`. + // @ts-expect-error — `limit` is not part of PipeRef.fetch's options + await client.pipe("top_pages").fetch({ limit: 10 }); + // `signal` is accepted. + await client.pipe("top_pages").fetch({ signal: AbortSignal.timeout(1000) }); + }); }); describe("WaveHouseClient.sql()", () => { @@ -222,6 +238,8 @@ describe("options.headers", () => { await client.from("clicks").select("*").limit(1); await client.from("clicks").select("*").limit(1); + // Exactly two — proves call 1 is a second request, not a retry of the first. + expect(fetchSpy).toHaveBeenCalledTimes(2); for (const call of [0, 1]) { expect(headersOf(call)).toMatchObject({ "CF-Access-Client-Id": "abc.access", diff --git a/clients/ts/src/pipes.ts b/clients/ts/src/pipes.ts index 81c2d855..e054e8e3 100644 --- a/clients/ts/src/pipes.ts +++ b/clients/ts/src/pipes.ts @@ -24,8 +24,15 @@ export class PipeRef> implements PromiseLike> { + /** + * Execute the pipe and return results. + * + * Takes only `signal` — deliberately narrower than the `RequestOptions` the + * query builder accepts. The pipes endpoint binds the body as the pipe's + * parameters, so there is no row cap to forward; bound a pipe with a + * `{{limit}}` parameter in its SQL and pass it via `wh.pipe(name, { limit })`. + */ + async fetch(opts?: Pick): Promise> { const { data, error } = await request(this._ctx, { method: "POST", path: `/v1/pipes/${encodeURIComponent(this._name)}`, From a83de0cc8a7e2a25b6c13d37b29dbf2b963ef790 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 16:44:31 -0400 Subject: [PATCH 16/18] docs(sdk): record the PipeRef.fetch narrowing where consumers will look Both reviewers caught the same miss on the previous commit: it changed a public method's signature and documented that only in the JSDoc. CHANGELOG had no entry at all, which is worse than an omission here -- the existing BREAKING entry for the FetchOptions rename lists pipes.ts in its file set and says "renaming the import is the whole migration", so a consumer reading [Unreleased] end to end would conclude the only pipes.ts change was cosmetic and their .fetch({ limit }) still compiles. Added its own BREAKING entry with the migration, and qualified the rename entry so the two don't contradict. sdk/pipes.md documented the method as a bare `.fetch(opts?)` with opts undefined, while sdk/queries.md documents an identically-titled `.fetch(opts?)` with an options table including limit. Someone learning opts on the queries page and carrying it to pipes got a type error with nothing in the docs explaining it. The pipes page now states the narrower type and redirects to the pipe-parameter route, and the API tree in reference.md annotates the entry, which previously rendered byte-identically to the two that do take limit. Also fixed "bound a pipe with a {{limit}} parameter" in the JSDoc, which read as the noun rather than an imperative. No prose anywhere showed `.fetch({ limit })` on a pipe, so no example was broken -- both docs and README already passed limit as a pipe parameter, which is the route the narrowing pushes people toward. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 4 +++- clients/ts/src/pipes.ts | 4 ++-- docs/src/content/docs/sdk/pipes.md | 8 +++++++- docs/src/content/docs/sdk/reference.md | 2 +- 4 files changed, 13 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d52ff27a..8feae2cd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **BREAKING (SDK): `FetchOptions` is renamed `RequestOptions`** (`clients/ts/src/types.ts`, `clients/ts/src/index.ts`, `clients/ts/src/query-builder.ts`, `clients/ts/src/table.ts`, `clients/ts/src/pipes.ts`): the per-call options type accepted by `.fetch()`. The old name collided conceptually with the new `options.fetchOptions` — which, following OpenAI, Anthropic, and the wider ecosystem, means "extra `RequestInit` fields", not "options for our `.fetch()` method". Shipping both would have left `FetchOptions` and `fetchOptions` in the same SDK one capital letter apart, meaning unrelated things. `RequestOptions` is what Anthropic's SDK calls the identical concept. No deprecated alias: the type is unreferenced by anything consuming the pre-1.0 package, and keeping it would preserve exactly the ambiguity the rename removes. Renaming the import is the whole migration. The module-private `RequestOptions` in `http.ts` — the internal request descriptor — becomes `RequestSpec` to free the name. +- **BREAKING (SDK): `PipeRef.fetch` no longer accepts a `limit` it silently ignored** (`clients/ts/src/pipes.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/pipes.md`, `docs/src/content/docs/sdk/reference.md`): closes #464, raised by CodeRabbit on #456. It took the same per-call options type as the query builder — which carries `limit` — but forwarded only `signal`, so `wh.pipe('top_pages').fetch({ limit: 10 })` type-checked, ran, and quietly returned whatever the pipe's SQL returned. `QueryBuilder.fetch` and `TableRef.fetch` both honour `limit`, so the inconsistency sat inside one shared type. There is nothing to forward: the endpoint binds the request body as the pipe's *parameters* (`internal/api/pipes.go` → `pipes.BindParams`), and a key the SQL doesn't declare is ignored, so a client-side row cap is not something the pipes surface offers. The parameter is now `Pick`, making the dead option a compile error rather than a silent no-op, with a `@ts-expect-error` test pinning the rejection. **Migration:** declare a `{{limit}}` parameter in the pipe's SQL and pass it as a pipe parameter — `wh.pipe(name, { limit })` — which is what the docs already showed. Pre-existing rather than introduced by #456, folded in there because that PR renames the type in question. + +- **BREAKING (SDK): `FetchOptions` is renamed `RequestOptions`** (`clients/ts/src/types.ts`, `clients/ts/src/index.ts`, `clients/ts/src/query-builder.ts`, `clients/ts/src/table.ts`, `clients/ts/src/pipes.ts`): the per-call options type accepted by `.fetch()`. The old name collided conceptually with the new `options.fetchOptions` — which, following OpenAI, Anthropic, and the wider ecosystem, means "extra `RequestInit` fields", not "options for our `.fetch()` method". Shipping both would have left `FetchOptions` and `fetchOptions` in the same SDK one capital letter apart, meaning unrelated things. `RequestOptions` is what Anthropic's SDK calls the identical concept. No deprecated alias: the type is unreferenced by anything consuming the pre-1.0 package, and keeping it would preserve exactly the ambiguity the rename removes. Renaming the import is the whole migration for this entry — note the separate `PipeRef.fetch` narrowing above, which is a behavioural break in the same file. The module-private `RequestOptions` in `http.ts` — the internal request descriptor — becomes `RequestSpec` to free the name. - **`@wavehouse/sdk` `engines.node` floor back to `>=22`, matching the only line we test** (`clients/ts/package.json`, `clients/ts/README.md`, `docs/src/content/docs/sdk/index.mdx`, `docs/src/content/docs/sdk/queries.md`, `pnpm-workspace.yaml`): the floor was relaxed to `>=18` when the browser-first distribution landed (see the entry below), on the reasoning that the runtime needs only `fetch`. Nothing ever tested 18, though — `.nvmrc` pins 22 and `.github/actions/setup-env` consumes it via `node-version-file`, so 22 is the single version CI exercises — and Node 18 and 20 have both since reached upstream end-of-life. Declaring a floor we neither test nor is supported upstream promises more than it can back, so it returns to `>=22`. **Consumer impact:** installing on Node < 22 now warns with `EBADENGINE` under npm, and fails outright under pnpm with `engine-strict` enabled. The SDK README and the docs' Runtime support section state the requirement, which they previously either omitted or quoted as 18. diff --git a/clients/ts/src/pipes.ts b/clients/ts/src/pipes.ts index e054e8e3..55b621fd 100644 --- a/clients/ts/src/pipes.ts +++ b/clients/ts/src/pipes.ts @@ -29,8 +29,8 @@ export class PipeRef> implements PromiseLike): Promise> { const { data, error } = await request(this._ctx, { diff --git a/docs/src/content/docs/sdk/pipes.md b/docs/src/content/docs/sdk/pipes.md index 3e1cca2f..4ffb79c2 100644 --- a/docs/src/content/docs/sdk/pipes.md +++ b/docs/src/content/docs/sdk/pipes.md @@ -21,7 +21,13 @@ const { data } = await wh.pipe('top_pages', { start_date: '2026-01-01', limit: 5 ### `.fetch(opts?)` -Execute and return results. +Execute and return results. Takes `{ signal }` only — narrower than the +`.fetch(opts?)` on a [query builder](/sdk/queries), which also accepts `limit`. + +There is no per-call row cap here: the endpoint binds your `params` as the +pipe's parameters, so a limit has to be declared in the pipe's SQL as +`{{limit}}` (see [Named Pipes](/pipes)) and passed as `wh.pipe(name, { limit })`, +as in the example above. ### `.stream(opts?)` diff --git a/docs/src/content/docs/sdk/reference.md b/docs/src/content/docs/sdk/reference.md index 040c4044..71d1727e 100644 --- a/docs/src/content/docs/sdk/reference.md +++ b/docs/src/content/docs/sdk/reference.md @@ -63,7 +63,7 @@ createClient(config) → WaveHouseClient │ ├── .schema() → Promise> │ └── .stream(opts?) → StreamController ├── .pipe(name, params?) → PipeRef (PromiseLike) -│ ├── .fetch(opts?) → Promise> +│ ├── .fetch(opts?) → Promise> // { signal } only — no limit │ └── .stream(opts?) → StreamController ├── .pipes (admin) │ ├── .list() → Promise> From f65f9a8be46b8fd10141d8666b1af3d1a187a831 Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 17:20:21 -0400 Subject: [PATCH 17/18] fix(sdk): close the named-value hole in PipeRef.fetch's limit rejection From CodeRabbit on #456. The previous fix was half a fix. Pick only rejects a fresh object literal. TypeScript's excess-property check does not apply to a variable, so: const opts: RequestOptions = { signal, limit: 10 }; wh.pipe('x').fetch(opts); // compiled, limit silently dropped Verified with tsc: the literal form errored while the named form passed. That leaves the original defect intact for anyone who builds options once and shares them, which is the more realistic shape in real code than an inline literal. Replaced with an exported PipeRequestOptions declaring signal and limit?: never. `never` rather than omitting the property, because omission is exactly what leaves the named case open. Both forms are now pinned by @ts-expect-error tests. The tradeoff, measured rather than assumed: a RequestOptions value now fails to assign even when it carries no limit at runtime, since the declared type still permits one. That is correct -- the compiler cannot know the value is limit-free, and the two option sets genuinely differ -- but it is a real ergonomic cost for a caller sharing one options object across query and pipe fetches, so it is worth stating plainly. Note this reverses a judgement from an earlier review round, which read Pick<>'s permissiveness as a compatibility feature for the wrapper case. It is the same behaviour; the disagreement is whether admitting a silently-ignored limit is forgiving or defective. Given the whole point of #464 is that the type advertised something it ignored, defective. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- clients/ts/src/client.test.ts | 19 +++++++++++++++---- clients/ts/src/index.ts | 1 + clients/ts/src/pipes.ts | 4 ++-- clients/ts/src/types.ts | 17 +++++++++++++++++ docs/src/content/docs/sdk/pipes.md | 6 ++++-- 6 files changed, 40 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8feae2cd..f932e049 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **BREAKING (SDK): `PipeRef.fetch` no longer accepts a `limit` it silently ignored** (`clients/ts/src/pipes.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/pipes.md`, `docs/src/content/docs/sdk/reference.md`): closes #464, raised by CodeRabbit on #456. It took the same per-call options type as the query builder — which carries `limit` — but forwarded only `signal`, so `wh.pipe('top_pages').fetch({ limit: 10 })` type-checked, ran, and quietly returned whatever the pipe's SQL returned. `QueryBuilder.fetch` and `TableRef.fetch` both honour `limit`, so the inconsistency sat inside one shared type. There is nothing to forward: the endpoint binds the request body as the pipe's *parameters* (`internal/api/pipes.go` → `pipes.BindParams`), and a key the SQL doesn't declare is ignored, so a client-side row cap is not something the pipes surface offers. The parameter is now `Pick`, making the dead option a compile error rather than a silent no-op, with a `@ts-expect-error` test pinning the rejection. **Migration:** declare a `{{limit}}` parameter in the pipe's SQL and pass it as a pipe parameter — `wh.pipe(name, { limit })` — which is what the docs already showed. Pre-existing rather than introduced by #456, folded in there because that PR renames the type in question. +- **BREAKING (SDK): `PipeRef.fetch` no longer accepts a `limit` it silently ignored** (`clients/ts/src/pipes.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/pipes.md`, `docs/src/content/docs/sdk/reference.md`): closes #464, raised by CodeRabbit on #456. It took the same per-call options type as the query builder — which carries `limit` — but forwarded only `signal`, so `wh.pipe('top_pages').fetch({ limit: 10 })` type-checked, ran, and quietly returned whatever the pipe's SQL returned. `QueryBuilder.fetch` and `TableRef.fetch` both honour `limit`, so the inconsistency sat inside one shared type. There is nothing to forward: the endpoint binds the request body as the pipe's *parameters* (`internal/api/pipes.go` → `pipes.BindParams`), and a key the SQL doesn't declare is ignored, so a client-side row cap is not something the pipes surface offers. The parameter is now a dedicated `PipeRequestOptions` (exported) declaring `signal?: AbortSignal` and `limit?: never`, making the dead option a compile error rather than a silent no-op. `never` rather than simply omitting `limit`, because omitting it only rejects fresh object literals — TypeScript's excess-property check doesn't apply to a *variable*, so a shared `const opts: RequestOptions` carrying a limit would still have passed and still been dropped, which is the defect rather than a narrower version of it. Both cases are pinned by `@ts-expect-error` tests. **Migration:** declare a `{{limit}}` parameter in the pipe's SQL and pass it as a pipe parameter — `wh.pipe(name, { limit })` — which is what the docs already showed. Pre-existing rather than introduced by #456, folded in there because that PR renames the type in question. - **BREAKING (SDK): `FetchOptions` is renamed `RequestOptions`** (`clients/ts/src/types.ts`, `clients/ts/src/index.ts`, `clients/ts/src/query-builder.ts`, `clients/ts/src/table.ts`, `clients/ts/src/pipes.ts`): the per-call options type accepted by `.fetch()`. The old name collided conceptually with the new `options.fetchOptions` — which, following OpenAI, Anthropic, and the wider ecosystem, means "extra `RequestInit` fields", not "options for our `.fetch()` method". Shipping both would have left `FetchOptions` and `fetchOptions` in the same SDK one capital letter apart, meaning unrelated things. `RequestOptions` is what Anthropic's SDK calls the identical concept. No deprecated alias: the type is unreferenced by anything consuming the pre-1.0 package, and keeping it would preserve exactly the ambiguity the rename removes. Renaming the import is the whole migration for this entry — note the separate `PipeRef.fetch` narrowing above, which is a behavioural break in the same file. The module-private `RequestOptions` in `http.ts` — the internal request descriptor — becomes `RequestSpec` to free the name. diff --git a/clients/ts/src/client.test.ts b/clients/ts/src/client.test.ts index f704ce7a..9fa69ab5 100644 --- a/clients/ts/src/client.test.ts +++ b/clients/ts/src/client.test.ts @@ -6,7 +6,7 @@ import { PolicyNamespace } from "./policy.js"; import { SchemaNamespace } from "./schema.js"; import { SysNamespace } from "./sys.js"; import { TableRef } from "./table.js"; -import type { FetchLike } from "./types.js"; +import type { FetchLike, PipeRequestOptions, RequestOptions } from "./types.js"; let fetchSpy: ReturnType; @@ -122,13 +122,24 @@ describe("WaveHouseClient.pipe()", () => { it("rejects a per-call limit, which the pipes endpoint cannot honour", async () => { const client = createClient({ baseURL: "http://localhost:8080" }); - // Compile-time assertion: accepting `limit` here would silently drop it, + // Compile-time assertions: accepting `limit` here would silently drop it, // since the endpoint binds the body as the pipe's parameters. A row cap // belongs in the pipe's own SQL, supplied via `wh.pipe(name, { limit })`. - // @ts-expect-error — `limit` is not part of PipeRef.fetch's options + + // @ts-expect-error — as a fresh literal await client.pipe("top_pages").fetch({ limit: 10 }); - // `signal` is accepted. + + // ...and as a named value. This is the case a plain `Pick<>` would let + // through, since excess-property checking only rejects literals — so the + // limit would reach the endpoint, be ignored, and never be flagged. + const shared: RequestOptions = { signal: AbortSignal.timeout(1000), limit: 10 }; + // @ts-expect-error — `limit` is not part of PipeRef.fetch's options + await client.pipe("top_pages").fetch(shared); + + // `signal` alone is accepted, literal or named. await client.pipe("top_pages").fetch({ signal: AbortSignal.timeout(1000) }); + const signalOnly: PipeRequestOptions = { signal: AbortSignal.timeout(1000) }; + await client.pipe("top_pages").fetch(signalOnly); }); }); diff --git a/clients/ts/src/index.ts b/clients/ts/src/index.ts index 2756cf29..2274b816 100644 --- a/clients/ts/src/index.ts +++ b/clients/ts/src/index.ts @@ -39,6 +39,7 @@ export type { ParamDef, // Pipes Pipe, + PipeRequestOptions, // Policy Policy, PolicyFilter, diff --git a/clients/ts/src/pipes.ts b/clients/ts/src/pipes.ts index 55b621fd..e20c4ae4 100644 --- a/clients/ts/src/pipes.ts +++ b/clients/ts/src/pipes.ts @@ -1,7 +1,7 @@ import { err, ok } from "./errors.js"; import { request } from "./http.js"; import type { StreamController } from "./stream/controller.js"; -import type { HttpContext, Pipe, RequestOptions, Result, StreamOptions } from "./types.js"; +import type { HttpContext, Pipe, PipeRequestOptions, Result, StreamOptions } from "./types.js"; type CreateStreamFn = (table: string, opts?: StreamOptions) => StreamController; @@ -32,7 +32,7 @@ export class PipeRef> implements PromiseLike): Promise> { + async fetch(opts?: PipeRequestOptions): Promise> { const { data, error } = await request(this._ctx, { method: "POST", path: `/v1/pipes/${encodeURIComponent(this._name)}`, diff --git a/clients/ts/src/types.ts b/clients/ts/src/types.ts index 109ad04b..a77b9685 100644 --- a/clients/ts/src/types.ts +++ b/clients/ts/src/types.ts @@ -344,6 +344,23 @@ export interface RequestOptions { limit?: number; } +/** + * Options for a single `PipeRef.fetch()` call. + * + * `limit` is declared `never` rather than simply left out. Omitting it would + * still admit a `RequestOptions` value that carries one — excess-property + * checking only rejects fresh object literals, so a variable would pass and the + * limit would be silently dropped, which is the whole defect this prevents. + * + * A pipe's row cap belongs in its SQL as a `{{limit}}` parameter, supplied via + * `wh.pipe(name, { limit })`. + */ +export interface PipeRequestOptions { + signal?: AbortSignal; + /** Not supported on pipes — pass `limit` as a pipe parameter instead. */ + limit?: never; +} + // --- Stream options --- export interface StreamOptions { diff --git a/docs/src/content/docs/sdk/pipes.md b/docs/src/content/docs/sdk/pipes.md index 4ffb79c2..3a2d5636 100644 --- a/docs/src/content/docs/sdk/pipes.md +++ b/docs/src/content/docs/sdk/pipes.md @@ -21,8 +21,10 @@ const { data } = await wh.pipe('top_pages', { start_date: '2026-01-01', limit: 5 ### `.fetch(opts?)` -Execute and return results. Takes `{ signal }` only — narrower than the -`.fetch(opts?)` on a [query builder](/sdk/queries), which also accepts `limit`. +Execute and return results. Takes `PipeRequestOptions` — `{ signal }` only, +narrower than the `.fetch(opts?)` on a [query builder](/sdk/queries), which also +accepts `limit`. Passing a `limit` is a compile error, including via a shared +`RequestOptions` value, rather than being silently dropped. There is no per-call row cap here: the endpoint binds your `params` as the pipe's parameters, so a limit has to be declared in the pipe's SQL as From f68c2bad4c2ab3de2db805f9cca8f317fe0dc3fb Mon Sep 17 00:00:00 2001 From: Eric Andrechek Date: Wed, 12 Aug 2026 17:31:12 -0400 Subject: [PATCH 18/18] docs(sdk): state the half of the pipes break consumers will actually hit Both reviewers caught the same under-description. The docs and CHANGELOG framed the new compile error as conditional on passing a limit. It is not: limit?: never makes any value *declared* RequestOptions unassignable, limit or no limit, because assignability is decided on the declared type. Verified -- a limit-free `const o: RequestOptions = { signal }` fails with "Type 'number | undefined' is not assignable to type 'undefined'", which reads like an SDK bug when the value plainly has no limit. That is the error consumers will meet, so it now leads with the way out. Verified both halves of the advice rather than asserting them: a PipeRequestOptions value is accepted by the pipe, table AND query-builder .fetch(), so it works as a single shared type; and an un-annotated `const o = { signal }` passes by inference. Also documented a reassuring property I had not checked, from the code reviewer: method parameters compare bivariantly, so PipeRef still satisfies a structural `interface Fetchable { fetch(opts?: RequestOptions) }`. Verified. Only direct argument passing breaks, which shrinks the blast radius considerably and is worth saying, since a reader otherwise has to guess. Separately: RequestOptions had become a term of art in the docs without being named anywhere it is defined -- queries.md documented the query builder's options as an untitled table -- so the pipes page contrasted against a name the linked page never used. queries.md now names the type at its home and points at the pipes variant. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01G2TCZsU46Qeu3efG1cjaXM --- CHANGELOG.md | 2 +- docs/src/content/docs/sdk/pipes.md | 12 ++++++++++-- docs/src/content/docs/sdk/queries.md | 5 ++++- 3 files changed, 15 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f932e049..82199699 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- **BREAKING (SDK): `PipeRef.fetch` no longer accepts a `limit` it silently ignored** (`clients/ts/src/pipes.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/pipes.md`, `docs/src/content/docs/sdk/reference.md`): closes #464, raised by CodeRabbit on #456. It took the same per-call options type as the query builder — which carries `limit` — but forwarded only `signal`, so `wh.pipe('top_pages').fetch({ limit: 10 })` type-checked, ran, and quietly returned whatever the pipe's SQL returned. `QueryBuilder.fetch` and `TableRef.fetch` both honour `limit`, so the inconsistency sat inside one shared type. There is nothing to forward: the endpoint binds the request body as the pipe's *parameters* (`internal/api/pipes.go` → `pipes.BindParams`), and a key the SQL doesn't declare is ignored, so a client-side row cap is not something the pipes surface offers. The parameter is now a dedicated `PipeRequestOptions` (exported) declaring `signal?: AbortSignal` and `limit?: never`, making the dead option a compile error rather than a silent no-op. `never` rather than simply omitting `limit`, because omitting it only rejects fresh object literals — TypeScript's excess-property check doesn't apply to a *variable*, so a shared `const opts: RequestOptions` carrying a limit would still have passed and still been dropped, which is the defect rather than a narrower version of it. Both cases are pinned by `@ts-expect-error` tests. **Migration:** declare a `{{limit}}` parameter in the pipe's SQL and pass it as a pipe parameter — `wh.pipe(name, { limit })` — which is what the docs already showed. Pre-existing rather than introduced by #456, folded in there because that PR renames the type in question. +- **BREAKING (SDK): `PipeRef.fetch` no longer accepts a `limit` it silently ignored** (`clients/ts/src/pipes.ts`, `clients/ts/src/client.test.ts`, `docs/src/content/docs/sdk/pipes.md`, `docs/src/content/docs/sdk/reference.md`): closes #464, raised by CodeRabbit on #456. It took the same per-call options type as the query builder — which carries `limit` — but forwarded only `signal`, so `wh.pipe('top_pages').fetch({ limit: 10 })` type-checked, ran, and quietly returned whatever the pipe's SQL returned. `QueryBuilder.fetch` and `TableRef.fetch` both honour `limit`, so the inconsistency sat inside one shared type. There is nothing to forward: the endpoint binds the request body as the pipe's *parameters* (`internal/api/pipes.go` → `pipes.BindParams`), and a key the SQL doesn't declare is ignored, so a client-side row cap is not something the pipes surface offers. The parameter is now a dedicated `PipeRequestOptions` (exported) declaring `signal?: AbortSignal` and `limit?: never`, making the dead option a compile error rather than a silent no-op. `never` rather than simply omitting `limit`, because omitting it only rejects fresh object literals — TypeScript's excess-property check doesn't apply to a *variable*, so a shared `const opts: RequestOptions` carrying a limit would still have passed and still been dropped, which is the defect rather than a narrower version of it. Both cases are pinned by `@ts-expect-error` tests. **Note the collateral effect**, which is the half most consumers will actually meet: a value *declared* `RequestOptions` no longer assigns to a pipe `.fetch()` at all, even when it carries no limit at runtime, because the declared type permits one and assignability is decided on the type. Type a shared options object as `PipeRequestOptions` — the table and query-builder `.fetch()` accept it too, so it works everywhere — or inline `{ signal }` at the pipe call. Structural wrappers are unaffected: method parameters compare bivariantly, so an `interface Fetchable { fetch(opts?: RequestOptions): … }` is still satisfied by `PipeRef`. **Migration:** declare a `{{limit}}` parameter in the pipe's SQL and pass it as a pipe parameter — `wh.pipe(name, { limit })` — which is what the docs already showed. Pre-existing rather than introduced by #456, folded in there because that PR renames the type in question. - **BREAKING (SDK): `FetchOptions` is renamed `RequestOptions`** (`clients/ts/src/types.ts`, `clients/ts/src/index.ts`, `clients/ts/src/query-builder.ts`, `clients/ts/src/table.ts`, `clients/ts/src/pipes.ts`): the per-call options type accepted by `.fetch()`. The old name collided conceptually with the new `options.fetchOptions` — which, following OpenAI, Anthropic, and the wider ecosystem, means "extra `RequestInit` fields", not "options for our `.fetch()` method". Shipping both would have left `FetchOptions` and `fetchOptions` in the same SDK one capital letter apart, meaning unrelated things. `RequestOptions` is what Anthropic's SDK calls the identical concept. No deprecated alias: the type is unreferenced by anything consuming the pre-1.0 package, and keeping it would preserve exactly the ambiguity the rename removes. Renaming the import is the whole migration for this entry — note the separate `PipeRef.fetch` narrowing above, which is a behavioural break in the same file. The module-private `RequestOptions` in `http.ts` — the internal request descriptor — becomes `RequestSpec` to free the name. diff --git a/docs/src/content/docs/sdk/pipes.md b/docs/src/content/docs/sdk/pipes.md index 3a2d5636..f0e5b8ed 100644 --- a/docs/src/content/docs/sdk/pipes.md +++ b/docs/src/content/docs/sdk/pipes.md @@ -23,8 +23,16 @@ const { data } = await wh.pipe('top_pages', { start_date: '2026-01-01', limit: 5 Execute and return results. Takes `PipeRequestOptions` — `{ signal }` only, narrower than the `.fetch(opts?)` on a [query builder](/sdk/queries), which also -accepts `limit`. Passing a `limit` is a compile error, including via a shared -`RequestOptions` value, rather than being silently dropped. +accepts `limit`. Passing a `limit` is a compile error rather than a silent +no-op. + +`limit` is typed `never` rather than left out, so the rejection also catches a +value passed in a variable — leaving it out would only reject an inline object. +That cuts both ways: a value *declared* as `RequestOptions` is rejected whether +or not it actually carries a limit, since the type permits one. If you share one +options object across calls, type it as `PipeRequestOptions` — the table and +query-builder `.fetch()` accept that too — or inline `{ signal }` at the pipe +call. There is no per-call row cap here: the endpoint binds your `params` as the pipe's parameters, so a limit has to be declared in the pipe's SQL as diff --git a/docs/src/content/docs/sdk/queries.md b/docs/src/content/docs/sdk/queries.md index 0da64b91..58a6506d 100644 --- a/docs/src/content/docs/sdk/queries.md +++ b/docs/src/content/docs/sdk/queries.md @@ -223,13 +223,16 @@ if (hasMore && next) { } ``` -**Options:** +**Options** — `RequestOptions`: | Field | Type | Description | |-------|------|-------------| | `signal` | `AbortSignal` | Cancel the request | | `limit` | `number` | Override builder limit for this fetch | +A pipe's `.fetch()` takes the narrower `PipeRequestOptions` instead — see +[Pipes](/sdk/pipes#fetchopts). + ### `.stream(opts?)` Open a live stream from the builder's table. See [Streaming](/sdk/streaming).