Skip to content

Fix SSE client cancellation#9503

Merged
michaelstaib merged 2 commits intomain-version-15from
mst/sse-cancellation
Apr 3, 2026
Merged

Fix SSE client cancellation#9503
michaelstaib merged 2 commits intomain-version-15from
mst/sse-cancellation

Conversation

@michaelstaib
Copy link
Copy Markdown
Member

No description provided.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to fix cancellation behavior for GraphQL over SSE result streaming by ensuring the cancellation token provided to ReadAsResultStreamAsync actually influences the underlying SSE read loop.

Changes:

  • Pass the ReadAsResultStreamAsync cancellation token into the SSE reader.
  • Link the request-level cancellation token with the enumerator cancellation token inside SseReader.
  • Update the SSE subscription cancellation test to pass the token directly to ReadAsResultStreamAsync.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
src/HotChocolate/AspNetCore/test/Transport.Http.Tests/GraphQLHttpClientTests.cs Updates the SSE subscription test to pass a token directly to ReadAsResultStreamAsync.
src/HotChocolate/AspNetCore/src/Transport.Http/Sse/SseReader.cs Adds a request-level cancellation token and links it with enumerator cancellation.
src/HotChocolate/AspNetCore/src/Transport.Http/GraphQLHttpResponse.cs For SSE responses, forwards the provided cancellation token into SseReader.
Comments suppressed due to low confidence (1)

src/HotChocolate/AspNetCore/test/Transport.Http.Tests/GraphQLHttpClientTests.cs:723

  • This test now passes the token to ReadAsResultStreamAsync but no longer uses WithCancellation. await foreach won’t pass that token to the enumerator’s GetAsyncEnumerator unless you use WithCancellation, and the SSE reader currently can also end the sequence without throwing on cancellation. As a result, the OperationCanceledException catch/assert here is brittle and may not trigger. Consider either reintroducing .WithCancellation(cts.Token) (to make the exception expectation deterministic) or changing the assertion to verify that enumeration stops promptly after cts.CancelAsync() without requiring an exception.
            await foreach (var result in subscriptionResponse.ReadAsResultStreamAsync(cts.Token))
            {
                result.MatchInlineSnapshot(
                    """
                    Data: {"onReview":{"stars":5}}
                    """);
                await cts.CancelAsync();
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 166 to 169
if (contentType?.MediaType.EqualsOrdinal(ContentType.EventStream) ?? false)
{
return new SseReader(_message);
return new SseReader(_message, cancellationToken);
}
Copy link

Copilot AI Apr 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ReadAsResultStreamAsync now forwards the provided cancellationToken into SseReader, but SseReader currently treats a canceled PipeReader read as a normal end of stream (yield break) rather than propagating an OperationCanceledException. That makes cancellation behavior inconsistent with the non-SSE paths (which throw when the token is canceled) and makes it hard for callers to distinguish cancellation vs completion. Consider updating the SSE reader to throw when cancellation is requested (e.g., when the read is canceled) so this token has the expected observable effect.

Copilot uses AI. Check for mistakes.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants