Skip to content

fix(typescript-client): prevent handle -next suffix accumulation on repeated 409s - #3961

Merged
KyleAMathews merged 9 commits into
mainfrom
fix/handle-next-suffix-accumulation
Mar 5, 2026
Merged

KyleAMathews merged 9 commits into
mainfrom
fix/handle-next-suffix-accumulation

Conversation

@KyleAMathews

@KyleAMathews KyleAMathews commented Mar 5, 2026 •

Copy link
Copy Markdown
Contributor

Fix unbounded URL growth on 409 retries when proxy strips the handle header, which caused 414 URI Too Long errors and cascading 5xx failures.

Root Cause

When the server returns a 409 "must-refetch" response, the client needs a new shape handle. If the response includes the electric-handle header, the client uses it directly. If not (e.g., a proxy/CDN strips the header), the old code appended -next to the current handle as a cache buster. This violated two invariants:

  1. Handles are identities, not cache busters — concatenating -next conflated identity with URL uniqueness
  2. Retry URLs must be unique — handle-next was the same on every retry, only busting the cache once. And it grew unboundedly: handle-next-next-next-next...

Two patterns were observed in production (ELECTRIC-4GV, 8,764 events in 9 hours):

  • Pattern A: expired_handle=undefined-next-next-next... — handle was never initialized
  • Pattern B: handle=87840909-...-next-next-next... — valid handle that spiraled

Approach

Removed the -next suffix hack entirely. When a 409 lacks a handle header:

  1. Reset without fabricating a handle — #reset(undefined) via markMustRefetch, preserving the state machine's contract (per SPEC.md I10)
  2. Use a random cache-buster query param — the same CACHE_BUSTER_QUERY_PARAM mechanism already used by StaleRetryState for stale CDN responses
// Before: conflated handle with cache busting, grew unboundedly
const newShapeHandle =
  e.headers[SHAPE_HANDLE_HEADER] || `${this.#syncState.handle!}-next`

// After: proper separation of concerns
if (!newShapeHandle) {
  this.#refetchCacheBuster = `${Date.now()}-${Math.random()...}`
}
this.#reset(newShapeHandle || undefined)

The cache buster is consumed once in #constructUrl and cleared, ensuring each retry gets a unique URL without polluting the handle.

Key Invariants

  • Every retry URL must be unique — enforced by random cache-buster param (tested)
  • Handles are never fabricated — no -next, no undefined stringification
  • State machine contract preserved — markMustRefetch(handle?) produces InitialState per SPEC.md I10
  • Both code paths covered — #requestShape (line 890) and fetchSnapshot (line 1886)

Non-goals

  • Not changing the happy path (409 with handle header works as before)
  • Not adding retry limits — the existing fast-loop detector handles that

Verification

cd packages/typescript-client
pnpm exec vitest run --config vitest.unit.config.ts
# 295 tests pass (10 files)

Tests assert the real invariants:

  • All retry URLs are unique (no duplicates)
  • No URL contains -next or undefined in any parameter
  • Cache buster or expired_handle param present on every retry

Files Changed

File Change
packages/typescript-client/src/client.ts Replace -next hack with #refetchCacheBuster at both 409 paths + consumption in #constructUrl
packages/typescript-client/test/expired-shapes-cache.test.ts Rewrite regression tests to assert URL uniqueness invariant
AGENTS.md Add note to read SPEC.md before working on typescript-client
.changeset/fix-handle-next-accumulation.md Changeset for patch release

🤖 Generated with Claude Code

…epeated 409s

When a proxy strips the handle header from 409 responses, the client
falls back to appending "-next" to the current handle as a cache buster.
On repeated retries this compounds (handle-next-next-next-next...) until
the URL exceeds server URI limits (414), cascading into 5xx errors.

Strip any existing -next chain before appending a single -next suffix,
keeping the cache-busting behavior bounded.

Fixes ELECTRIC-4GV

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@pkg-pr-new

pkg-pr-new Bot commented Mar 5, 2026 •

Copy link
Copy Markdown

Open in StackBlitz

npm i https://pkg.pr.new/@electric-sql/react@3961
npm i https://pkg.pr.new/@electric-sql/client@3961
npm i https://pkg.pr.new/@electric-sql/y-electric@3961

commit: 7e7d88c

@codecov

codecov Bot commented Mar 5, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.78%. Comparing base (deb2a61) to head (992502d).
⚠️ Report is 1 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
packages/typescript-client/src/client.ts 80.00% 3 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3961   +/-   ##
=======================================
  Coverage   88.77%   88.78%           
=======================================
  Files          25       25           
  Lines        2415     2425   +10     
  Branches      604      612    +8     
=======================================
+ Hits         2144     2153    +9     
- Misses        269      270    +1     
  Partials        2        2           
Flag Coverage Δ
packages/experimental 87.73% <ø> (ø)
packages/react-hooks 86.48% <ø> (ø)
packages/start 82.83% <ø> (ø)
packages/typescript-client 93.99% <80.00%> (-0.03%) ⬇️
packages/y-electric 56.05% <ø> (ø)
typescript 88.78% <80.00%> (+<0.01%) ⬆️
unit-tests 88.78% <80.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@alco
alco requested a review from kevin-dp March 5, 2026 15:23
KyleAMathews and others added 8 commits March 5, 2026 08:23
- Simplify regex from /-next((-next)*)$/ to /(-next)+$/
- Add console.warn when 409 response lacks handle header
- Strengthen test assertion to check exactly 1 -next suffix

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Covers ELECTRIC-4GV Pattern A where the client never received an initial
handle. Verifies that "undefined" is never stringified into the handle
parameter and that -next does not accumulate.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The -next suffix approach conflated the handle (an identity) with cache
busting (a URL uniqueness concern). It also didn't produce unique URLs
across retries since "handle-next" was the same every time.

Now when a 409 response lacks a handle header, we:
1. Reset without fabricating a handle
2. Set a one-shot random cache-buster query param (same mechanism used
   by StaleRetryState) to ensure the retry URL is unique

Tests now assert the real invariant: all retry URLs must be unique, and
no URL should contain "-next" or "undefined" in any parameter.

Also adds note to AGENTS.md to always read SPEC.md before working on
the typescript client.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… path

Tests the state machine contract that markMustRefetch with no handle
produces InitialState with undefined handle, offset -1, and omits
the handle query param from URLs. Algebraic test covers all 7 states.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@KyleAMathews
KyleAMathews merged commit 0031e81 into main Mar 5, 2026
42 checks passed
@KyleAMathews
KyleAMathews deleted the fix/handle-next-suffix-accumulation branch March 5, 2026 19:59
@github-actions

github-actions Bot commented Mar 5, 2026

Copy link
Copy Markdown
Contributor

This PR has been released! 🚀

The following packages include changes from this PR:

  • @electric-sql/client@1.5.11

Thanks for contributing to Electric!

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.

2 participants