From 4e8f9d87671e067c2970314bbc7174c4b19f9211 Mon Sep 17 00:00:00 2001 From: developerehsan Date: Wed, 21 Oct 2026 17:06:57 +0500 Subject: [PATCH 1/3] =?UTF-8?q?feat(core)!:=20v2=20=E2=80=94=20safer=20def?= =?UTF-8?q?aults,=20file=20transfer,=20pagination=20(#37=E2=80=93#46)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ten issues reported by a production consumer. Three change existing defaults, each because the old default was the unsafe one. BREAKING CHANGES * Response cache now defaults to off (#40). It was on with a 60s cache-first TTL, serving stale reads underneath an app's own query layer after a write. Pass `cache: { enabled: true }` to keep it. * Unsafe HTTP methods are no longer retried (#42). A POST that fails with a 5xx or a dropped connection may already have committed, so a blind retry double-creates. Opt back in with `retryUnsafeMethods`. * Next.js server-context helpers moved to the `/next` subpath (#41). The root entry no longer references `next/headers`, which was breaking Vite `optimizeDeps` in projects without Next installed. Added * `http.idempotency` — one key per logical call, replayed across its retries, so an opted-in write retry is collapsible server-side (#42). * `errorEnvelope` — teach the classifier a nested error shape, so `error.message` is the server's message and not a JSON blob (#39). * `paginate` / `paginateItems` / `collectPages` for cursor pagination, with same-cursor and maxPages guards (#43). * `createIndexedDbStore({ crossTab: true })` — cross-tab invalidation over BroadcastChannel, matching the Redis store (#46). * FormData bodies on the direct client and over the RPC bridge, via `createRpcRouteHandler({ multipart: true })` (#38). * Binary responses through the RPC bridge via a server-declared `binary` allowlist (#37). Client-supplied responseType still cannot force it — sanitizePerCall (S4) is unchanged. * `openapi` is optional on GlobalConfig (#45). Fixed * A configured `Content-Type: application/json` no longer corrupts a FormData body by suppressing the multipart boundary (#38). * Request bodies are now size-capped against the bytes actually read rather than the client-supplied Content-Length, which can lie or be absent under chunked encoding. Pre-existing on the JSON path. * Multipart envelope revival uses defineProperty, so a `__proto__` key stays an own property and is still caught by the handler's FORBIDDEN_KEYS guard instead of silently moving to the prototype. * Removed a stale "stubbed with TODOs" note from createClient's shipped JSDoc that contradicted the README (#44). Co-Authored-By: Claude Opus 5 --- .changeset/v2-consumer-issues.md | 26 + .gitignore | 3 + docs/README.md | 1 + docs/migrating-v2.md | 178 +++++++ packages/core/README.md | 58 +- packages/core/package.json | 9 +- packages/core/src/browser/transports.ts | 60 ++- .../src/cache-stores/cache-stores.test.ts | 2 +- packages/core/src/cache-stores/index.ts | 70 ++- packages/core/src/cache-stores/mode.test.ts | 2 +- .../cache-stores/redis-crossinstance.test.ts | 6 +- packages/core/src/e2e.test.ts | 22 +- .../core/src/environment/serverContext.ts | 21 +- packages/core/src/errors/errorClassifier.ts | 41 +- .../core/src/factory/cacheKeyParts.test.ts | 8 +- .../core/src/factory/cachePreview.test.ts | 1 + packages/core/src/factory/cacheStats.test.ts | 2 +- packages/core/src/factory/createClient.ts | 69 ++- packages/core/src/factory/etag.test.ts | 2 +- .../core/src/factory/mergeModuleConfig.ts | 12 +- .../core/src/factory/negativeCaching.test.ts | 10 +- .../factory/schemaVersionCacheBusting.test.ts | 2 +- .../core/src/factory/tagInvalidation.test.ts | 21 +- .../core/src/http/adapters/axiosAdapter.ts | 26 +- .../core/src/http/adapters/fetchAdapter.ts | 15 + packages/core/src/index.ts | 13 +- packages/core/src/next/index.ts | 14 + packages/core/src/rpc/protocol.ts | 51 ++ packages/core/src/server/createRpcHandler.ts | 99 +++- packages/core/src/server/index.ts | 8 +- packages/core/src/server/routeHandler.ts | 163 +++++- packages/core/src/types/config.types.ts | 102 +++- .../core/src/utilities/memoryPressure.test.ts | 2 +- packages/core/src/utilities/paginate.ts | 120 +++++ packages/core/src/v2-consumer-issues.test.ts | 499 ++++++++++++++++++ packages/core/tsup.config.ts | 1 + 36 files changed, 1640 insertions(+), 99 deletions(-) create mode 100644 .changeset/v2-consumer-issues.md create mode 100644 docs/migrating-v2.md create mode 100644 packages/core/src/next/index.ts create mode 100644 packages/core/src/utilities/paginate.ts create mode 100644 packages/core/src/v2-consumer-issues.test.ts diff --git a/.changeset/v2-consumer-issues.md b/.changeset/v2-consumer-issues.md new file mode 100644 index 0000000..1c00fb3 --- /dev/null +++ b/.changeset/v2-consumer-issues.md @@ -0,0 +1,26 @@ +--- +'@developerehsan/api-client': major +--- + +v2: safer defaults and first-class file transfer, from production consumer reports (#37–#46). + +**Breaking** + +- **Response cache is off by default** (#40). `cache.enabled` was `true` with a 60s `cache-first` TTL, which served stale reads under an app's own query layer after a write. Pass `cache: { enabled: true }` to keep it. +- **Unsafe methods are no longer retried** (#42). Only `GET`/`HEAD`/`OPTIONS`/`TRACE`/`PUT`/`DELETE` retry by default — a failed `POST` may have already committed server-side. Opt back in with `retry: { retryUnsafeMethods: true }`. +- **Next.js server-context helpers moved to `@developerehsan/api-client/next`** (#41). The root entry no longer references `next/headers`, which was breaking Vite `optimizeDeps` in projects without Next.js installed. + +**Added** + +- `http.idempotency` — one auto-generated `Idempotency-Key` per logical call, replayed across its retries, making a retried write collapsible server-side (#42). +- `errorEnvelope` — teach the classifier a nested error shape so `error.message` is the server's message rather than a stringified payload (#39). +- `paginate` / `paginateItems` / `collectPages` — drive a cursor to exhaustion as an `AsyncIterable`, with same-cursor and `maxPages` guards (#43). +- `createIndexedDbStore({ crossTab: true })` — cross-tab cache invalidation over `BroadcastChannel`, matching the Redis store's `crossInstance` (#46). +- `FormData` bodies over the direct client and the RPC bridge, via `createRpcRouteHandler({ multipart: true })` (#38). +- Binary/file responses through the RPC bridge, via a server-declared `binary` allowlist on `createRpcHandler` (#37). Client-supplied `responseType` still cannot force it. +- `openapi` is now optional on `GlobalConfig` (#45). + +**Fixed** + +- A configured `Content-Type: application/json` no longer corrupts a `FormData` body by suppressing the multipart boundary (#38). +- Removed a stale "stubbed with TODOs" note from `createClient`'s shipped JSDoc that contradicted the README (#44). diff --git a/.gitignore b/.gitignore index 96ff800..504ba81 100644 --- a/.gitignore +++ b/.gitignore @@ -6,3 +6,6 @@ coverage/ .DS_Store *.tsbuildinfo .pnpm-store/ + +# Serena MCP server workspace +.serena/ diff --git a/docs/README.md b/docs/README.md index 2148847..ec19dc2 100644 --- a/docs/README.md +++ b/docs/README.md @@ -30,6 +30,7 @@ so they run with **no backend setup**. ## Guides by topic ### Foundations +- [Migrating to v2](./migrating-v2.md) — three changed defaults, and everything new - [Getting started](./getting-started.md) — your first client in 5 minutes - [Mental model](./mental-model.md) — the three concepts and the request pipeline - [Modules & methods](./modules-and-methods.md) — `defineModule`, `ctx.request`, composed calls diff --git a/docs/migrating-v2.md b/docs/migrating-v2.md new file mode 100644 index 0000000..fd707aa --- /dev/null +++ b/docs/migrating-v2.md @@ -0,0 +1,178 @@ +# Migrating to v2 + +v2 is a small release with a sharp edge: three defaults changed, and all three +changed because the old default was the unsafe one. Everything else is additive. + +Every change below came from [issues #37–#46](https://github.com/developerEhsan/api-client/issues), +reported by a production consumer. + +## Breaking changes + +### 1. The response cache is off by default (#40) + +**Before:** `cache.enabled` defaulted to `true` with a 60s `cache-first` TTL. +**Now:** it defaults to `false`. + +A write would commit, your query layer would refetch, and the client's own +cache would still serve the 60s-old response underneath it — a stale read with +nothing in the UI to explain it. Apps pairing this client with TanStack Query +or SWR were silently double-cached. + +```diff + createClient({ + baseURL, ++ cache: { enabled: true }, // only if you actually want the L1 cache + cache: { ttl: 60_000, strategy: 'cache-first' }, + }) +``` + +Nothing to do if you were already passing `cache: { enabled: false }`, or if +you never wanted the cache. + +### 2. Unsafe HTTP methods are no longer retried (#42) + +**Before:** all 5xx/429/network failures retried, on any method. +**Now:** only `GET`, `HEAD`, `OPTIONS`, `TRACE`, `PUT` and `DELETE` retry. + +A `POST` that fails with a 5xx or a dropped connection may have already +committed server-side. Retrying it creates the resource twice. + +To keep the old behavior on a write, opt in — and send an idempotency key so +the duplicate is collapsed rather than committed: + +```ts +createClient({ + baseURL, + http: { + retry: { attempts: 3, retryUnsafeMethods: true }, + idempotency: { enabled: true }, // Idempotency-Key on POST + PATCH + }, +}) +``` + +One key is generated per logical call and replayed across every retry of that +call. Your API has to honor the header for it to mean anything — the client +only guarantees the key is stable. + +A custom `retryOn` predicate still overrides this entirely; it is you stating +the policy yourself. + +### 3. Next.js helpers moved to `/next` (#41) + +`readServerHeader`, `readServerCookie`, `serverTenantResolver` and +`serverTokenFromCookie` are no longer exported from the root entry. + +```diff +- import { serverTokenFromCookie } from '@developerehsan/api-client' ++ import { serverTokenFromCookie } from '@developerehsan/api-client/next' +``` + +They reach for `next/headers`, and having them on the root entry meant every +consumer's bundler tried to resolve `next` — which broke Vite dev servers in +projects with no Next.js installed. The root entry now contains no reference to +`next` at all, so the `optimizeDeps.exclude` workaround is no longer needed. + +## New + +### `openapi` is optional (#45) + +```diff +- createClient({ baseURL, openapi: {} }) ++ createClient({ baseURL }) +``` + +Every field inside it was already optional; requiring the block bought no type +safety. + +### Custom error envelopes (#39) + +The classifier only recognized a flat `{ code, message }`. A nested envelope +fell through to `rawBody`, so `error.message` came out as a stringified JSON +blob — in practice, raw JSON rendered into user-facing toasts. + +```ts +createClient({ + baseURL, + // API returns { error: { code, message, details } } + errorEnvelope: (body) => (body as { error?: ServerErrorBody }).error ?? null, +}) +``` + +Return `null` to fall back to the built-in detection. A parser that throws is +caught — it can never mask the original failure. + +### Cursor pagination (#43) + +```ts +import { paginate, paginateItems, collectPages } from '@developerehsan/api-client' + +for await (const flag of paginateItems( + (cursor) => api.flags.list({ query: { cursor, limit: 100 } }), + { nextCursor: (p) => p.nextCursor, items: (p) => p.data }, +)) { /* ... */ } +``` + +`paginate` yields pages, `paginateItems` yields items, `collectPages` drains +into an array. A server that echoes the same cursor back terminates the loop +rather than spinning, and `maxPages` (default 10,000) backstops the rest. + +### Cross-tab cache invalidation for IndexedDB (#46) + +The browser-side L2 store now has the same cross-instance coherency the Redis +store got, over `BroadcastChannel`: + +```ts +createIndexedDbStore({ crossTab: true }) +``` + +A mutation in one tab now invalidates the cached GET in every other open tab. +No-ops where `BroadcastChannel` is unavailable. + +### `FormData` bodies (#38) + +Multipart bodies now work on both paths. + +**Direct client** — a `FormData` body is passed through untouched, and a +globally-configured `Content-Type: application/json` is dropped for it so the +runtime can generate the multipart boundary: + +```ts +await api.files.upload(formData) +``` + +**RPC bridge** — `httpTransport` detects `File`/`Blob` arguments and switches +that call to a multipart body: the JSON envelope travels in an `__rpc` part +with `{ __rpcFile }` placeholders, the bytes as their own parts. Opt the route +in: + +```ts +createRpcRouteHandler(handler, { + multipart: true, + maxUploadBytes: 10 * 1024 * 1024, // default 10 MB +}) +``` + +Off by default: multipart widens the accepted content types, and such a body is +not covered by the JSON-content-type half of the CSRF defence — it leans +entirely on the Origin check. Keep `csrf: true` when enabling it. + +### Binary responses through the RPC bridge (#37) + +A module method can now return a file through the bridge. **The server +declares which methods are binary** — a client-supplied `responseType` is still +stripped by `sanitizePerCall` (S4), so the browser can never turn a JSON method +into a byte stream: + +```ts +createRpcHandler(api, { + expose: { exports: ['flagsCsv', 'logsCsv'] }, + binary: { exports: ['flagsCsv', 'logsCsv'] }, // must be in BOTH + binaryFilename: (call) => `${call.method}-${Date.now()}.csv`, +}) +``` + +The route streams the result back with its own `Content-Type` and +`Content-Disposition` instead of wrapping it in the JSON envelope, and +`httpTransport` surfaces it to the caller as a `Blob`. Filenames are sanitized +against header injection. A binary method inside a *batch* is refused — a batch +is one JSON array, with nowhere for a byte stream to go. diff --git a/packages/core/README.md b/packages/core/README.md index 2def3ce..4f3e3ec 100644 --- a/packages/core/README.md +++ b/packages/core/README.md @@ -53,7 +53,8 @@ export const api = createClient({ openapi: { mode: 'runtime' }, auth: { strategy: 'bearer', getToken: () => localStorage.getItem('access_token') }, http: { timeout: 10_000, retry: { attempts: 3 } }, - cache: { strategy: 'stale-while-revalidate', ttl: 60_000 }, + // Caching is OFF unless you ask for it — see "Defaults worth knowing" below. + cache: { enabled: true, strategy: 'stale-while-revalidate', ttl: 60_000 }, modules: { users: defineModule({ methods: { @@ -80,6 +81,52 @@ const made = await api.users.create({ name: 'Ada', email: 'ada@x.com' }) That is a fully working client with auth, caching, retries, timeouts, and dedup already active. +## Defaults worth knowing + +The defaults are deliberately conservative — the client should never silently +change what your app observes. + +| Behavior | Default | Why | +| --- | --- | --- | +| **Response cache** | **off** (`cache.enabled: false`) | Most apps already run TanStack Query / SWR. A second cache underneath that one serves a stale read after a write with nothing in the UI to explain it. Opt in with `cache: { enabled: true }` (then `ttl` 60s, `cache-first`). | +| **Retries** | 3 attempts, **safe methods only** | `GET`/`HEAD`/`OPTIONS`/`TRACE`/`PUT`/`DELETE`. A `POST` is never retried automatically: a 5xx or dropped connection can arrive *after* the server committed the write, so a retry would create the resource twice. | +| **Idempotency keys** | off | Turn on with `http: { idempotency: { enabled: true } }` to make retrying a write safe (see below). | +| **Dedup** | on, `GET` only | Identical in-flight requests share one network call. | +| **Timeout** | 10s | Per request, enforced on every adapter. | +| **OpenAPI** | none | `openapi` is optional; omit it entirely unless you use codegen, runtime validation, or drift detection. | + +### Retrying writes safely + +To retry a `POST`/`PATCH`, opt the method in *and* send an idempotency key so +the server can collapse the duplicate: + +```ts +createClient({ + baseURL, + http: { + retry: { attempts: 3, retryUnsafeMethods: true }, + idempotency: { enabled: true }, // Idempotency-Key, POST + PATCH + }, +}) +``` + +One key is generated per logical call and replayed on every retry of that call, +which is what makes the retry safe. Your API has to honor the header. + +### Custom error shapes + +The classifier recognizes a flat `{ code, message }` body. If your API nests its +errors, teach it the shape — otherwise `error.message` is the whole payload as a +JSON string: + +```ts +createClient({ + baseURL, + // API returns { error: { code, message, details } } + errorEnvelope: (body) => (body as { error?: ServerErrorBody }).error ?? null, +}) +``` + ## Features | Area | What you get | @@ -87,9 +134,9 @@ already active. | **Typed proxy** | `api.[module].[method](args)` with full input/output inference | | **Adapters** | Axios (default) or native `fetch`; auto-fallback to `fetch` on edge | | **Auth** | Bearer, Cookie, API key, OAuth2 (with automatic 401 → refresh → retry) | -| **Caching** | In-memory LRU, TTL, `cache-first` / `network-first` / `stale-while-revalidate`, glob invalidation | +| **Caching** | Opt-in (`cache.enabled`); in-memory LRU, TTL, `cache-first` / `network-first` / `stale-while-revalidate`, glob invalidation | | **Dedup** | Identical in-flight requests share one network call | -| **Retries** | Exponential/linear/fixed backoff, full-jitter, honors `Retry-After` | +| **Retries** | Exponential/linear/fixed backoff, full-jitter, honors `Retry-After`; safe methods only by default, with optional idempotency keys | | **Timeouts** | Per-request, enforced on every adapter (incl. `fetch`) | | **Cancellation** | `AbortSignal` support + debounce-cancel | | **Concurrency** | Global request queue with a configurable limit | @@ -103,7 +150,9 @@ already active. | **RPC rate limiter** | Built-in `createRateLimiter` for the handler `onRequest` (per-IP / per-session, pluggable store) | | **Streaming** | `ctx.stream()` → `AsyncIterable` for NDJSON / SSE / raw byte streams | | **Modules beyond HTTP** | `ctx.run()` runs any async logic with opt-in queue/dedup/retry/timeout; `ctx.emit` / `ctx.logger` / `ctx.config` | -| **Cache persistence** | Pluggable L2 stores (IndexedDB / Redis) behind the in-memory LRU | +| **Cache persistence** | Pluggable L2 stores (IndexedDB / Redis) behind the in-memory LRU; cross-instance invalidation via Redis pub/sub or `BroadcastChannel` across tabs | +| **Pagination** | `paginate` / `paginateItems` / `collectPages` drive a cursor to exhaustion as an `AsyncIterable` | +| **File transfer** | `FormData` bodies and binary (`Blob`/stream) responses, over the direct client *and* the RPC bridge | | **Hooks** | `onRequest`/`onResponse`/`onError`/`onRetry`/`onSuccess`/`onSettled` composed across global → module → per-call | | **Testing** | `createMockClient` + `MockAdapter` | @@ -116,6 +165,7 @@ already active. | `@developerehsan/api-client/testing` | Mock client & adapter | | `@developerehsan/api-client/server` | SSR RPC bridge — server half (`createRpcHandler`, batching, `createRateLimiter`, Next / TanStack Start / Remix glue) | | `@developerehsan/api-client/browser` | SSR RPC bridge — dependency-free browser client (`createRpcClient`, transports, batching) | +| `@developerehsan/api-client/next` | Next.js server-context helpers (`serverTokenFromCookie`, `serverTenantResolver`) — the only entry that touches `next/headers` | | `@developerehsan/api-client/cache-stores` | Pluggable persistent cache stores (memory / IndexedDB / Redis) | ## Related packages diff --git a/packages/core/package.json b/packages/core/package.json index e496e93..10e79bf 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -50,7 +50,9 @@ "access": "public", "provenance": true }, - "files": ["dist"], + "files": [ + "dist" + ], "exports": { ".": { "types": "./dist/index.d.ts", @@ -77,6 +79,11 @@ "import": "./dist/server.js", "require": "./dist/server.cjs" }, + "./next": { + "types": "./dist/next.d.ts", + "import": "./dist/next.js", + "require": "./dist/next.cjs" + }, "./browser": { "types": "./dist/browser.d.ts", "browser": "./dist/browser.js", diff --git a/packages/core/src/browser/transports.ts b/packages/core/src/browser/transports.ts index c0f0109..5e5d5fb 100644 --- a/packages/core/src/browser/transports.ts +++ b/packages/core/src/browser/transports.ts @@ -9,6 +9,7 @@ */ import { + RPC_MULTIPART_ENVELOPE, type RpcBatchRequest, type RpcCall, type RpcResponse, @@ -80,6 +81,44 @@ export interface HttpTransportOptions { * ) * await api.pet.getPetById({ petId: 1 }) */ +/** True for a value that must travel as its own multipart part, not as JSON. */ +function isBinaryArg(value: unknown): value is Blob { + return typeof Blob !== 'undefined' && value instanceof Blob; +} + +/** + * Walk a call's args, pulling every `File`/`Blob` out into its own multipart + * part and leaving a `{ __rpcFile }` placeholder behind. Returns `null` when + * there was nothing binary to extract, so the plain JSON path is unchanged. + */ +function toMultipart(call: RpcCall): FormData | null { + if (typeof FormData === 'undefined') return null; + const form = new FormData(); + let found = 0; + + const extract = (value: unknown): unknown => { + if (isBinaryArg(value)) { + const field = `__file${found++}`; + form.append(field, value); + return { __rpcFile: field }; + } + if (Array.isArray(value)) return value.map(extract); + // Only plain objects are walked: a Date/Map/class instance would not + // survive JSON round-tripping as itself anyway. + if (typeof value === 'object' && value !== null && value.constructor === Object) { + const out: Record = {}; + for (const [k, v] of Object.entries(value as Record)) out[k] = extract(v); + return out; + } + return value; + }; + + const envelope = { ...call, args: call.args.map(extract) }; + if (found === 0) return null; + form.append(RPC_MULTIPART_ENVELOPE, JSON.stringify(envelope)); + return form; +} + export function httpTransport(options: HttpTransportOptions): Transport { const { endpoint, headers } = options; const doFetch = options.fetch ?? globalThis.fetch; @@ -95,16 +134,29 @@ export function httpTransport(options: HttpTransportOptions): Transport { }, }); - const post = async (payload: unknown): Promise<{ status: number; body: unknown }> => { + const post = async ( + payload: unknown, + form?: FormData | null, + ): Promise<{ status: number; body: unknown }> => { if (typeof doFetch !== 'function') { throw new Error('httpTransport: no fetch implementation available.'); } const res = await doFetch(endpoint, { method: 'POST', - headers: { 'content-type': 'application/json', ...headers }, - body: JSON.stringify(payload), + // A multipart body sets its own Content-Type (it needs the generated + // boundary), so the JSON header is omitted for that path only. + headers: form ? { ...headers } : { 'content-type': 'application/json', ...headers }, + body: form ?? JSON.stringify(payload), credentials: 'same-origin', }); + + // A server-declared binary method streams bytes back rather than an + // envelope (#37) — hand the caller the Blob inside a normal ok envelope. + const resType = res.headers.get('content-type') ?? ''; + if (res.ok && !resType.includes('application/json')) { + return { status: res.status, body: { ok: true, data: await res.blob() } }; + } + let body: unknown; try { body = await res.json(); @@ -115,7 +167,7 @@ export function httpTransport(options: HttpTransportOptions): Transport { }; const transport: Transport = async (call: RpcCall): Promise => { - const { status, body } = await post(call); + const { status, body } = await post(call, toMultipart(call)); // Transport-level failure (CSRF reject, 4xx/5xx without an envelope): surface // a generic error envelope rather than leaking the raw response. return isRpcResponse(body) ? body : transportError(status); diff --git a/packages/core/src/cache-stores/cache-stores.test.ts b/packages/core/src/cache-stores/cache-stores.test.ts index f925957..6d8d86e 100644 --- a/packages/core/src/cache-stores/cache-stores.test.ts +++ b/packages/core/src/cache-stores/cache-stores.test.ts @@ -114,7 +114,7 @@ describe('persistentStore through createClient', () => { baseURL: 'http://mock.test', openapi: { mode: 'runtime' }, http: { adapter: mock, retry: { attempts: 1, baseDelay: 0, maxDelay: 0, jitter: false } }, - cache: { persistentStore: l2 }, + cache: { enabled: true, persistentStore: l2 }, modules: { auto: false as const, things: { diff --git a/packages/core/src/cache-stores/index.ts b/packages/core/src/cache-stores/index.ts index abbc765..9e59085 100644 --- a/packages/core/src/cache-stores/index.ts +++ b/packages/core/src/cache-stores/index.ts @@ -157,6 +157,27 @@ export interface IndexedDbStoreOptions { * @default optional, unset means entries are stored as plain JSON (today's behavior) */ encrypt?: CacheCipher; + /** + * Broadcast local invalidations to other tabs of the same origin over + * `BroadcastChannel`, and apply theirs locally — the browser-side + * counterpart to the Redis store's `crossInstance` (#46). + * + * Without it a mutation in one tab leaves every other open tab serving the + * stale cached GET until its TTL expires. + * + * No-ops where `BroadcastChannel` is unavailable. + * + * @default false + * @example `createIndexedDbStore({ crossTab: true })` + */ + crossTab?: boolean; + /** + * `BroadcastChannel` name used when `crossTab` is on. Change it to isolate + * two independent caches sharing one origin. + * @default 'apicache:invalidate' + * @example `createIndexedDbStore({ crossTab: true, channel: 'admin-cache' })` + */ + channel?: string; } /** Pluggable cipher for {@link IndexedDbStoreOptions.encrypt}. See its security note. */ @@ -274,7 +295,7 @@ export function createIndexedDbStore(options: IndexedDbStoreOptions = {}): Persi }), ); - return { + const store: PersistentCacheStore = { get: (key) => guard( tx('readonly', (s) => s.get(key)).then(async (v) => { @@ -318,6 +339,53 @@ export function createIndexedDbStore(options: IndexedDbStoreOptions = {}): Persi { op: 'clear' }, ), }; + + if (!options.crossTab) return store; + + interface BroadcastChannelLike { + postMessage(data: unknown): void; + onmessage: ((ev: { data: unknown }) => void) | null; + } + const BC = (globalThis as { BroadcastChannel?: new (name: string) => BroadcastChannelLike }) + .BroadcastChannel; + // Same contract as every other capability here: an unavailable backend + // degrades to the plain store rather than throwing at construction. + if (!BC) return store; + + const channelName = options.channel ?? 'apicache:invalidate'; + const origin = randomOrigin(); + const handlers = new Set<(msg: InvalidationMessage) => void>(); + let channel: BroadcastChannelLike; + try { + channel = new BC(channelName); + } catch { + return store; + } + + channel.onmessage = (ev) => { + // Payload comes from a sibling tab, so it is parsed through the same + // validator as the Redis path rather than trusted as-is. + const msg = parseInvalidationMessage( + typeof ev.data === 'string' ? ev.data : JSON.stringify(ev.data), + ); + if (!msg || msg.origin === origin) return; + for (const handler of handlers) handler(msg); + }; + + return { + ...store, + onRemoteInvalidate(handler) { + handlers.add(handler); + }, + publishInvalidate(msg: OutgoingInvalidationMessage) { + try { + channel.postMessage(JSON.stringify({ ...msg, origin })); + } catch { + // Fire-and-forget: a closed channel must never fail a request. + } + return Promise.resolve(); + }, + }; } /** Options for {@link createFileSystemStore}. */ diff --git a/packages/core/src/cache-stores/mode.test.ts b/packages/core/src/cache-stores/mode.test.ts index 8656695..ab2cf84 100644 --- a/packages/core/src/cache-stores/mode.test.ts +++ b/packages/core/src/cache-stores/mode.test.ts @@ -17,7 +17,7 @@ function makeThingsClient( baseURL: 'http://mock.test', openapi: { mode: 'runtime' }, http: { adapter: mock, retry: { attempts: 1, baseDelay: 0, maxDelay: 0, jitter: false } }, - cache, + cache: { enabled: true, ...cache }, modules: { auto: false as const, things: { diff --git a/packages/core/src/cache-stores/redis-crossinstance.test.ts b/packages/core/src/cache-stores/redis-crossinstance.test.ts index f7d3906..cd72868 100644 --- a/packages/core/src/cache-stores/redis-crossinstance.test.ts +++ b/packages/core/src/cache-stores/redis-crossinstance.test.ts @@ -56,7 +56,7 @@ function makeUsersClient(store: ReturnType) { baseURL: 'http://mock.test', openapi: { mode: 'runtime' }, http: { adapter: mock, retry: { attempts: 1, baseDelay: 0, maxDelay: 0, jitter: false } }, - cache: { persistentStore: store }, + cache: { enabled: true, persistentStore: store }, modules: { auto: false as const, users: { @@ -65,7 +65,7 @@ function makeUsersClient(store: ReturnType) { ( await ctx.request( { method: 'GET', path: '/users/{id}', pathParams: { id } }, - { cache: { tags: [`user:${id}`] } }, + { cache: { enabled: true, tags: [`user:${id}`] } }, ) ).data, }, @@ -73,7 +73,7 @@ function makeUsersClient(store: ReturnType) { }, }) as unknown as { users: { getUser: (id: string) => Promise }; - cache: { invalidateTags(tags: string[]): number }; + cache: { enabled: true; invalidateTags(tags: string[]): number }; }; return { api, mock }; } diff --git a/packages/core/src/e2e.test.ts b/packages/core/src/e2e.test.ts index decf33a..1669f6d 100644 --- a/packages/core/src/e2e.test.ts +++ b/packages/core/src/e2e.test.ts @@ -196,7 +196,7 @@ describe('E2E · auth', () => { describe('E2E · caching', () => { it('cache-first serves the second identical GET from cache', async () => { - const { api, mock } = make({ cache: { strategy: 'cache-first', ttl: 60_000 } }); + const { api, mock } = make({ cache: { enabled: true, strategy: 'cache-first', ttl: 60_000 } }); let hits = 0; mock.on('GET', '/things/1', () => { hits++; @@ -208,7 +208,9 @@ describe('E2E · caching', () => { }); it('network-first falls back to cache when the network fails', async () => { - const { api, raw, mock } = make({ cache: { strategy: 'network-first', ttl: 60_000 } }); + const { api, raw, mock } = make({ + cache: { enabled: true, strategy: 'network-first', ttl: 60_000 }, + }); let ok = true; mock.on('GET', '/things/1', () => (ok ? { data: { id: '1' } } : { status: 500, data: {} })); await api.things.get('1'); // primes the cache @@ -216,12 +218,16 @@ describe('E2E · caching', () => { const fallback = await api.things.get('1'); // network 500 -> cached value expect(fallback).toEqual({ id: '1' }); // Glob invalidation drops the entry. - (raw as unknown as { cache: { invalidate: (p: string) => void } }).cache.invalidate('*'); + ( + raw as unknown as { cache: { enabled: true; invalidate: (p: string) => void } } + ).cache.invalidate('*'); await expect(api.things.get('1')).rejects.toBeInstanceOf(ApiError); }); it('stale-while-revalidate serves stale immediately then refreshes', async () => { - const { api, mock } = make({ cache: { strategy: 'stale-while-revalidate', ttl: 0 } }); + const { api, mock } = make({ + cache: { enabled: true, strategy: 'stale-while-revalidate', ttl: 0 }, + }); let version = 1; mock.on('GET', '/things/1', () => ({ data: { v: version } })); const first = await api.things.get('1'); @@ -237,14 +243,14 @@ describe('E2E · caching', () => { }); it('per-call cache bust forces a fresh fetch', async () => { - const { api, mock } = make({ cache: { strategy: 'cache-first', ttl: 60_000 } }); + const { api, mock } = make({ cache: { enabled: true, strategy: 'cache-first', ttl: 60_000 } }); let hits = 0; mock.on('GET', '/things/1', () => { hits++; return { data: { id: '1' } }; }); await api.things.get('1'); - await api.things.get('1', { cache: { bust: true } }); + await api.things.get('1', { cache: { enabled: true, bust: true } }); expect(hits).toBe(2); }); @@ -252,7 +258,7 @@ describe('E2E · caching', () => { const l2 = createMemoryPersistentStore(); const setSpy = vi.spyOn(l2, 'set'); const { api, mock } = make({ - cache: { strategy: 'cache-first', ttl: 60_000, persistentStore: l2 }, + cache: { enabled: true, strategy: 'cache-first', ttl: 60_000, persistentStore: l2 }, }); mock.on('GET', '/things/1', { data: { id: '1' } }); await api.things.get('1'); @@ -322,7 +328,7 @@ describe('E2E · tenancy', () => { let current = 'acme'; const { api, mock } = make({ tenancy: { headerName: 'X-Tenant-ID', getTenantId: () => current }, - cache: { strategy: 'cache-first', ttl: 60_000 }, + cache: { enabled: true, strategy: 'cache-first', ttl: 60_000 }, }); const seen: string[] = []; mock.on('GET', '/things/1', (r) => { diff --git a/packages/core/src/environment/serverContext.ts b/packages/core/src/environment/serverContext.ts index 6c471c3..1932aa9 100644 --- a/packages/core/src/environment/serverContext.ts +++ b/packages/core/src/environment/serverContext.ts @@ -6,6 +6,9 @@ * `next/headers` is imported dynamically and guarded so this module is safe to * include in any bundle: outside a Next.js server request every reader resolves * to `undefined`. + * + * These helpers are published from the `/next` subpath, not the root entry, so + * a non-Next.js app never pulls this module into its graph (issue #41). */ interface NextHeaders { @@ -22,14 +25,24 @@ interface NextHeadersModule { cookies: () => NextCookies | Promise; } +// Assembled at runtime from fragments so no bundler — including Vite's +// `optimizeDeps` esbuild scan, which does not honor `@vite-ignore` inside a +// pre-bundled dependency — can constant-fold this back into a resolvable +// specifier and try to resolve `next` in a project that does not have it. +const NEXT_HEADERS_SPECIFIER = ['next', 'headers'].join('/'); + +let nextHeadersModule: NextHeadersModule | null | undefined; + async function loadNextHeaders(): Promise { + if (nextHeadersModule !== undefined) return nextHeadersModule; try { - // Non-literal specifier keeps bundlers from hard-resolving an optional dep. - const specifier = 'next/headers'; - return (await import(/* @vite-ignore */ specifier)) as unknown as NextHeadersModule; + nextHeadersModule = (await import( + /* @vite-ignore */ /* webpackIgnore: true */ NEXT_HEADERS_SPECIFIER + )) as unknown as NextHeadersModule; } catch { - return null; + nextHeadersModule = null; } + return nextHeadersModule; } /** Read a request header on the server, or `undefined` outside a Next.js request. */ diff --git a/packages/core/src/errors/errorClassifier.ts b/packages/core/src/errors/errorClassifier.ts index 504e8ec..c16258a 100644 --- a/packages/core/src/errors/errorClassifier.ts +++ b/packages/core/src/errors/errorClassifier.ts @@ -34,6 +34,25 @@ export interface TimeoutFailure { /** Discriminated description of a request failure, consumed by {@link classifyError}. */ export type ClassifierInput = HttpFailure | NetworkFailure | TimeoutFailure; +/** + * Teaches the classifier a non-default error shape. Return the flattened + * `{ code, message, details }` for a body you recognize, or `null` to fall + * back to the built-in flat-shape detection. + * + * Configure it once via `createClient({ errorEnvelope })` and every error that + * flows through the pipeline is parsed with it (issue #39). + * + * @example + * ```ts + * // API returns { error: { code, message, details } } + * const errorEnvelope: ErrorEnvelopeParser = (body) => + * typeof body === 'object' && body !== null && 'error' in body + * ? ((body as { error: ServerErrorBody }).error ?? null) + * : null + * ``` + */ +export type ErrorEnvelopeParser = (body: unknown) => ServerErrorBody | null | undefined; + /** Result of splitting a response body into a structured error vs. raw text. */ export interface ExtractedServerError { serverError: ServerErrorBody | null; @@ -55,9 +74,25 @@ export interface ExtractedServerError { * // -> { serverError: null, rawBody: '502' } * ``` */ -export function extractServerError(data: unknown): ExtractedServerError { +export function extractServerError( + data: unknown, + envelope?: ErrorEnvelopeParser, +): ExtractedServerError { if (data === undefined || data === null) return { serverError: null }; + // A consumer-supplied envelope parser wins over the built-in flat shape, so + // a nested `{ error: { code, message } }` body resolves to a real message + // instead of being stringified into `rawBody` (#39). It is user code on an + // error path, so a throw must never mask the original failure. + if (envelope) { + try { + const unwrapped = envelope(data); + if (isServerErrorBody(unwrapped)) return { serverError: unwrapped }; + } catch { + // Fall through to the built-in detection below. + } + } + if (isServerErrorBody(data)) { return { serverError: data }; } @@ -110,7 +145,7 @@ function stringifyBody(data: unknown): string | undefined { * classifyError({ kind: 'timeout', timeoutMs: 2000 }) // -> TimeoutError * ``` */ -export function classifyError(input: ClassifierInput): ApiError { +export function classifyError(input: ClassifierInput, envelope?: ErrorEnvelopeParser): ApiError { switch (input.kind) { case 'network': return new NetworkError({ @@ -130,7 +165,7 @@ export function classifyError(input: ClassifierInput): ApiError { }); case 'http': { - const { serverError, rawBody } = extractServerError(input.data); + const { serverError, rawBody } = extractServerError(input.data, envelope); const code = serverError?.code; // A structured message is used verbatim; an unstructured body (e.g. an // HTML error page) is only used as a short, truncated hint so the message diff --git a/packages/core/src/factory/cacheKeyParts.test.ts b/packages/core/src/factory/cacheKeyParts.test.ts index 2650854..e36ebe1 100644 --- a/packages/core/src/factory/cacheKeyParts.test.ts +++ b/packages/core/src/factory/cacheKeyParts.test.ts @@ -31,7 +31,7 @@ describe('cacheKeyParts (spec 10)', () => { ( await ctx.request( { method: 'GET', path: '/dashboard/summary' }, - { cache: { cacheKeyParts: { workspaceId } } }, + { cache: { enabled: true, cacheKeyParts: { workspaceId } } }, ) ).data, }, @@ -111,7 +111,7 @@ describe('cacheKeyParts (spec 10)', () => { baseURL: 'http://mock.test', openapi: { mode: 'runtime' }, http: { adapter: mock, retry: { attempts: 1, baseDelay: 0, maxDelay: 0, jitter: false } }, - cache: { keyResolver: (req) => `custom:${req.method}:${req.url}` }, + cache: { enabled: true, keyResolver: (req) => `custom:${req.method}:${req.url}` }, modules: { auto: false as const, dashboard: { @@ -136,7 +136,7 @@ describe('cacheKeyParts (spec 10)', () => { baseURL: 'http://mock.test', openapi: { mode: 'runtime' }, http: { adapter: mock, retry: { attempts: 1, baseDelay: 0, maxDelay: 0, jitter: false } }, - cache: { keyResolver: (req) => `custom:${req.method}:${req.url}` }, + cache: { enabled: true, keyResolver: (req) => `custom:${req.method}:${req.url}` }, modules: { auto: false as const, dashboard: { @@ -145,7 +145,7 @@ describe('cacheKeyParts (spec 10)', () => { ( await ctx.request( { method: 'GET', path: '/dashboard/summary' }, - { cache: { cacheKeyParts: { workspaceId } } }, + { cache: { enabled: true, cacheKeyParts: { workspaceId } } }, ) ).data, }, diff --git a/packages/core/src/factory/cachePreview.test.ts b/packages/core/src/factory/cachePreview.test.ts index e2183da..7bb98de 100644 --- a/packages/core/src/factory/cachePreview.test.ts +++ b/packages/core/src/factory/cachePreview.test.ts @@ -10,6 +10,7 @@ import type { ModuleContext } from '../types/module.types'; describe('client.cache.preview()', () => { it('returns the same keys invalidate(pattern) would remove, without removing them', async () => { const { api, mock } = createMockClient({ + cache: { enabled: true }, modules: { auto: false as const, things: { diff --git a/packages/core/src/factory/cacheStats.test.ts b/packages/core/src/factory/cacheStats.test.ts index 3b521e9..b9c9c16 100644 --- a/packages/core/src/factory/cacheStats.test.ts +++ b/packages/core/src/factory/cacheStats.test.ts @@ -12,7 +12,7 @@ function makeThingsClient(cache: { ttl?: number; }) { const { api, mock } = createMockClient({ - cache, + cache: { enabled: true, ...cache }, modules: { auto: false as const, things: { diff --git a/packages/core/src/factory/createClient.ts b/packages/core/src/factory/createClient.ts index 60669e3..1396eaf 100644 --- a/packages/core/src/factory/createClient.ts +++ b/packages/core/src/factory/createClient.ts @@ -3,8 +3,8 @@ * an HTTP adapter for the detected runtime, builds the request pipeline, and * returns a `Proxy`-backed client exposing modules plus utility members. * - * Retry / cache / dedup / schema-loading are Phase 3+/Phase 6 concerns and are - * stubbed with TODOs here. + * The pipeline every call flows through is: + * queue -> dedup -> cache -> auth -> dispatch(retry) -> validate -> cache write. */ import type { AdapterName } from '../environment/edgeSafe'; @@ -46,7 +46,7 @@ import { NetworkError } from '../errors/NetworkError'; import { OperationError } from '../errors/OperationError'; import { SchemaError } from '../errors/SchemaError'; import { TimeoutError } from '../errors/TimeoutError'; -import { classifyError } from '../errors/errorClassifier'; +import { type ClassifierInput, classifyError } from '../errors/errorClassifier'; import { createAxiosAdapter } from '../http/adapters/axiosAdapter'; import { createFetchAdapter } from '../http/adapters/fetchAdapter'; import { @@ -214,13 +214,27 @@ function isAdapterLike(value: unknown): value is HttpAdapterLike { ); } +/** Methods whose semantics make a repeat safe, per RFC 9110 §9.2.2. */ +const IDEMPOTENT_METHODS = new Set(['GET', 'HEAD', 'OPTIONS', 'TRACE', 'PUT', 'DELETE']); + +function isIdempotentMethod(method: string): boolean { + return IDEMPOTENT_METHODS.has(method.toUpperCase()); +} + +/** Default idempotency-key factory: `crypto.randomUUID()` where available. */ +function randomKey(): string { + const c = (globalThis as { crypto?: { randomUUID?: () => string } }).crypto; + if (typeof c?.randomUUID === 'function') return c.randomUUID(); + return `${Date.now().toString(36)}-${Math.random().toString(36).slice(2, 12)}`; +} + /** Eager configuration validation (spec E1, R7). */ function validateConfig(config: GlobalConfig): void { if (!isPlainObject(config)) { throw new ConfigurationError('createClient expects a configuration object.'); } - if (!isPlainObject(config.openapi)) { - throw new ConfigurationError('createClient requires an "openapi" configuration block (R7).'); + if (config.openapi !== undefined && !isPlainObject(config.openapi)) { + throw new ConfigurationError('"openapi" must be a configuration object when provided (R7).'); } const hasBaseURL = typeof config.baseURL === 'string' && config.baseURL.length > 0; const hasEnv = typeof config.activeEnvironment === 'string'; @@ -348,6 +362,12 @@ export function createClient(config: GlobalConfig): ApiClient { // Mutable working copy so `setEnvironment` / `config.update` can mutate. let currentConfig: GlobalConfig = { ...config }; + + // Every error raised by the pipeline goes through here so a consumer's + // `errorEnvelope` parser applies uniformly (#39). Read lazily from + // `currentConfig` so `setEnvironment` and friends stay honored. + const classify = (input: ClassifierInput): ApiError => + classifyError(input, currentConfig.errorEnvelope); const adapter = instantiateAdapter(currentConfig); const moduleDefinitions = collectModuleDefinitions(currentConfig.modules); @@ -375,7 +395,7 @@ export function createClient(config: GlobalConfig): ApiClient { // concurrent 401s trigger exactly one refresh (spec 6.2). const authManager: AuthManager = createAuthManager({ adapter, - classifyError, + classifyError: classify, }); // --- Dev logging --------------------------------------------------------- @@ -481,7 +501,7 @@ export function createClient(config: GlobalConfig): ApiClient { // on it without hanging forever when no runtime schema is configured. let schemaReady: Promise = Promise.resolve(); { - const oa = currentConfig.openapi; + const oa = currentConfig.openapi ?? {}; const wantsRuntime = oa.mode !== 'codegen' && typeof oa.runtimeURL === 'string'; if (wantsRuntime && oa.runtimeURL) { const loader = createSchemaLoader({ cache: schemaCache }); @@ -566,6 +586,19 @@ export function createClient(config: GlobalConfig): ApiClient { // post-refresh retry. const baseHeaders: Record = { ...resolved.headers }; const baseQuery: Record = { ...spec.query }; + + // Idempotency key (#42): one key per logical call, reused across every + // retry attempt of that call, so a retry the server already applied is + // collapsed rather than committed twice. + const idempotencyCfg = currentConfig.http?.idempotency; + const idempotencyHeader = idempotencyCfg?.header ?? 'Idempotency-Key'; + const idempotencyMethods = idempotencyCfg?.methods ?? ['POST', 'PATCH']; + const idempotencyKey = + idempotencyCfg?.enabled && + idempotencyMethods.some((m) => m.toUpperCase() === spec.method.toUpperCase()) && + baseHeaders[idempotencyHeader] === undefined + ? (idempotencyCfg.generate ?? randomKey)() + : undefined; // Set by `fetchThrough` just before a revalidation fetch when a prior // cache entry carries an `ETag` (spec 08.2) — merged into every attempt // (incl. a post-401-refresh retry) for this one `run()` call. @@ -619,6 +652,11 @@ export function createClient(config: GlobalConfig): ApiClient { ...auth.headers, ...conditionalHeaders, }; + // Generated once per logical call (see `idempotencyKey` below) and + // replayed on every attempt — a key that changed per attempt would + // defeat the point (#42). + if (idempotencyKey !== undefined) headers[idempotencyHeader] = idempotencyKey; + const query: Record = { ...baseQuery, ...auth.query }; const url = buildUrl({ @@ -687,7 +725,7 @@ export function createClient(config: GlobalConfig): ApiClient { timeoutMs, }); } - throw classifyError({ kind: 'network', cause, request: outgoing }); + throw classify({ kind: 'network', cause, request: outgoing }); } finally { if (timer !== undefined) clearTimeout(timer); if (onExternalAbort && externalSignal) { @@ -714,8 +752,13 @@ export function createClient(config: GlobalConfig): ApiClient { // Retry options resolved for this request (defaults applied upstream). const retryConfigOnRetry = resolved.retry.onRetry; + // A retried write can double-commit when the failure happened after the + // server already applied it, so unsafe methods are excluded unless the + // caller opts in (#42). An explicit `retryOn` replaces this entirely — + // it is the caller stating the policy themselves. + const retryableByMethod = resolved.retry.retryUnsafeMethods || isIdempotentMethod(spec.method); const retryOpts: ResolvedRetryOptions = { - attempts: resolved.retry.attempts, + attempts: retryableByMethod || resolved.retry.retryOn ? resolved.retry.attempts : 1, backoff: resolved.retry.backoff, baseDelay: resolved.retry.baseDelay, maxDelay: resolved.retry.maxDelay, @@ -740,7 +783,7 @@ export function createClient(config: GlobalConfig): ApiClient { const result = await attempt(); const { status, statusText, headers, data } = result.raw; if (status >= 500 || status === 429) { - throw classifyError({ + throw classify({ kind: 'http', status, statusText, @@ -776,7 +819,7 @@ export function createClient(config: GlobalConfig): ApiClient { cacheConfigured && (resolved.cache.cacheableStatuses ?? []).includes(raw.status); if (raw.status >= 400 && !isNegativeCacheable) { - throw classifyError({ + throw classify({ kind: 'http', status: raw.status, statusText: raw.statusText, @@ -878,7 +921,7 @@ export function createClient(config: GlobalConfig): ApiClient { const negativeTtl = resolved.cache.negativeTtl ?? cacheTtl; /** A negative-cacheable entry rethrows on a cache hit (spec 08.1) — it never resolves as success. */ const throwFromEntry = (entry: CacheEntry): never => { - throw classifyError({ + throw classify({ kind: 'http', status: entry.status, statusText: '', @@ -971,7 +1014,7 @@ export function createClient(config: GlobalConfig): ApiClient { // without throwing so it could be cached above — now enforce the // normal "this call rejects on an error status" contract. if (isNegative) { - throw classifyError({ + throw classify({ kind: 'http', status: response.status, statusText: response.statusText ?? '', diff --git a/packages/core/src/factory/etag.test.ts b/packages/core/src/factory/etag.test.ts index 6b322b0..b815689 100644 --- a/packages/core/src/factory/etag.test.ts +++ b/packages/core/src/factory/etag.test.ts @@ -9,7 +9,7 @@ import type { ModuleContext } from '../types/module.types'; function makeThingsClient() { const { api, mock } = createMockClient({ - cache: { strategy: 'network-first', ttl: 60_000 }, + cache: { enabled: true, strategy: 'network-first', ttl: 60_000 }, modules: { auto: false as const, things: { diff --git a/packages/core/src/factory/mergeModuleConfig.ts b/packages/core/src/factory/mergeModuleConfig.ts index aca2685..12b6f96 100644 --- a/packages/core/src/factory/mergeModuleConfig.ts +++ b/packages/core/src/factory/mergeModuleConfig.ts @@ -124,7 +124,11 @@ export function resolveRequestConfig( ); const cache = deepMerge( - { enabled: true, ttl: 60000, strategy: 'cache-first', maxSize: 500 }, + // v2: off by default. Most consumers pair this client with a caching + // layer of their own (TanStack Query, SWR); a second, invisible cache + // underneath it serves stale reads after a write with no way to see it + // (#40). Opt in with `cache: { enabled: true }`. + { enabled: false, ttl: 60000, strategy: 'cache-first', maxSize: 500 }, global.cache ?? {}, moduleCfg?.cache ?? {}, perCall?.cache ?? {}, @@ -138,6 +142,12 @@ export function resolveRequestConfig( baseDelay: 500, maxDelay: 30000, jitter: true, + // v2: only methods that are idempotent by definition are retried by + // default. A 5xx or dropped connection on a POST can land *after* the + // server committed the write, so a blind retry double-creates (#42). + // Opt an unsafe method in with `retryUnsafeMethods: true` — pair it with + // `idempotency` so the server can collapse the duplicate. + retryUnsafeMethods: false, }, global.http?.retry ?? {}, moduleCfg?.retry ?? {}, diff --git a/packages/core/src/factory/negativeCaching.test.ts b/packages/core/src/factory/negativeCaching.test.ts index 8caa7b2..106ddf1 100644 --- a/packages/core/src/factory/negativeCaching.test.ts +++ b/packages/core/src/factory/negativeCaching.test.ts @@ -9,7 +9,7 @@ import type { ModuleContext } from '../types/module.types'; function makeUsersClient(cache: { cacheableStatuses?: number[]; negativeTtl?: number }) { const { api, mock } = createMockClient({ - cache, + cache: { enabled: true, ...cache }, modules: { auto: false as const, users: { @@ -20,7 +20,7 @@ function makeUsersClient(cache: { cacheableStatuses?: number[]; negativeTtl?: nu ( await ctx.request( { method: 'POST', path: '/users', body: { id } }, - { cache: { invalidatesTags: [`user:${id}`] } }, + { cache: { enabled: true, invalidatesTags: [`user:${id}`] } }, ) ).data, }, @@ -97,7 +97,7 @@ describe('negative caching', () => { it('a tagged negative entry IS cleared by invalidatesTags, same as any other entry', async () => { const mock2 = createMockClient({ - cache: { cacheableStatuses: [404], negativeTtl: 60_000 }, + cache: { enabled: true, cacheableStatuses: [404], negativeTtl: 60_000 }, modules: { auto: false as const, users: { @@ -106,14 +106,14 @@ describe('negative caching', () => { ( await ctx.request( { method: 'GET', path: '/users/{id}', pathParams: { id } }, - { cache: { tags: [`user:${id}`] } }, + { cache: { enabled: true, tags: [`user:${id}`] } }, ) ).data, createUser: async (ctx: ModuleContext, id: string) => ( await ctx.request( { method: 'POST', path: '/users', body: { id } }, - { cache: { invalidatesTags: [`user:${id}`] } }, + { cache: { enabled: true, invalidatesTags: [`user:${id}`] } }, ) ).data, }, diff --git a/packages/core/src/factory/schemaVersionCacheBusting.test.ts b/packages/core/src/factory/schemaVersionCacheBusting.test.ts index d80d088..b7d317f 100644 --- a/packages/core/src/factory/schemaVersionCacheBusting.test.ts +++ b/packages/core/src/factory/schemaVersionCacheBusting.test.ts @@ -27,7 +27,7 @@ function makeClient(mock: ReturnType, currentDoc: () = openapi: { mode: 'runtime', runtimeURL: 'http://schema.test/openapi.json' }, http: { adapter: mock, retry: { attempts: 1, baseDelay: 0, maxDelay: 0, jitter: false } }, dev: { schemaRefreshInterval: 20 }, - cache: { strategy: 'cache-first', ttl: 60_000 }, + cache: { enabled: true, strategy: 'cache-first', ttl: 60_000 }, modules: { auto: false as const, things: { diff --git a/packages/core/src/factory/tagInvalidation.test.ts b/packages/core/src/factory/tagInvalidation.test.ts index adfd69b..900048c 100644 --- a/packages/core/src/factory/tagInvalidation.test.ts +++ b/packages/core/src/factory/tagInvalidation.test.ts @@ -17,10 +17,7 @@ type Api = { reports: { getDashboard: () => Promise; }; - cache: { - invalidateTags(tags: string[]): number; - get(key: string): unknown; - }; + cache: { enabled: true; invalidateTags(tags: string[]): number; get(key: string): unknown }; }; /** A client whose `users` module tags GET-by-id under `user:` and whose @@ -40,14 +37,14 @@ function makeClient(mock: ReturnType, getToken: () => ( await ctx.request( { method: 'GET', path: '/users/{id}', pathParams: { id } }, - { cache: { tags: [`user:${id}`] } }, + { cache: { enabled: true, tags: [`user:${id}`] } }, ) ).data, updateUser: async (ctx: ModuleContext, id: string) => ( await ctx.request( { method: 'PATCH', path: '/users/{id}', pathParams: { id }, body: {} }, - { cache: { invalidatesTags: [`user:${id}`] } }, + { cache: { enabled: true, invalidatesTags: [`user:${id}`] } }, ) ).data, }, @@ -58,7 +55,7 @@ function makeClient(mock: ReturnType, getToken: () => ( await ctx.request( { method: 'GET', path: '/dashboard' }, - { cache: { tags: ['dashboard'] } }, + { cache: { enabled: true, tags: ['dashboard'] } }, ) ).data, }, @@ -136,7 +133,7 @@ describe('tag-based invalidation', () => { ( await ctx.request( { method: 'GET', path: '/users/{id}', pathParams: { id } }, - { cache: { tags: [`user:${id}`, 'extra-tag'] } }, + { cache: { enabled: true, tags: [`user:${id}`, 'extra-tag'] } }, ) ).data, }, @@ -144,7 +141,7 @@ describe('tag-based invalidation', () => { }, }) as unknown as { users: { getUser: (id: string) => Promise }; - cache: { invalidateTags(tags: string[]): number }; + cache: { enabled: true; invalidateTags(tags: string[]): number }; }; await api.users.getUser('9'); @@ -163,7 +160,7 @@ describe('tag-based invalidation', () => { baseURL: 'http://mock.test', openapi: { mode: 'runtime' }, http: { adapter: mock, retry: { attempts: 1, baseDelay: 0, maxDelay: 0, jitter: false } }, - cache: { maxSize: 1 }, + cache: { enabled: true, maxSize: 1 }, modules: { auto: false as const, users: { @@ -172,7 +169,7 @@ describe('tag-based invalidation', () => { ( await ctx.request( { method: 'GET', path: '/users/{id}', pathParams: { id } }, - { cache: { tags: [`user:${id}`] } }, + { cache: { enabled: true, tags: [`user:${id}`] } }, ) ).data, }, @@ -180,7 +177,7 @@ describe('tag-based invalidation', () => { }, }) as unknown as { users: { getUser: (id: string) => Promise }; - cache: { invalidateTags(tags: string[]): number }; + cache: { enabled: true; invalidateTags(tags: string[]): number }; }; await api.users.getUser('1'); // stored under user:1, maxSize 1 diff --git a/packages/core/src/http/adapters/axiosAdapter.ts b/packages/core/src/http/adapters/axiosAdapter.ts index cff7e10..44e42d6 100644 --- a/packages/core/src/http/adapters/axiosAdapter.ts +++ b/packages/core/src/http/adapters/axiosAdapter.ts @@ -6,6 +6,19 @@ import type { AdapterResponse, ApiRequest, ResponseType } from '../../types/http.types'; import type { HttpAdapter } from './adapterInterface'; +/** + * True for a body that carries (or needs the runtime to generate) its own + * Content-Type: FormData needs a multipart boundary, URLSearchParams its form + * encoding, a Blob its own type. + */ +function isSelfTypedBody(body: unknown): boolean { + return ( + (typeof FormData !== 'undefined' && body instanceof FormData) || + (typeof URLSearchParams !== 'undefined' && body instanceof URLSearchParams) || + (typeof Blob !== 'undefined' && body instanceof Blob) + ); +} + /** * Minimal structural shape of the axios surface we rely on. Kept local so the * package has no hard type dependency on axios. @@ -67,7 +80,18 @@ export function createAxiosAdapter(instance?: AxiosInstanceLike): HttpAdapter { responseType, validateStatus: () => true, }; - if (request.body !== undefined) config.data = request.body; + if (request.body !== undefined) { + config.data = request.body; + // Let axios/the runtime set the multipart boundary or form encoding — + // a configured `application/json` header would corrupt the body (#38). + if (isSelfTypedBody(request.body)) { + for (const key of Object.keys(config.headers as Record)) { + if (key.toLowerCase() === 'content-type') { + delete (config.headers as Record)[key]; + } + } + } + } if (request.signal) config.signal = request.signal; if (request.timeout !== undefined) config.timeout = request.timeout; if (request.meta?.['cookieAuth'] === true) config.withCredentials = true; diff --git a/packages/core/src/http/adapters/fetchAdapter.ts b/packages/core/src/http/adapters/fetchAdapter.ts index 9b1d779..37f92c4 100644 --- a/packages/core/src/http/adapters/fetchAdapter.ts +++ b/packages/core/src/http/adapters/fetchAdapter.ts @@ -15,6 +15,14 @@ function flattenHeaders(headers: Headers): Record { return out; } +/** Remove a header regardless of the casing it was supplied in. */ +function deleteHeader(headers: Record, name: string): void { + const lower = name.toLowerCase(); + for (const key of Object.keys(headers)) { + if (key.toLowerCase() === lower) delete headers[key]; + } +} + /** True for plain JSON-serializable objects (not Blob/FormData/ArrayBuffer/etc.). */ function isPlainBody(body: unknown): boolean { if (body === null || typeof body !== 'object') return false; @@ -69,6 +77,13 @@ function buildInit(request: ApiRequest): RequestInit { } } else { body = request.body as BodyInit; + // FormData/URLSearchParams must set their own Content-Type: multipart + // needs a generated boundary the caller cannot know. A globally + // configured `application/json` header would otherwise silently produce + // an unparsable body (#38). + if (body instanceof FormData || body instanceof URLSearchParams) { + deleteHeader(headers, 'content-type'); + } } } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a0e9014..2a33097 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -41,6 +41,7 @@ export { AuthError } from './errors/AuthError'; export { SchemaError, SchemaParseError } from './errors/SchemaError'; export { ConfigurationError } from './errors/ConfigurationError'; export { classifyError, extractServerError } from './errors/errorClassifier'; +export type { ErrorEnvelopeParser } from './errors/errorClassifier'; // --- HTTP adapters --- export type { HttpAdapter, AdapterFactory } from './http/adapters/adapterInterface'; @@ -61,6 +62,8 @@ export { } from './utilities/cancellation'; export type { CancellationManager } from './utilities/cancellation'; export { withRetry, computeBackoff, parseRetryAfter } from './utilities/retry'; +export { paginate, paginateItems, collectPages } from './utilities/paginate'; +export type { Cursor, PaginateOptions } from './utilities/paginate'; export type { ResolvedRetryOptions } from './utilities/retry'; // --- Auth --- @@ -108,12 +111,9 @@ export { // --- Environment --- export { detectEnvironment } from './environment/detect'; -export { - readServerHeader, - readServerCookie, - serverTenantResolver, - serverTokenFromCookie, -} from './environment/serverContext'; +// Next.js server-context helpers moved to the `/next` subpath in v2 so that +// non-Next projects never pull `next/headers` into their bundle graph (#41): +// import { serverTokenFromCookie } from '@developerehsan/api-client/next' // --- Types --- export type { @@ -122,6 +122,7 @@ export type { PerCallConfig, ResolvedRequestConfig, RetryConfig, + IdempotencyConfig, HttpConfig, CancellationConfig, TenancyConfig, diff --git a/packages/core/src/next/index.ts b/packages/core/src/next/index.ts new file mode 100644 index 0000000..ab1c9da --- /dev/null +++ b/packages/core/src/next/index.ts @@ -0,0 +1,14 @@ +/** + * Next.js-only entry: `@developerehsan/api-client/next`. + * + * Everything here reaches for `next/headers`, so it lives behind its own + * subpath. Projects that are not Next.js never import this module and so never + * make a bundler try to resolve `next` (issue #41). + */ + +export { + readServerHeader, + readServerCookie, + serverTenantResolver, + serverTokenFromCookie, +} from '../environment/serverContext'; diff --git a/packages/core/src/rpc/protocol.ts b/packages/core/src/rpc/protocol.ts index 0850909..eee9662 100644 --- a/packages/core/src/rpc/protocol.ts +++ b/packages/core/src/rpc/protocol.ts @@ -62,9 +62,60 @@ export interface RpcErr { error: RpcErrorShape; } +/** + * A result the route must stream back verbatim instead of JSON-encoding it: + * the method was declared binary server-side (`binary` on the handler) and + * produced a `Blob`/`ArrayBuffer`/`Uint8Array` (#37). + * + * Only the server can mark a method binary — a client-supplied `responseType` + * is dropped by `sanitizePerCall` (S4) and that stays true here. + */ +export interface RpcBinaryOk { + ok: true; + binary: true; + data: unknown; + contentType?: string; + contentDisposition?: string; +} + /** Uniform result of a handled RPC call. `handle()` never rejects with this. */ export type RpcResponse = RpcOk | RpcErr; +/** What `handle()` can return once binary methods are in play. */ +export type RpcHandledResponse = RpcResponse | RpcBinaryOk; + +/** True for the binary envelope, which must bypass `JSON.stringify`. */ +export function isRpcBinaryOk(value: unknown): value is RpcBinaryOk { + return ( + typeof value === 'object' && + value !== null && + (value as { ok?: unknown }).ok === true && + (value as { binary?: unknown }).binary === true + ); +} + +/** + * Body a browser sends when a call carries binary parts (#38). The JSON + * envelope travels in the `__rpc` field with each binary argument replaced by + * a `{ __rpcFile: '' }` marker; the bytes travel as their own part + * under that field name, so nothing has to be base64-inflated. + */ +export const RPC_MULTIPART_ENVELOPE = '__rpc'; + +/** Placeholder standing in for a binary argument inside the JSON envelope. */ +export interface RpcFileMarker { + __rpcFile: string; +} + +/** True for a {@link RpcFileMarker} placeholder. */ +export function isRpcFileMarker(value: unknown): value is RpcFileMarker { + return ( + typeof value === 'object' && + value !== null && + typeof (value as { __rpcFile?: unknown }).__rpcFile === 'string' + ); +} + /** * A batch of calls coalesced by the browser client into one round-trip. The * server validates and dispatches each sub-call INDIVIDUALLY (allowlist + diff --git a/packages/core/src/server/createRpcHandler.ts b/packages/core/src/server/createRpcHandler.ts index 84ae39a..90eb331 100644 --- a/packages/core/src/server/createRpcHandler.ts +++ b/packages/core/src/server/createRpcHandler.ts @@ -10,7 +10,13 @@ * rehydratable errors to the browser. */ -import { type RpcCall, type RpcResponse, isRpcBatchRequest } from '../rpc/protocol'; +import { + type RpcCall, + type RpcHandledResponse, + type RpcResponse, + isRpcBatchRequest, + isRpcBinaryOk, +} from '../rpc/protocol'; import { assertServerOnly } from '../utilities/serverOnly'; import { RpcSecurityError, @@ -74,6 +80,31 @@ export type ExposeMap = { export interface RpcHandlerOptions { /** The allowlist of callable `module.method` pairs (S1). Required. */ expose: ExposeMap; + /** + * Methods whose response is an opaque file/stream rather than JSON — CSV and + * PDF exports, attachment downloads (#37). Same shape as `expose`, and a + * method must be in BOTH to be callable. + * + * Declared here and nowhere else on purpose: a client-supplied + * `responseType` is stripped by `sanitizePerCall` (S4), so the browser can + * never turn a JSON method into a byte stream. The handler forces + * `responseType: 'blob'` for these and the route streams the result back + * with its own `Content-Type`/`Content-Disposition` instead of wrapping it + * in the JSON envelope. + * + * @default undefined (every exposed method is treated as JSON) + * @example `binary: { exports: ['flagsCsv', 'logsCsv'] }` + */ + binary?: ExposeMap; + /** + * Names the file a binary response downloads as, by setting + * `Content-Disposition`. Return `undefined` to leave it unset (the browser + * then renders inline where it can). + * + * @default undefined (no Content-Disposition header) + * @example `binaryFilename: (call) => `${call.method}-${Date.now()}.csv`` + */ + binaryFilename?: (call: RpcCall, ctx: RpcRequestContext) => string | undefined; /** * Per-call permission check (S3), run AFTER the allowlist but BEFORE dispatch. * Return `false` (or reject) to deny — the caller then gets the same uniform @@ -162,7 +193,7 @@ export interface RpcHandler { * ) * if (res.ok) console.log(res.data) // → Pet */ - handle(payload: unknown, ctx?: RpcRequestContext): Promise; + handle(payload: unknown, ctx?: RpcRequestContext): Promise; /** * Validate and dispatch a batch envelope (`{ __rpcBatch: RpcCall[] }`). Each * sub-call runs through the SAME per-call trust boundary as {@link handle} @@ -187,13 +218,37 @@ function defaultDev(): boolean { } /** Whether `module.method` is present in the allowlist (S1). */ -function isExposed(expose: ExposeMap, moduleName: string, methodName: string): boolean { +function isExposed( + expose: ExposeMap | undefined, + moduleName: string, + methodName: string, +): boolean { + if (!expose) return false; const entry = (expose as Record)[moduleName]; if (entry === undefined) return false; if (entry === true) return true; return entry.includes(methodName); } +/** True for a payload the route can stream back as-is. */ +function isBinaryPayload(value: unknown): boolean { + return ( + (typeof Blob !== 'undefined' && value instanceof Blob) || + value instanceof ArrayBuffer || + ArrayBuffer.isView(value) || + (typeof ReadableStream !== 'undefined' && value instanceof ReadableStream) + ); +} + +/** + * Strip anything that could break out of the quoted `filename="..."` of a + * `Content-Disposition` header — a header-injection guard, since the name may + * be derived from user-controlled data. + */ +function sanitizeFilename(name: string): string { + return name.replace(/[^\w.\-+ ]/g, '_').slice(0, 120); +} + /** Validate the envelope shape and names; unsafe names collapse to not-available (S1, S2, S9). */ function parsePayload(payload: unknown): RpcCall { if (typeof payload !== 'object' || payload === null) { @@ -255,7 +310,10 @@ export function createRpcHandler( * The single per-call trust boundary. Runs for every call — standalone AND * each batch entry — so a batch can never skip a check (S13/S15). */ - const dispatchOne = async (payload: unknown, ctx: RpcRequestContext): Promise => { + const dispatchOne = async ( + payload: unknown, + ctx: RpcRequestContext, + ): Promise => { let call: RpcCall | null = null; try { call = parsePayload(payload); @@ -286,20 +344,42 @@ export function createRpcHandler( // S4: drop client-controlled perCall except a clamped timeout. const perCall = sanitizePerCall(call.args[1], maxTimeout); + // `responseType` is forced from the SERVER-side `binary` allowlist, never + // from the wire — sanitizePerCall already dropped any client value (S4). + const wantsBinary = isExposed(options.binary, call.module, call.method); + const effectivePerCall = wantsBinary + ? { ...perCall, responseType: 'blob' as const } + : perCall; + let data: unknown = await (fn as (input?: unknown, perCall?: unknown) => Promise)( input, - perCall, + effectivePerCall, ); // S12: optional response projection/redaction. if (options.transformResult) data = await options.transformResult(data, call, ctx); + if (wantsBinary && isBinaryPayload(data)) { + const disposition = options.binaryFilename?.(call, ctx); + return { + ok: true, + binary: true, + data, + ...(typeof (data as Blob).type === 'string' && (data as Blob).type + ? { contentType: (data as Blob).type } + : {}), + ...(disposition + ? { contentDisposition: `attachment; filename="${sanitizeFilename(disposition)}"` } + : {}), + }; + } + return { ok: true, data }; } catch (error) { return fail(error, call); } }; - const handle = (payload: unknown, ctx: RpcRequestContext = {}): Promise => + const handle = (payload: unknown, ctx: RpcRequestContext = {}): Promise => dispatchOne(payload, ctx); const handleBatch = async ( @@ -320,7 +400,12 @@ export function createRpcHandler( } // Each sub-call is fully, independently validated + dispatched; one failure // never affects its siblings (partial success is expected). - return Promise.all(calls.map((c) => dispatchOne(c, ctx))); + const results = await Promise.all(calls.map((c) => dispatchOne(c, ctx))); + // A batch is one JSON array, so a byte stream has nowhere to go in it. + // Call a binary method on its own rather than silently JSON-mangling it. + return results.map((r) => + isRpcBinaryOk(r) ? fail(new RpcSecurityError('binary_not_batchable', 400), null) : r, + ); }; return { handle, handleBatch }; diff --git a/packages/core/src/server/index.ts b/packages/core/src/server/index.ts index 47eceb6..150b204 100644 --- a/packages/core/src/server/index.ts +++ b/packages/core/src/server/index.ts @@ -24,4 +24,10 @@ export { type RateLimitHit, type RateLimitCall, } from './rateLimit'; -export type { RpcCall, RpcResponse, RpcErrorShape } from '../rpc/protocol'; +export type { + RpcCall, + RpcResponse, + RpcErrorShape, + RpcBinaryOk, + RpcHandledResponse, +} from '../rpc/protocol'; diff --git a/packages/core/src/server/routeHandler.ts b/packages/core/src/server/routeHandler.ts index 63aadf3..2b5e4ee 100644 --- a/packages/core/src/server/routeHandler.ts +++ b/packages/core/src/server/routeHandler.ts @@ -6,7 +6,13 @@ * plus an Origin / Sec-Fetch-Site check. It also caps body size (S6). */ -import { type RpcResponse, isRpcBatchRequest } from '../rpc/protocol'; +import { + RPC_MULTIPART_ENVELOPE, + type RpcResponse, + isRpcBatchRequest, + isRpcBinaryOk, + isRpcFileMarker, +} from '../rpc/protocol'; import type { RpcHandler, RpcRequestContext } from './createRpcHandler'; /** Options for {@link createRpcRouteHandler}. */ @@ -21,6 +27,25 @@ export interface RpcRouteOptions { allowedOrigins?: readonly string[] | ((origin: string) => boolean); /** Max request body size in bytes. Default 128 KB. */ maxBodyBytes?: number; + /** + * Accept `multipart/form-data` bodies so calls can carry `File`/`Blob` + * arguments (#38). Off by default: it widens the accepted content types, and + * a multipart body is not covered by the JSON-content-type half of the CSRF + * defence, so it leans entirely on the Origin check. + * + * Leave `csrf` on when enabling this, and size it with `maxUploadBytes`. + * + * @default false + * @example `{ multipart: true, maxUploadBytes: 10 * 1024 * 1024 }` + */ + multipart?: boolean; + /** + * Max multipart body size in bytes. Separate from `maxBodyBytes` because an + * upload is legitimately far larger than a JSON envelope — raising the JSON + * cap to fit a file would widen the DoS surface on every other call. + * @default 10485760 (10 MB) + */ + maxUploadBytes?: number; } /** Read one cookie value from a raw `Cookie` header, or `undefined`. */ @@ -43,6 +68,87 @@ function jsonResponse(body: RpcResponse | RpcResponse[], status: number): Respon }); } +/** + * Read at most `limit` bytes off the request body, aborting as soon as the cap + * is passed. `Content-Length` is a client-supplied claim — it can be wrong, or + * absent entirely under chunked transfer-encoding — so the limit has to be + * enforced against the bytes themselves, not the header. + * + * Returns `null` when the body exceeds the cap. + */ +async function readLimited(req: Request, limit: number): Promise { + const body = req.body; + if (!body) return new ArrayBuffer(0); + + const reader = body.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + total += value.byteLength; + if (total > limit) { + await reader.cancel(); + return null; + } + chunks.push(value); + } + } finally { + reader.releaseLock(); + } + + const out = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + out.set(chunk, offset); + offset += chunk.byteLength; + } + return out.buffer as ArrayBuffer; +} + +/** + * Rebuild the call envelope from a multipart body: the JSON lives in the + * `__rpc` part with `{ __rpcFile }` placeholders, each naming the part that + * carries its bytes (#38). + * + * Only placeholders are substituted, and only from parts actually present, so + * a crafted body can at worst inject a `File` the handler's own input + * validation then sees. + */ +function reviveMultipart(form: FormData): unknown { + const raw = form.get(RPC_MULTIPART_ENVELOPE); + if (typeof raw !== 'string') throw new SyntaxError('missing envelope'); + const envelope: unknown = JSON.parse(raw); + + const substitute = (value: unknown): unknown => { + if (isRpcFileMarker(value)) { + const file = form.get(value.__rpcFile); + return file === null ? undefined : file; + } + if (Array.isArray(value)) return value.map(substitute); + if (typeof value === 'object' && value !== null) { + const out: Record = {}; + for (const [k, v] of Object.entries(value as Record)) { + // defineProperty, not `out[k] = …`: a plain assignment with the key + // `__proto__` invokes the prototype setter, so the key vanishes from + // `Object.keys` and slips past the handler's FORBIDDEN_KEYS check. + // This keeps it an own, enumerable property so that guard still sees it. + Object.defineProperty(out, k, { + value: substitute(v), + enumerable: true, + writable: true, + configurable: true, + }); + } + return out; + } + return value; + }; + + return substitute(envelope); +} + /** Uniform generic error envelope (S8/S9). */ function genericError(status: number, code: string): RpcResponse { return { @@ -105,34 +211,53 @@ export function createRpcRouteHandler( ): (req: Request) => Promise { const csrf = options.csrf ?? true; const maxBodyBytes = options.maxBodyBytes ?? 128 * 1024; + const multipart = options.multipart ?? false; + const maxUploadBytes = options.maxUploadBytes ?? 10 * 1024 * 1024; return async (req: Request): Promise => { if (req.method !== 'POST') return jsonResponse(genericError(405, 'method_not_allowed'), 405); const contentType = req.headers.get('content-type') ?? ''; - if (!contentType.includes('application/json')) { + const isMultipart = multipart && contentType.includes('multipart/form-data'); + if (!isMultipart && !contentType.includes('application/json')) { return jsonResponse(genericError(415, 'unsupported_media_type'), 415); } + // Ordered before any body read so a rejected origin never costs us the + // parse of a large upload. if (csrf && !originAllowed(req, options.allowedOrigins)) { return jsonResponse(genericError(403, 'forbidden'), 403); } + const limit = isMultipart ? maxUploadBytes : maxBodyBytes; const contentLength = req.headers.get('content-length'); - if (contentLength && Number(contentLength) > maxBodyBytes) { + // Cheap pre-check on the declared length; `readLimited` below is what + // actually enforces the cap, since the header can lie or be absent. + if (contentLength && Number(contentLength) > limit) { return jsonResponse(genericError(413, 'payload_too_large'), 413); } - const text = await req.text(); - if (text.length > maxBodyBytes) { - return jsonResponse(genericError(413, 'payload_too_large'), 413); - } + const raw = await readLimited(req, limit); + if (raw === null) return jsonResponse(genericError(413, 'payload_too_large'), 413); let payload: unknown; - try { - payload = JSON.parse(text); - } catch { - return jsonResponse(genericError(400, 'bad_json'), 400); + if (isMultipart) { + try { + // Re-wrap the already-bounded bytes so the runtime's own multipart + // parser does the parsing, with the original Content-Type (boundary). + const form = await new Response(raw, { + headers: { 'content-type': contentType }, + }).formData(); + payload = reviveMultipart(form); + } catch { + return jsonResponse(genericError(400, 'bad_multipart'), 400); + } + } else { + try { + payload = JSON.parse(new TextDecoder().decode(raw)); + } catch { + return jsonResponse(genericError(400, 'bad_json'), 400); + } } const ctx: RpcRequestContext = { @@ -145,6 +270,20 @@ export function createRpcRouteHandler( if (isRpcBatchRequest(payload)) { return jsonResponse(await handler.handleBatch(payload, ctx), 200); } - return jsonResponse(await handler.handle(payload, ctx), 200); + const result = await handler.handle(payload, ctx); + // A method the server declared `binary` streams back as-is, carrying its + // own Content-Type/Content-Disposition instead of a JSON envelope (#37). + if (isRpcBinaryOk(result)) { + return new Response(result.data as BodyInit, { + status: 200, + headers: { + 'content-type': result.contentType || 'application/octet-stream', + ...(result.contentDisposition + ? { 'content-disposition': result.contentDisposition } + : {}), + }, + }); + } + return jsonResponse(result, 200); }; } diff --git a/packages/core/src/types/config.types.ts b/packages/core/src/types/config.types.ts index f1662f7..c1232c1 100644 --- a/packages/core/src/types/config.types.ts +++ b/packages/core/src/types/config.types.ts @@ -9,6 +9,7 @@ */ import type { ApiError } from '../errors/ApiError'; +import type { ErrorEnvelopeParser } from '../errors/errorClassifier'; import type { ComposedHooks } from '../factory/composeHooks'; import type { AuthConfig } from './auth.types'; import type { CacheConfig } from './cache.types'; @@ -86,6 +87,67 @@ export interface RetryConfig { * @example `retry: { onRetry: (attempt, error) => console.warn('retry', attempt, error) }` */ onRetry?: (attempt: number, error: ApiError) => void; + /** + * Allow retries on methods that are not idempotent by definition + * (`POST`/`PATCH`). + * + * Off by default since v2: a 5xx or dropped connection on a write can arrive + * *after* the server already committed it, so an automatic retry can create + * the resource twice (#42). Turn it on only when the endpoint is safe to + * repeat — ideally together with {@link IdempotencyConfig} so the server can + * collapse the duplicate itself. + * + * `retryOn`, when supplied, replaces this check entirely. + * + * @default false + * @example `retry: { retryUnsafeMethods: true }` + */ + retryUnsafeMethods?: boolean; +} + +/** + * Attaches an idempotency key to unsafe requests so a retry — this client's or + * the caller's — is collapsed server-side instead of committing twice (#42). + * + * The key is generated once per logical call and reused across every retry + * attempt of that call, which is what makes the retry provably safe. A + * different call gets a different key. + * + * Your API has to honor the header for this to mean anything; the client only + * guarantees the key is stable across retries. + * + * @example + * ```ts + * http: { idempotency: { enabled: true } } // Idempotency-Key, POST+PATCH + * http: { idempotency: { enabled: true, header: 'X-Request-Id' } } + * ``` + */ +export interface IdempotencyConfig { + /** + * Generate and send a key for every matching request. + * @default false + * @example `idempotency: { enabled: true }` + */ + enabled?: boolean; + /** + * Header the key is sent in. + * @default 'Idempotency-Key' + * @example `idempotency: { header: 'X-Idempotency-Key' }` + */ + header?: string; + /** + * Methods that get a key. Safe methods never need one. + * @default ['POST', 'PATCH'] + * @example `idempotency: { methods: ['POST', 'PUT', 'PATCH'] }` + */ + methods?: string[]; + /** + * Key factory. Defaults to `crypto.randomUUID()`, falling back to a + * random/timestamp string where that is unavailable. + * @default optional, unset means a random UUID per call + * @example `idempotency: { generate: () => myRequestId() }` + */ + generate?: () => string; } /** @@ -171,6 +233,12 @@ export interface HttpConfig { * @example `http: { retry: { attempts: 5 } }` */ retry?: RetryConfig; + /** + * Idempotency-key policy for unsafe methods (see {@link IdempotencyConfig}). + * @default optional, unset means no key is sent + * @example `http: { idempotency: { enabled: true } }` + */ + idempotency?: IdempotencyConfig; /** * When `true`, in-flight identical requests are coalesced into a single * network call. @@ -530,10 +598,11 @@ export interface GlobalConfig { /** * OpenAPI wiring and validation policy (see {@link OpenApiConfig}). - * @default no default — required + * @default optional, unset means `{}` — no codegen spec, no runtime fetch, + * no response validation. Only needed for the OpenAPI-driven features. * @example `{ openapi: { runtimeURL: 'https://petstore3.swagger.io/api/v3/openapi.json' } }` */ - openapi: OpenApiConfig; + openapi?: OpenApiConfig; /** * Transport configuration (see {@link HttpConfig}). @@ -547,6 +616,28 @@ export interface GlobalConfig { * @example `{ auth: { strategy: 'bearer', getToken: () => localStorage.getItem('token') } }` */ auth?: AuthConfig; + /** + * Teaches the error classifier a non-default error-body shape. Return the + * flattened `{ code, message, details }` for a body you recognize, or `null` + * to fall back to the built-in flat `{ code, message }` detection. + * + * Without this, a nested envelope is not recognized as an error body and the + * whole payload is stringified into `rawBody`, so `error.message` becomes a + * raw JSON blob instead of the server's message (#39). + * + * @default optional, unset means only the flat `{ code, message }` shape is + * recognized + * @example + * ```ts + * // API returns { error: { code, message, details } } + * errorEnvelope: (body) => + * typeof body === 'object' && body !== null && 'error' in body + * ? ((body as { error: ServerErrorBody }).error ?? null) + * : null + * ``` + */ + errorEnvelope?: ErrorEnvelopeParser; + /** * Response cache configuration (see {@link CacheConfig}). * @default optional, unset means the CacheConfig defaults apply @@ -797,7 +888,12 @@ export interface ResolvedRequestConfig { auth: AuthConfig; cache: Required> & CacheConfig & { bust?: boolean; tags?: string[]; invalidatesTags?: string[] }; - retry: Required> & + retry: Required< + Pick< + RetryConfig, + 'attempts' | 'backoff' | 'baseDelay' | 'maxDelay' | 'jitter' | 'retryUnsafeMethods' + > + > & RetryConfig; tenancy: TenancyConfig; validation: ValidationConfig; diff --git a/packages/core/src/utilities/memoryPressure.test.ts b/packages/core/src/utilities/memoryPressure.test.ts index 00bfacf..3771316 100644 --- a/packages/core/src/utilities/memoryPressure.test.ts +++ b/packages/core/src/utilities/memoryPressure.test.ts @@ -16,7 +16,7 @@ function makeThingsClient(mock: ReturnType, cache: unk adapter: mock, retry: { attempts: 1, baseDelay: 0, maxDelay: 0, jitter: false }, }, - cache: cache as never, + cache: { enabled: true, ...(cache as object) } as never, modules: { auto: false as const, things: { diff --git a/packages/core/src/utilities/paginate.ts b/packages/core/src/utilities/paginate.ts new file mode 100644 index 0000000..45a5dc4 --- /dev/null +++ b/packages/core/src/utilities/paginate.ts @@ -0,0 +1,120 @@ +/** + * Cursor-pagination driver (#43). Turns "call this again with the next cursor + * until it stops returning one" into an `AsyncIterable`, so the cursor-threading + * loop is written once rather than re-hand-rolled per endpoint. + * + * Transport-agnostic on purpose: it takes a function, so it works with a module + * method, an RPC-bridge call, or a plain fetch. + */ + +/** Cursor values a page can hand back. */ +export type Cursor = string | number; + +/** How to read a page: where the next cursor lives, and optionally its items. */ +export interface PaginateOptions { + /** + * Pull the cursor for the next page out of this one. Return `null`/ + * `undefined` (or an empty string) when there are no more pages. + * + * @example `nextCursor: (page) => page.nextCursor` + */ + nextCursor: (page: Page) => Cursor | null | undefined; + /** + * Pull the items out of a page. Required only by {@link paginateItems}. + * @example `items: (page) => page.data` + */ + items?: (page: Page) => readonly Item[]; + /** + * Stop after this many pages. A backstop against an endpoint that keeps + * handing back cursors forever. + * @default 10000 + */ + maxPages?: number; + /** Abort between pages. Checked before each fetch, so no extra call is made. */ + signal?: AbortSignal; +} + +const DEFAULT_MAX_PAGES = 10_000; + +/** + * Yield each page in turn, following the cursor until it runs out. + * + * @example + * ```ts + * for await (const page of paginate( + * (cursor) => api.flags.list({ query: { cursor, limit: 100 } }), + * { nextCursor: (p) => p.nextCursor }, + * )) { + * console.log(page.data.length) + * } + * ``` + */ +export async function* paginate( + fetchPage: (cursor?: Cursor) => Promise, + options: PaginateOptions, +): AsyncGenerator { + const maxPages = options.maxPages ?? DEFAULT_MAX_PAGES; + const seen = new Set(); + let cursor: Cursor | undefined; + + for (let page = 0; page < maxPages; page++) { + if (options.signal?.aborted) return; + + const result = cursor === undefined ? await fetchPage() : await fetchPage(cursor); + yield result; + + const next = options.nextCursor(result); + if (next === null || next === undefined || next === '') return; + // A server that echoes the same cursor back would otherwise spin forever + // against a live endpoint. Stop instead, and let maxPages stay a backstop + // for the genuinely-advancing-but-endless case. + if (seen.has(next)) return; + seen.add(next); + cursor = next; + } +} + +/** + * Flatten {@link paginate} into a stream of individual items. Requires + * `options.items`. + * + * @example + * ```ts + * for await (const flag of paginateItems( + * (cursor) => api.flags.list({ query: { cursor } }), + * { nextCursor: (p) => p.nextCursor, items: (p) => p.data }, + * )) { + * console.log(flag.key) + * } + * ``` + */ +export async function* paginateItems( + fetchPage: (cursor?: Cursor) => Promise, + options: PaginateOptions & { items: (page: Page) => readonly Item[] }, +): AsyncGenerator { + for await (const page of paginate(fetchPage, options)) { + yield* options.items(page); + } +} + +/** + * Drain every page into one array. Convenience over {@link paginateItems} for + * the "just give me all of them" case — it buys an unbounded array, so prefer + * iterating when the result set can be large. + * + * @example + * ```ts + * const all = await collectPages( + * (cursor) => api.flags.list({ query: { cursor } }), + * { nextCursor: (p) => p.nextCursor, items: (p) => p.data }, + * ) + * ``` + */ +export async function collectPages( + fetchPage: (cursor?: Cursor) => Promise, + options: PaginateOptions & { items: (page: Page) => readonly Item[] }, +): Promise { + const out: Item[] = []; + for await (const item of paginateItems(fetchPage, options)) out.push(item); + return out; +} diff --git a/packages/core/src/v2-consumer-issues.test.ts b/packages/core/src/v2-consumer-issues.test.ts new file mode 100644 index 0000000..32fe35c --- /dev/null +++ b/packages/core/src/v2-consumer-issues.test.ts @@ -0,0 +1,499 @@ +/** + * Regression cover for the v2 consumer-reported issues (#37–#46). One test per + * behavior that a consumer actually got bitten by. + */ +import { describe, expect, it, vi } from 'vitest'; +import { createIndexedDbStore } from './cache-stores'; +import { classifyError, extractServerError } from './errors/errorClassifier'; +import { createClient } from './factory/createClient'; +import { createRpcHandler } from './server/createRpcHandler'; +import { createRpcRouteHandler } from './server/routeHandler'; +import { createMockClient } from './testing/createMockClient'; +import type { ModuleContext } from './types/module.types'; +import { collectPages, paginate, paginateItems } from './utilities/paginate'; + +// --- #45: openapi optional ------------------------------------------------ +describe('#45 openapi is optional', () => { + it('constructs without an openapi block', () => { + expect(() => createClient({ baseURL: 'https://example.test' })).not.toThrow(); + }); + + it('still rejects a non-object openapi', () => { + expect(() => + createClient({ baseURL: 'https://example.test', openapi: 'nope' as never }), + ).toThrow(/openapi/); + }); +}); + +// --- #39: custom error envelope ------------------------------------------ +describe('#39 errorEnvelope', () => { + const nested = { error: { code: 'flag_not_found', message: 'Flag not found' } }; + + it('without a parser, a nested envelope is not recognized', () => { + expect(extractServerError(nested).serverError).toBeNull(); + }); + + it('a parser flattens the envelope into the error message', () => { + const err = classifyError( + { kind: 'http', status: 404, statusText: 'Not Found', headers: {}, data: nested }, + (body) => (body as typeof nested).error, + ); + expect(err.message).toBe('Flag not found'); + expect(err.code).toBe('flag_not_found'); + }); + + it('a throwing parser falls back instead of masking the failure', () => { + const err = classifyError( + { kind: 'http', status: 500, statusText: 'Server Error', headers: {}, data: nested }, + () => { + throw new Error('bad parser'); + }, + ); + expect(err.status).toBe(500); + }); + + it('is applied to errors raised by the pipeline', async () => { + const { api, mock } = createMockClient({ + errorEnvelope: (body) => + (body as { error?: { code: string; message: string } }).error ?? null, + modules: { + auto: false as const, + flags: { + methods: { + get: async (ctx: ModuleContext) => + (await ctx.request({ method: 'GET', path: '/flags/1' })).data, + }, + }, + }, + }); + mock.on('GET', '/flags/1', { status: 404, data: nested }); + const typed = api as unknown as { flags: { get: () => Promise } }; + await expect(typed.flags.get()).rejects.toThrow('Flag not found'); + }); +}); + +// --- #40: cache off by default ------------------------------------------- +describe('#40 cache is off by default', () => { + const build = (cache?: Record) => { + const { api, mock } = createMockClient({ + ...(cache ? { cache } : {}), + modules: { + auto: false as const, + things: { + methods: { + get: async (ctx: ModuleContext) => + (await ctx.request({ method: 'GET', path: '/things/1' })).data, + }, + }, + }, + }); + return { api: api as unknown as { things: { get: () => Promise } }, mock }; + }; + + it('a repeated GET hits the network twice', async () => { + let hits = 0; + const { api, mock } = build(); + mock.on('GET', '/things/1', () => { + hits++; + return { data: { id: 1 } }; + }); + await api.things.get(); + await api.things.get(); + expect(hits).toBe(2); + }); + + it('opting in restores caching', async () => { + let hits = 0; + const { api, mock } = build({ enabled: true }); + mock.on('GET', '/things/1', () => { + hits++; + return { data: { id: 1 } }; + }); + await api.things.get(); + await api.things.get(); + expect(hits).toBe(1); + }); +}); + +// --- #42: retry safety + idempotency key --------------------------------- +describe('#42 unsafe methods are not retried by default', () => { + const build = (config: Record = {}) => { + const { api, mock } = createMockClient({ + ...config, + modules: { + auto: false as const, + things: { + methods: { + create: async (ctx: ModuleContext) => + (await ctx.request({ method: 'POST', path: '/things', body: { a: 1 } })).data, + replace: async (ctx: ModuleContext) => + (await ctx.request({ method: 'PUT', path: '/things/1', body: { a: 1 } })).data, + }, + }, + }, + }); + return { + api: api as unknown as { + things: { create: () => Promise; replace: () => Promise }; + }, + mock, + }; + }; + + it('a 500 on POST is attempted exactly once', async () => { + let attempts = 0; + const { api, mock } = build(); + mock.on('POST', '/things', () => { + attempts++; + return { status: 500, data: { message: 'boom' } }; + }); + await expect(api.things.create()).rejects.toThrow(); + expect(attempts).toBe(1); + }); + + it('a 500 on PUT still retries (PUT is idempotent)', async () => { + let attempts = 0; + const { api, mock } = build({ http: { retry: { attempts: 3, baseDelay: 0 } } }); + mock.on('PUT', '/things/1', () => { + attempts++; + return { status: 500, data: { message: 'boom' } }; + }); + await expect(api.things.replace()).rejects.toThrow(); + expect(attempts).toBe(3); + }); + + it('retryUnsafeMethods opts POST back in', async () => { + let attempts = 0; + const { api, mock } = build({ + http: { retry: { attempts: 3, baseDelay: 0, retryUnsafeMethods: true } }, + }); + mock.on('POST', '/things', () => { + attempts++; + return { status: 500, data: { message: 'boom' } }; + }); + await expect(api.things.create()).rejects.toThrow(); + expect(attempts).toBe(3); + }); + + it('reuses ONE idempotency key across every retry of a call', async () => { + const keys: Array = []; + const { api, mock } = build({ + http: { + retry: { attempts: 3, baseDelay: 0, retryUnsafeMethods: true }, + idempotency: { enabled: true }, + }, + }); + mock.on('POST', '/things', (req: { headers: Record }) => { + keys.push(req.headers['Idempotency-Key'] ?? req.headers['idempotency-key']); + return { status: 500, data: { message: 'boom' } }; + }); + await expect(api.things.create()).rejects.toThrow(); + expect(keys).toHaveLength(3); + expect(new Set(keys).size).toBe(1); + expect(keys[0]).toBeTruthy(); + }); + + it('sends no key when idempotency is off', async () => { + const seen: Array = []; + const { api, mock } = build(); + mock.on('POST', '/things', (req: { headers: Record }) => { + seen.push(req.headers['Idempotency-Key']); + return { data: { ok: true } }; + }); + await api.things.create(); + expect(seen[0]).toBeUndefined(); + }); +}); + +// --- #43: pagination ------------------------------------------------------ +describe('#43 pagination helpers', () => { + interface Page { + data: number[]; + nextCursor: string | null; + } + const pages: Record = { + __first: { data: [1, 2], nextCursor: 'b' }, + b: { data: [3], nextCursor: null }, + }; + const fetchPage = (cursor?: string | number): Promise => + Promise.resolve(pages[cursor === undefined ? '__first' : String(cursor)] as Page); + + it('walks every page until the cursor runs out', async () => { + const seen: Page[] = []; + for await (const page of paginate(fetchPage, { nextCursor: (p) => p.nextCursor })) { + seen.push(page); + } + expect(seen).toHaveLength(2); + }); + + it('flattens items across pages', async () => { + const items: number[] = []; + for await (const item of paginateItems(fetchPage, { + nextCursor: (p) => p.nextCursor, + items: (p) => p.data, + })) { + items.push(item); + } + expect(items).toEqual([1, 2, 3]); + }); + + it('collectPages drains into one array', async () => { + await expect( + collectPages(fetchPage, { nextCursor: (p) => p.nextCursor, items: (p) => p.data }), + ).resolves.toEqual([1, 2, 3]); + }); + + it('stops instead of spinning when the server echoes the same cursor', async () => { + let calls = 0; + const stuck = (): Promise => { + calls++; + return Promise.resolve({ data: [0], nextCursor: 'same' }); + }; + const seen: Page[] = []; + for await (const page of paginate(stuck, { nextCursor: (p) => p.nextCursor })) { + seen.push(page); + } + // First page, then one more for cursor 'same', then the repeat is detected. + expect(calls).toBe(2); + }); + + it('honors maxPages', async () => { + let n = 0; + const endless = (): Promise => Promise.resolve({ data: [n], nextCursor: String(n++) }); + const seen: Page[] = []; + for await (const page of paginate(endless, { + nextCursor: (p) => p.nextCursor, + maxPages: 3, + })) { + seen.push(page); + } + expect(seen).toHaveLength(3); + }); +}); + +// --- #46: IndexedDB cross-tab invalidation -------------------------------- +describe('#46 IndexedDB crossTab invalidation', () => { + it('does not expose the hooks unless crossTab is on', () => { + const store = createIndexedDbStore(); + expect(store.publishInvalidate).toBeUndefined(); + expect(store.onRemoteInvalidate).toBeUndefined(); + }); + + it('broadcasts to a sibling tab and never echoes to itself', async () => { + const channels: Array<{ name: string; onmessage: ((e: { data: unknown }) => void) | null }> = + []; + class FakeBC { + onmessage: ((e: { data: unknown }) => void) | null = null; + constructor(public name: string) { + channels.push(this); + } + postMessage(data: unknown) { + for (const c of channels) if (c !== this) c.onmessage?.({ data }); + } + } + vi.stubGlobal('BroadcastChannel', FakeBC); + + const tabA = createIndexedDbStore({ crossTab: true }); + const tabB = createIndexedDbStore({ crossTab: true }); + const onA: unknown[] = []; + const onB: unknown[] = []; + tabA.onRemoteInvalidate?.((m) => onA.push(m)); + tabB.onRemoteInvalidate?.((m) => onB.push(m)); + + await tabA.publishInvalidate?.({ type: 'tags', values: ['flag:1'] }); + + expect(onB).toHaveLength(1); + expect(onA).toHaveLength(0); // own-origin message is ignored + vi.unstubAllGlobals(); + }); + + it('degrades to a plain store where BroadcastChannel is absent', () => { + vi.stubGlobal('BroadcastChannel', undefined); + const store = createIndexedDbStore({ crossTab: true }); + expect(store.publishInvalidate).toBeUndefined(); + vi.unstubAllGlobals(); + }); +}); + +// --- #37 / #38: RPC binary + multipart ------------------------------------ +describe('#37/#38 RPC binary responses and multipart uploads', () => { + const makeApi = () => ({ + exports: { + flagsCsv: () => Promise.resolve(new Blob(['a,b\n1,2'], { type: 'text/csv' })), + asJson: () => Promise.resolve({ rows: 2 }), + }, + files: { + upload: (input: unknown) => Promise.resolve({ got: input }), + }, + }); + + const post = (body: BodyInit, contentType?: string): Request => + new Request('https://app.test/api/rpc', { + method: 'POST', + headers: { + origin: 'https://app.test', + host: 'app.test', + ...(contentType ? { 'content-type': contentType } : {}), + }, + body, + }); + + it('streams a declared-binary method back with its own content type', async () => { + const route = createRpcRouteHandler( + createRpcHandler(makeApi(), { + expose: { exports: ['flagsCsv'] }, + binary: { exports: ['flagsCsv'] }, + binaryFilename: () => 'flags.csv', + }), + ); + const res = await route( + post(JSON.stringify({ module: 'exports', method: 'flagsCsv', args: [] }), 'application/json'), + ); + expect(res.headers.get('content-type')).toContain('text/csv'); + expect(res.headers.get('content-disposition')).toBe('attachment; filename="flags.csv"'); + expect(await res.text()).toBe('a,b\n1,2'); + }); + + it('a method not declared binary still returns the JSON envelope', async () => { + const route = createRpcRouteHandler( + createRpcHandler(makeApi(), { expose: { exports: ['asJson'] } }), + ); + const res = await route( + post(JSON.stringify({ module: 'exports', method: 'asJson', args: [] }), 'application/json'), + ); + expect(res.headers.get('content-type')).toContain('application/json'); + expect(await res.json()).toEqual({ ok: true, data: { rows: 2 } }); + }); + + it('binary is server-declared: a client-supplied responseType cannot force it', async () => { + const route = createRpcRouteHandler( + createRpcHandler(makeApi(), { expose: { exports: ['asJson'] } }), + ); + const res = await route( + post( + JSON.stringify({ + module: 'exports', + method: 'asJson', + args: [{}, { responseType: 'blob' }], + }), + 'application/json', + ), + ); + expect(res.headers.get('content-type')).toContain('application/json'); + }); + + it('rejects multipart unless the route opted in', async () => { + const form = new FormData(); + form.append('__rpc', JSON.stringify({ module: 'files', method: 'upload', args: [{}] })); + const route = createRpcRouteHandler( + createRpcHandler(makeApi(), { expose: { files: ['upload'] } }), + ); + const res = await route(post(form)); + expect(res.status).toBe(415); + }); + + it('revives a File argument from a multipart body', async () => { + // Asserted server-side: the response is JSON, so a File would flatten to + // {} on the way back no matter what the handler received. + let received: unknown; + const api = { + files: { + upload: (input: unknown) => { + received = input; + return Promise.resolve({ ok: true }); + }, + }, + }; + const form = new FormData(); + form.append( + '__rpc', + JSON.stringify({ + module: 'files', + method: 'upload', + args: [{ body: { avatar: { __rpcFile: '__file0' } } }], + }), + ); + form.append('__file0', new Blob(['hello'], { type: 'text/plain' }), 'a.txt'); + + const route = createRpcRouteHandler(createRpcHandler(api, { expose: { files: ['upload'] } }), { + multipart: true, + }); + const res = await route(post(form)); + expect((await res.json()).ok).toBe(true); + + const avatar = (received as { body: { avatar: unknown } }).body.avatar; + expect(avatar).toBeInstanceOf(Blob); + expect(await (avatar as Blob).text()).toBe('hello'); + }); + + it('rejects a body that exceeds maxUploadBytes even with no Content-Length', async () => { + const form = new FormData(); + form.append('__rpc', JSON.stringify({ module: 'files', method: 'upload', args: [{}] })); + form.append('__file0', new Blob(['x'.repeat(5000)]), 'big.bin'); + + const route = createRpcRouteHandler( + createRpcHandler(makeApi(), { expose: { files: ['upload'] } }), + { multipart: true, maxUploadBytes: 1024 }, + ); + // Streaming the body strips Content-Length, so only the byte-level cap + // can catch this. + const built = new Request('https://app.test/api/rpc', { method: 'POST', body: form }); + const res = await route( + new Request('https://app.test/api/rpc', { + method: 'POST', + headers: { + origin: 'https://app.test', + host: 'app.test', + 'content-type': built.headers.get('content-type') as string, + }, + body: built.body, + duplex: 'half', + } as RequestInit), + ); + expect(res.status).toBe(413); + }); + + it('a __proto__ key in a multipart envelope stays visible to the input guard', async () => { + let received: unknown = 'NOT CALLED'; + const api = { + files: { + upload: (input: unknown) => { + received = input; + return Promise.resolve({ ok: true }); + }, + }, + }; + const form = new FormData(); + // Written as a raw string on purpose: `{ __proto__: ... }` in a JS object + // literal sets the prototype, so JSON.stringify would emit `{}` and the + // test would prove nothing. + form.append( + '__rpc', + '{"module":"files","method":"upload","args":[{"__proto__":{"polluted":true}}]}', + ); + form.append('__file0', new Blob(['x']), 'a.txt'); + + const route = createRpcRouteHandler(createRpcHandler(api, { expose: { files: ['upload'] } }), { + multipart: true, + }); + const res = await route(post(form)); + + // The handler's FORBIDDEN_KEYS guard must still reject it: a plain + // `out[k] = v` during revival would have moved the key onto the prototype, + // hiding it from Object.keys and silently letting the call through. + expect((await res.json()).ok).toBe(false); + expect(received).toBe('NOT CALLED'); + expect(({} as { polluted?: boolean }).polluted).toBeUndefined(); + }); + + it('a binary method inside a batch is refused rather than JSON-mangled', async () => { + const h = createRpcHandler(makeApi(), { + expose: { exports: ['flagsCsv'] }, + binary: { exports: ['flagsCsv'] }, + }); + const [res] = await h.handleBatch({ + __rpcBatch: [{ module: 'exports', method: 'flagsCsv', args: [] }], + }); + expect(res?.ok).toBe(false); + }); +}); diff --git a/packages/core/tsup.config.ts b/packages/core/tsup.config.ts index 7211acf..259dc9b 100644 --- a/packages/core/tsup.config.ts +++ b/packages/core/tsup.config.ts @@ -9,6 +9,7 @@ export default defineConfig({ testing: 'src/testing/index.ts', server: 'src/server/index.ts', browser: 'src/browser/index.ts', + next: 'src/next/index.ts', 'cache-stores': 'src/cache-stores/index.ts', }, format: ['esm', 'cjs'], From 5b72242b987a3ec64d0fa235b48d23269e841e26 Mon Sep 17 00:00:00 2001 From: developerehsan Date: Wed, 21 Oct 2026 17:09:57 +0500 Subject: [PATCH 2/3] fix: restore package.json formatting after exports edit Co-Authored-By: Claude Opus 5 --- packages/core/package.json | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/core/package.json b/packages/core/package.json index 10e79bf..94d7123 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -50,9 +50,7 @@ "access": "public", "provenance": true }, - "files": [ - "dist" - ], + "files": ["dist"], "exports": { ".": { "types": "./dist/index.d.ts", From 09d2e778629f9d73c7fad5cf4790d1eb8fe20aeb Mon Sep 17 00:00:00 2001 From: developerehsan Date: Wed, 21 Oct 2026 17:14:16 +0500 Subject: [PATCH 3/3] fix(server): drop reader.cancel() from the body size guard Cancelling the request-body stream closes it while the runtime may still be enqueueing already-received chunks, which surfaces as ERR_INVALID_STATE ('ReadableStream is already closed') thrown inside undici. That is an unhandled rejection no caller can catch, and an unhandled rejection can take down a server process. Caught by CI on Node 24 (all 387 tests passed, but the run reported one unhandled error); it is a timing-dependent race that did not reproduce locally across repeated runs on the same version. Stopping the read and releasing the lock is sufficient: the caller responds 413 immediately and the runtime tears the request down. The size cap itself is unchanged and still covered by its test. Co-Authored-By: Claude Opus 5 --- packages/core/src/server/routeHandler.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/core/src/server/routeHandler.ts b/packages/core/src/server/routeHandler.ts index 2b5e4ee..b2fa277 100644 --- a/packages/core/src/server/routeHandler.ts +++ b/packages/core/src/server/routeHandler.ts @@ -75,6 +75,14 @@ function jsonResponse(body: RpcResponse | RpcResponse[], status: number): Respon * enforced against the bytes themselves, not the header. * * Returns `null` when the body exceeds the cap. + * + * Over the cap we stop reading and release the lock WITHOUT calling + * `reader.cancel()`. Cancelling closes the stream while the runtime may still + * be enqueueing already-received chunks into it, which surfaces as + * `ERR_INVALID_STATE: ReadableStream is already closed` thrown inside undici — + * an unhandled rejection we cannot catch from here, and one that can take down + * a server process. Dropping the reader is enough: the caller responds 413 + * immediately and the runtime tears the request down. */ async function readLimited(req: Request, limit: number): Promise { const body = req.body; @@ -88,10 +96,7 @@ async function readLimited(req: Request, limit: number): Promise limit) { - await reader.cancel(); - return null; - } + if (total > limit) return null; chunks.push(value); } } finally {