Repository navigation
fix(client): cancel subset snapshots with the stream signal - #4831
Open
alec-watts wants to merge 1 commit into
Open
alec-watts wants to merge 1 commit into
alec-watts wants to merge 1 commit into
Conversation
This branch has not been deployed
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.
Aborting
ShapeStreamOptions.signalstops the live stream, but subset snapshot requests currently omit that signal from their fetch options. A pendingfetchSnapshot()orrequestSnapshot()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
packages/typescript-client/src/client.tspackages/typescript-client/test/stream.test.tspackages/typescript-client/SPEC.md.changeset/tidy-snapshots-abort.md