Skip to content

fix(client): cancel subset snapshots with the stream signal - #4831

Open
alec-watts wants to merge 1 commit into
electric-sql:mainfrom
alec-watts:codex/snapshot-lifecycle-abort
Open

alec-watts wants to merge 1 commit into
electric-sql:mainfrom
alec-watts:codex/snapshot-lifecycle-abort

Conversation

@alec-watts

Copy link
Copy Markdown

Aborting ShapeStreamOptions.signal stops the live stream, but subset snapshot requests currently omit that signal from their fetch options. A pending fetchSnapshot() or requestSnapshot() can therefore keep downloading after the caller aborts the stream.

Pass the external lifecycle signal to both GET and POST snapshot requests. Use options.signal, not the internal live-request controller: acquiring the snapshot pause lock intentionally aborts the latter, and must not cancel the snapshot itself. Existing fetch/backoff cancellation handling performs the teardown; no new controller or retry mechanism is introduced.

Add regression coverage in the existing stream suite for both snapshot APIs and both HTTP methods, document the lifecycle invariant, and include a patch changeset. All four regression cases fail against unmodified main and pass with the fix.

Validation:

  • pnpm exec vitest run --config vitest.unit.config.ts — 456 tests pass.
  • pnpm exec tsc --noEmit — passes.
  • pnpm exec eslint src/client.ts test/stream.test.ts — passes.

This does not add per-snapshot cancellation independent of the stream, or change late-response quarantine behavior.

Why each file changes

File Reason
packages/typescript-client/src/client.ts Pass the external stream signal to GET and POST snapshot fetches.
packages/typescript-client/test/stream.test.ts Reproduce in-flight cancellation for both snapshot APIs and HTTP methods.
packages/typescript-client/SPEC.md Document the external-signal requirement and distinction from the pause controller.
.changeset/tidy-snapshots-abort.md Publish the client fix as a patch release.

This branch has not been deployed

No deployments
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