Commit 9f767cf
test(client): add fast-check model-based property tests and retry bound analysis (#4089)
## Summary
Fixes **twelve** bugs in ShapeStream's retry, error-handling, caching,
and subscription paths — all uncovered by fast-check property-based
testing (model-based + micro-target) and AST-based static analysis.
Every bug is a concrete end-user hazard: infinite retry loops, stale
data delivery, stack-frame leaks, cache-key collisions, dropped
notifications, lost snapshots, wrong-row corruption, and deadlocks.
## Root Cause
Every bug in this PR is an edge case in an error-handling or
reconciliation path that only manifests under a specific *sequence* of
events. Hand-written tests don't naturally explore adversarial sequences
like "409 with same handle, then 409 with no handle, then 200 with empty
body" or "addSnapshot(mark=1, xmax=5); removeSnapshot(1);
addSnapshot(mark=1, xmax=10); reject(xid=5)". These bugs survived
because they required rare permutations — a proxy stripping a header,
two schema columns differing only in underscore count, a subscriber
pushed over the must-refetch/up-to-date boundary at just the wrong
instant.
PBT shakes them out mechanically.
## Approach
Two complementary verification strategies that catch bugs without a
human having to imagine the failure mode:
### Model-based property testing (`test/model-based.test.ts`)
`fc.asyncModelRun` with 21 command types drives adversarial server
response sequences. A simple model tracks `{ consecutiveErrors,
terminated }`, and each command predicts the model's state change before
asserting the real ShapeStream matches.
**FetchGate pattern**: a controllable mock fetch that blocks each
request until the test provides a response — turn-based coordination
where the test controls *what* the server returns while ShapeStream
drives *when* it fetches.
**Global invariants after every command**:
- URL length stays bounded (catches suffix growth)
- `isUpToDate` / `lastSyncedAt` consistency
- After 409, retry URL differs from pre-409 URL (catches identity loops)
### Micro-target PBT (`test/pbt-micro.test.ts`, new)
Twelve narrow TARGETs, one per suspected bug site. Each TARGET's opening
comment documents the invariant under test, then runs `fc.assert` with
500 runs in CI (2000-run soak available via `PBT_MICRO_RUNS=2000`). Runs
against the dedicated `vitest.pbt.config.ts` (skips the real-Electric
`globalSetup`). `bin/pbt-soak.sh` wraps it in a fresh-seed loop for
overnight soaks.
### Static analysis via AST walking
Seven rule types that mechanically detect structural bug patterns:
| Rule | What it catches |
|------|----------------|
| `unbounded-retry-loop` | Recursive calls in catch blocks without
detectable bounds |
| `conditional-409-cache-buster` | 409 handlers where
`createCacheBuster()` is conditional or missing |
| `parked-tail-await` | `await this.#method(); return` patterns that
park stack frames |
| `error-path-publish` | `#publish`/`#onMessages` calls inside catch
blocks or error status handlers |
| `shared-instance-field` | Mutable fields written before async
boundaries and read by other methods |
| `ignored-response-transition` | Non-delegate states returning `{
action: 'ignored' }` |
| `protocol-literal-drift` | Near-miss string literals for Electric
protocol params |
Each rule was RED/GREEN verified.
## Bugs Fixed
### From model-based tests + static analysis
**1. Conditional cache buster on 409.** Same-handle (and no-handle) 409s
produced identical retry URLs, causing infinite CDN-cached retry loops.
`createCacheBuster()` is now unconditional on every 409 in both
`#requestShape` and `#fetchSnapshotWithRetry`.
**2. Parked stack frame in `#start` retry.** `await this.#start();
return` kept the caller's frame alive for the whole recursive chain.
Switched to `return this.#start()` so the frame is released via promise
chaining.
**3. Missing `EXPERIMENTAL_LIVE_SSE_QUERY_PARAM` in protocol params.**
`canonicalShapeKey` wasn't stripping the deprecated param, so clients
using it landed on a different cache key than clients on current
`live_sse`. Added to the strip list; static analysis now enforces that
all internal `*_QUERY_PARAM` exports live in the list.
**4. Publishing 409 body to subscribers.** The 409 handler was parsing
`e.json` and calling `#publish(messages409)` before retrying, delivering
stale rotation frames. Replaced with a synthetic `must-refetch` control
message so the Shape still resets its cache on 409 rotation, without
leaking the raw payload.
### From `test/pbt-micro.test.ts` (new micro-target PBTs)
**5. `canonicalShapeKey` collapsed duplicate custom params.** Used
`URLSearchParams.set()` in the copy loop, so `?tag=a&tag=b` became
`?tag=b`. Two genuinely distinct shapes shared a cache key, producing
cross-shape cache poisoning via `expiredShapesCache`/`upToDateTracker`.
Switched to `append()`.
**6. `Shape#process` clobbered `shouldNotify`.** Three sites assigned
`shouldNotify = this.#updateShapeStatus(...)` instead of
OR-accumulating. Any sequence where a change message followed a
status-change message silently dropped the change's notification.
Changed to OR-accumulate, and `must-refetch` now tracks `hadData` before
clearing so subscribers still see the reset.
**7. `SubsetParams` GET serialization dropped `limit=0` / `offset=0`.**
Falsy checks. Switched to explicit `!== undefined` guards. Limit-0 is a
valid "probe" shape request.
**8. `Shape#requestedSubSnapshots` non-canonical JSON dedup.** Keyed on
`bigintSafeStringify`, which preserves insertion order — so `{a:1, b:2}`
and `{b:2, a:1}` re-executed the same snapshot twice on rotation. Added
`canonicalBigintSafeStringify` to `helpers.ts` that recursively sorts
object keys.
**9. `snakeToCamel` collided multi-underscore columns.** A run of `n`
underscores collapsed to a single camelCase boundary, so `user_id` and
`user__id` (distinct columns) decoded to the same app key — **wrong-row
corruption at the ColumnMapper layer**. `snakeToCamel` now preserves
`(n-1)` literal underscores for a run of `n`, and `camelToSnake`'s
boundary regex was widened to `([a-z_])([A-Z])` so the round-trip is
injective.
**10. `Shape#reexecuteSnapshots` silently swallowed errors.** Caught and
discarded errors from `stream.requestSnapshot` on shape rotation, so
failed sub-snapshot re-execution was invisible. Errors are now collected
and the first is surfaced via `#error` + `#notify`, plus the pre-step
`#awaitUpToDate` throw is caught and surfaced the same way.
**11. `SnapshotTracker` stale reverse indexes on re-add.**
`xmaxSnapshots` and `snapshotsByDatabaseLsn` were populated on
`addSnapshot` but never cleaned up on `removeSnapshot`, and re-adding
with the same mark left stale entries pointing at the old `(xmax,
database_lsn)`. A later `shouldRejectMessage` eviction loop would walk
the stale index and wrongly delete the current snapshot — **duplicate
change messages would slip through the filter**. Tracked `databaseLsn`
on each entry and added `#detachFromReverseIndexes` that runs on both
add (before overwriting) and remove.
**12. `Shape#awaitUpToDate` deadlocked on terminally-errored stream.**
The helper polled `stream.isUpToDate` via `setInterval(check, 10)` but
never observed `stream.error`, so calling `requestSnapshot` after the
stream had terminally failed hung forever. The helper now checks
`#error` / `stream.error` up front, subscribes to the stream's
`onError`, and settles the internal promise via `reject` on any terminal
error path.
## Key Invariants
1. Every 409 handler unconditionally calls `createCacheBuster()` before
retrying (static analysis + PBT).
2. Every recursive call in a catch block has a detectable retry bound.
3. No `await this.#method(); return` in recursive methods.
4. No `#publish` or `#onMessages` calls in error handling paths.
5. All internal `*_QUERY_PARAM` constants appear in
`ELECTRIC_PROTOCOL_QUERY_PARAMS`.
6. Consecutive-error counter resets only on proven success (200+data,
200 up-to-date, 204).
7. `canonicalShapeKey` is stable under protocol-param noise and distinct
under origin/pathname/custom-param changes (PBT).
8. `Shape#process` notifies subscribers on every data change, even when
interleaved with status-change messages (PBT).
9. `snakeToCamel` ∘ `camelToSnake` is identity; distinct db columns
never collide on the same app key (PBT).
10. `SnapshotTracker` reverse indexes are consistent with
`activeSnapshots` after every operation (PBT with 500–2000 runs).
11. `Shape#awaitUpToDate` settles on any terminal outcome — up-to-date
OR error — in bounded time.
## Non-goals
- Refactoring `#start`/`#requestShape` into iterative loops (structural
change, separate PR).
- Fixing the `pgArrayParser` double-push bug (pre-existing, unreachable
for valid PostgreSQL arrays).
- Testing network-level failures or backoff timing (model tests focus on
response sequences).
- The wake-detection `queueMicrotask` timing issue (pre-existing,
requires deeper investigation).
- Log-mode and `subset__*` collision in `canonicalShapeKey` — TARGET 10
pins current behavior with deterministic assertions; fixing them is a
separate scope decision.
## Verification
```bash
cd packages/typescript-client
# Unit suite (no Electric server required)
pnpm vitest run --config vitest.unit.config.ts # 319 tests
# Micro-target PBT suite (12 TARGETs)
PBT_MICRO_RUNS=500 pnpm vitest run --config vitest.pbt.config.ts test/pbt-micro.test.ts # 44 tests
# Soak runner (fresh seed each iteration, default 1 hour budget)
bin/pbt-soak.sh
# Static analysis tests
pnpm vitest run --config vitest.unit.config.ts test/static-analysis.test.ts # 10 tests
# Typecheck + lint
pnpm exec tsc --noEmit && pnpm exec eslint src test
```
Verified: 319 unit tests, 44 PBT tests at 500 runs each (plus 2000-run
soak), 62 column-mapper/snapshot-tracker tests, typecheck clean, eslint
clean, 10 static-analysis rules clean.
## Files changed
| File | Change |
|------|--------|
| `src/client.ts` | Unconditional cache buster on 409; `return
this.#start()`; synthetic `must-refetch` on 409 instead of raw publish;
`EXPERIMENTAL_LIVE_SSE_QUERY_PARAM` in strip list; `canonicalShapeKey`
uses `append()`; `SubsetParams` GET uses `!== undefined` |
| `src/shape.ts` | `Shape#process` OR-accumulates `shouldNotify` +
tracks `hadData` on must-refetch; `#reexecuteSnapshots` surfaces errors
via `#error`+`#notify`; `#awaitUpToDate` settles on terminal
stream/error states; `requestSnapshot` dedup uses canonical stringify |
| `src/column-mapper.ts` | `snakeToCamel` preserves `(n-1)` underscores;
`camelToSnake` boundary regex widened to `([a-z_])([A-Z])` for injective
round-trip |
| `src/snapshot-tracker.ts` | Reverse-index cleanup via
`#detachFromReverseIndexes` on both add-before-overwrite and remove;
tracks `databaseLsn` per entry |
| `src/helpers.ts` | Added `canonicalBigintSafeStringify` (recursive
key-sorted stringify) |
| `src/constants.ts` | `EXPERIMENTAL_LIVE_SSE_QUERY_PARAM` in protocol
strip list |
| `test/model-based.test.ts` | 11 response factories + FetchGate + 21
commands + global invariants; new update/delete/mixed-batch 200
responses with `lsn`/`op_position`/`txids` headers |
| `test/pbt-micro.test.ts` | **New.** 12 micro-target PBTs (44
individual tests) |
| `test/static-analysis.test.ts` | Tests for all 7 AST-based rules
including protocol param completeness |
| `bin/lib/shape-stream-static-analysis.mjs` | AST analysis: unbounded
retry, 409 cache buster, tail-position await, error-path publish,
protocol literal drift, shared-instance-field,
ignored-response-transition |
| `bin/pbt-soak.sh` | **New.** Fresh-seed soak runner with configurable
budget |
| `vitest.pbt.config.ts` | **New.** Dedicated config for PBT suites —
skips Electric `globalSetup` |
| `vitest.unit.config.ts` | Added model-based test to include list |
| `SPEC.md` | Cross-references L6 `fetchSnapshotWithRetry` PBT from
unconditional-409-cache-buster invariant; updated loop-back site line
numbers |
| `.changeset/add-model-based-property-tests.md` | Updated to cover all
12 bugs in one entry |
| `package.json` | Added `fast-check` devDependency |
---
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>1 parent 690e25a commit 9f767cf
File tree
20 files changed
+4081
-345
lines changed- .changeset
- packages/typescript-client
- bin
- lib
- src
- test
- fixtures/static-analysis
20 files changed
+4081
-345
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
28 | 29 | | |
29 | 30 | | |
30 | 31 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
292 | 292 | | |
293 | 293 | | |
294 | 294 | | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 334 | + | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
295 | 341 | | |
296 | 342 | | |
297 | 343 | | |
| |||
362 | 408 | | |
363 | 409 | | |
364 | 410 | | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
365 | 425 | | |
366 | 426 | | |
367 | 427 | | |
368 | 428 | | |
369 | 429 | | |
370 | 430 | | |
371 | | - | |
372 | | - | |
373 | | - | |
374 | | - | |
375 | | - | |
376 | | - | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
377 | 437 | | |
378 | 438 | | |
379 | 439 | | |
380 | | - | |
381 | | - | |
382 | | - | |
383 | | - | |
384 | | - | |
385 | | - | |
386 | | - | |
387 | | - | |
388 | | - | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
389 | 449 | | |
390 | 450 | | |
391 | 451 | | |
| |||
0 commit comments