feat: typed oversize-frame handling — client pre-send guard, outbound size warning (ADR-0018)#33
Merged
Merged
Conversation
… size warning (ADR-0018) Part of #28 (part b; column projection, part a, stays open). An oversize client mutation was dropped silently server-side (ADR-0012's maxFrameBytes guard), surfacing only as a confirmation timeout — and in production Cloudflare's ~1 MiB edge cap on inbound WS messages means the frame may never reach the DO at all, so a server-side rejection cannot be the surface. Outbound had no size awareness whatsoever. - Client: new `maxFrameBytes` transport option (default 1 MiB, aligned with the server and the edge cap). An oversize mut/call rejects locally and immediately with MutationRejectedError code "FRAME_TOO_LARGE" — typed, prompt optimistic rollback, no timeout. Encoded once; bytes reused for the send. String (JSON debug codec) frames are measured in UTF-8 bytes, not UTF-16 code units (codex review). - Server: the ADR-0012 silent drop stays as defense in depth — recovering the txId would mean decoding the very payload the guard exists not to decode; the comment now says so. - Outbound: new `warnOutboundFrameBytes` knob (default 1 MiB, null disables) — console.warn with frame type, size, and collection when an encoded outbound frame exceeds it. Observability only; the frame is still sent whole (column projection, #28a, is the real fix). - A synchronous send() throw now cleans up the pending-receipt waiter and timer before rejecting (codex review). Tests: tests/frame-limits.test.ts pins the typed rejection (raw transport + collection-level rollback), UTF-8 measurement, the outbound warning, and the still-true large-row / read-only / tiny-collection behaviors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…es option and warnOutboundFrameBytes tunable Maintainer review of #33: the 1 MiB limit is Cloudflare's inbound WS edge cap — an infrastructure fact, not an application preference. Both wire endpoints ship in this package and exactly one infra is supported, so facts don't get knobs: a client option could only lower the limit pointlessly or raise it into the edge cap's lie. - Client: `maxFrameBytes` TransportOptions option removed; the pre-send guard now checks against a module constant MAX_FRAME_BYTES (1_048_576). The FRAME_TOO_LARGE typed rejection is unchanged; the message cites the edge cap, not an option name. - Server: `warnOutboundFrameBytes: number | null` protected tunable (and its null-disable branch and sync-do.ts redeclare) removed; the outbound warn fires at a fixed WARN_OUTBOUND_FRAME_BYTES (1 MiB) constant and now points at the real fix (column projection, issue #28). Inbound = enforced constant; outbound = deliberately unenforced, warn as the one production breadcrumb. - ADR-0018 gains D0 stating the opinion and recording the rejected configurable-limits alternative; CHANGELOG no longer names options. - Tests: knob usages removed (UTF-8 test now crosses the real 1 MiB cap); added the sync-send-failure cleanup pin (a socket that throws on send surfaces that error immediately with a clean waiter, never a timeout). `!`: removes the `maxFrameBytes` transport option added earlier on this unreleased branch — no released API is affected. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Owner
Author
|
Applied the review verdict in 8a38f67: the frame limits are infrastructure facts (Cloudflare's ~1 MiB inbound WS edge cap), not application preferences — we ship both endpoints and support exactly one infra, so facts don't get knobs.
Full suite: 47 files / 227 tests green; typecheck clean. No new public API surface remains. 🤖 Generated with Claude Code |
… to error-paths, drop duplicate tiny-collection pins Final tidy for #33. tests/frame-limits.test.ts is now ADR-0018 frame limits only: - The "no mutation handler" client-altitude test moves to tests/error-paths.test.ts, beside its wire-level sibling ("unknown mutation collection → rejected"). Same why, kept intact: the wire pin proves the server answers /no mutation handler/; the moved pin proves that rejection surfaces through a real @tanstack/db collection as MutationRejectedError with prompt optimistic rollback. Room name unchanged (fl-readonly). - The "tiny collections" pair is deleted as confirmed duplicates: zero-row snapshot is already pinned at wire level (sync-read: "empty collection emits only snap-end") and client level (do-collection: "readies an empty collection without any write") — a third pin cannot fail uniquely; single-row-frequent-updates is a weaker restatement of coalesce.test.ts's burst pin, and its original why (migration due-diligence) was already banked. Full suite: 47 files / 225 tests (227 − 2 deleted; the moved one keeps counting); typecheck clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #28 — this addresses part (b), typed oversize-frame handling. Part (a), column projection / changed-columns-only patches, is deliberately not in scope and the issue stays open for it.
Problem
An oversize client mutation frame was dropped silently server-side (ADR-0012's
maxFrameBytesguard:console.error, no reply). The client saw only a generic confirmation timeout, with rollback-by-timeout seconds later. Worse, the server guard cannot be the fix: in production Cloudflare caps inbound WebSocket messages at ~1 MiB at the edge, so an oversize client frame may never reach the DO at all. Outbound (not capped the same way) had no size awareness whatsoever.Design (ADR-0018)
maxFrameBytestransport option (default 1 MiB, aligned with the server's ADR-0012 default and the edge cap). Before sending amut/call, the transport encodes once and checks the size; on breach it rejects locally and immediately with the existing typed surface —MutationRejectedError, code"FRAME_TOO_LARGE"— so the optimistic overlay rolls back promptly instead of waiting out a timeout. String (JSON debug codec) frames are measured in UTF-8 bytes, not UTF-16 code units.rejectedwould require thetxId, and recovering it means decoding the very payload ADR-0012's guard exists not to decode. The code comment now records why the drop is silent.warnOutboundFrameBytesknob (default 1 MiB,nulldisables): an encoded outbound frame over the threshold logs aconsole.warnwith frame type, byte size, and collection — and is still sent whole. Observability, not enforcement; the real fix for hydrated full-row re-sends is column projection (#28a).Tests
tests/frame-limits.test.ts(red-first: the flipped assertions timed out under the old silent-drop path):mut→ prompt typedFRAME_TOO_LARGE, row absent server-side; collection-level insert → prompt optimistic rollbackFull suite: 47 files / 226 tests green; typecheck clean.
Adversarial review (codex gpt-5.5)
Three findings: (1) UTF-16 vs UTF-8 undercount for string-codec frames — fixed client-side; the server's pre-existing
message.lengthmeasure (ADR-0012 D2) is flagged in the ADR rather than silently changed. (2)sub/fetchframes with pathological predicates unguarded — rebutted as out of scope in the ADR (pre-existing exposure, no row data, no typed rejection surface to reuse). (3) stale pending-tx waiter when a socket refuses a send synchronously — fixed.🤖 Generated with Claude Code