Skip to content

feat: typed oversize-frame handling — client pre-send guard, outbound size warning (ADR-0018)#33

Merged
grrowl merged 3 commits into
mainfrom
fix/oversize-frames
Jul 22, 2026
Merged

feat: typed oversize-frame handling — client pre-send guard, outbound size warning (ADR-0018)#33
grrowl merged 3 commits into
mainfrom
fix/oversize-frames

Conversation

@grrowl

@grrowl grrowl commented Jul 22, 2026

Copy link
Copy Markdown
Owner

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 maxFrameBytes guard: 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)

  • Client pre-send guard (the reliable half). New maxFrameBytes transport option (default 1 MiB, aligned with the server's ADR-0012 default and the edge cap). Before sending a mut/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.
  • Server drop stays a drop (defense in depth). Sending a typed rejected would require the txId, 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.
  • Outbound is warn-only. New warnOutboundFrameBytes knob (default 1 MiB, null disables): an encoded outbound frame over the threshold logs a console.warn with 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):

  • oversize mut → prompt typed FRAME_TOO_LARGE, row absent server-side; collection-level insert → prompt optimistic rollback
  • UTF-8 vs UTF-16 measurement for the string codec
  • outbound warning fires for a >1 MiB delta (which still arrives whole) and stays silent for small frames
  • still-true pins: ~1.5 MB rows sync server→client whole (snapshot + live delta); no-mutation-handler ops reject with rollback; zero-row/one-row snapshots

Full 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.length measure (ADR-0012 D2) is flagged in the ADR rather than silently changed. (2) sub/fetch frames 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

grrowl and others added 2 commits July 22, 2026 14:48
… 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>
@grrowl

grrowl commented Jul 22, 2026

Copy link
Copy Markdown
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.

  • Removed the client maxFrameBytes transport option → module constant MAX_FRAME_BYTES = 1_048_576. The pre-send guard and typed FRAME_TOO_LARGE rejection are unchanged; the error message now cites the edge cap rather than an option name. A knob here could only lower the limit pointlessly or raise it into the edge cap's lie.
  • Removed the warnOutboundFrameBytes: number | null tunable (field, null-disable branch, sync-do redeclare) → fixed WARN_OUTBOUND_FRAME_BYTES = 1_048_576. The warn now points at the real fix (column projection, Full-row delta hydration re-broadcasts large unchanged columns on every update; no outbound size guard #28). Stated opinion: inbound = enforced constant; outbound = deliberately unenforced (breaking a correct broadcast to save bandwidth inverts the failure hierarchy), with this warn as the only production breadcrumb.
  • ADR-0018 gains D0 stating the opinion and recording the rejected configurable-limits alternative; CHANGELOG no longer names any option.
  • Tests: knob usages removed (the UTF-8 measurement test now crosses the real 1 MiB cap); no behavioral pin changed meaning. One pin added: a synchronous socket send() failure surfaces immediately with a clean waiter, never a confirmation timeout.

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>
@grrowl
grrowl merged commit b89a8c5 into main Jul 22, 2026
1 check passed
@grrowl
grrowl deleted the fix/oversize-frames branch July 22, 2026 05:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant