Repository navigation
Production-ready subscriptions (SSE + graphql-ws) and a single UTF-8 envelope parser - #115
Merged
joadan merged 6 commits intoOct 16, 2026
Conversation
Subscriptions: - SSE and WebSocket transports with per-subscriber buffers and overflow policy (Fault, DropOldest, KeepLatest); each Subscribe() gets its own connection. - Server 'complete' ends the typed observable; overflow in one subscriber no longer starves others. - StartWithErrorsAsync(): a next event with GraphQL errors no longer terminates the subscription; data and errors are delivered together. StartAsync() unchanged. - Bounded WebSocket reconnect (SubscriptionRetryAttempts, default 5) with exponential backoff and jitter; every attempt opens a fresh socket; budget reset on connection_ack; fails once with inner cause. A 30 s no-message watchdog detects silent connections. - WebSocket fatal close codes follow the graphql-ws reference client; fatal closes cancel library reconnection. - Unknown WS message types are ignored; SSE idle timeout (SubscriptionIdleTimeout). Client: - Responses parsed from UTF-8 bytes (GraphEnvelopeReader): no JsonDocument, no UTF-16 round trip, only the requested root field is materialized. - One shared frozen JsonSerializerOptions per process. - __AdditionalProperties is null until first used. - Empty HTTP 200 body yields Data = default instead of throwing. Behaviour changes are listed in CHANGELOG.md under [Unreleased]. Verification: 0 build warnings, 289/289 tests, generated test clients match the generator.
Restore the escaped errors/next code spans, split two fused bullets, and add the missing trailing newline to GraphClientOptions.cs.
GraphEnvelopeReader is now the single GraphQL envelope parser: - ProcessResponseFull(string) transcodes to UTF-8 and uses the reader (empty string now returns Data = default instead of throwing). - ProcessResponseFull(JsonElement) hands its raw UTF-8 text to the reader. - GraphClient.ExecuteAsync (ExecuteDocumentAsync) uses new ReadDocument, which also decides whether a non-2xx body is a GraphQL result. Tests pin that all overloads agree, the empty-string behaviour, and the text-query transport rules. CHANGELOG documents the behaviour change; CLAUDE.md names GraphEnvelopeReader as the single parser.
… load - Buffer overflow tests wait until event 1 is delivered (processor stalled) instead of a fixed 50 ms delay; stall raised to 1 s. - Idle-timeout tests use a wider per-gap margin (10-15x) while the total duration still exceeds the timeout (re-arm still required; mutation-checked). - Dispose test waits for the event instead of a fixed delay.
Contributor
Author
|
@joadan could you take this one? I can't assign or request reviewers from a fork (no triage rights), so a mention will have to do. Happy to split it into smaller PRs if that makes review easier. |
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.
Summary
Makes the subscription transports (SSE and graphql-ws) production-ready and moves every response path onto a single, allocation-light envelope parser.
No public signature is changed or removed. Several behaviour changes are listed below; please read those first. Full details are in the new
CHANGELOG.md([Unreleased]).Behaviour changes (please review)
GraphClient.SerializerOptionsis a shared, frozen instance: mutating it after construction throwsInvalidOperationException.__AdditionalPropertiesis null until first used (no eager dictionary per entity).StartAsyncno longer awaits the handshake. Connection/ack failures and per-message parse errors surface asOnErroron the stream; unknown message types are ignored.ProcessResponseFull(string)/ProcessResponse(string)returnsData = defaultwith no errors instead of throwingJsonException.ProcessResponseFull(JsonElement)with a non-object root throwsJsonExceptioninstead ofInvalidOperationException.4408is now retryable). Reconnect is owned by the subscription, with a bounded retry budget (SubscriptionRetryAttempts, default 5), exponential backoff and jitter.Added
GraphSubscriptionExecute.StartWithErrorsAsync():IObservable<GraphResult<T>>, so field errors no longer end the subscription.GraphClientOptions:SubscriptionMaxBufferedEvents,SubscriptionOverflowPolicy(Fault/DropOldest/KeepLatest),SubscriptionIdleTimeout(SSE),SubscriptionRetryAttempts(WS).QueryExecutor.ProcessResponse(JsonElement, ...)andProcessResponse(byte[], ...)overloads.Fixed
completeframe now completes the typed subscription observable.Faultoverflow in one subscriber no longer starves the other subscribers.SSEClientrace betweenDisposeand connect (possible NRE / spurious failure).WSConnectionInitPayloaddefault returns a completed task instead ofnull.Single envelope parser:
GraphEnvelopeReaderAll response paths now parse
data/errors/extensionswith one internalUtf8JsonReader-based reader (Common/GraphEnvelopeReader): typed queries,GraphClient.ExecuteAsync(GraphQL text, including the non-2xx "is this a GraphQL body?" decision), everyProcessResponse/ProcessResponseFulloverload, and subscription payloads.CLAUDE.mdsaidQueryExecutor.ProcessResponseFullis "the single place both go through". That rule is kept but now namesGraphEnvelopeReader, since that is where the single path lives after this change. @joadan, you wrote that file, so please check the wording.EnvelopeAndResponseTests.AllOverloads_ProduceTheSameResultchecks that the overloads cannot drift apart again.Benchmark (BenchmarkDotNet 0.15.8, .NET 10.0.402, in-process, comparing the previous
JsonDocumentpath with the new reader; results checked equal):The Large timing is noisy (SD 1.4 ms); the allocation drop is the robust result. Not measured: copying the HTTP body into the buffer.
Testing
dotnet restore Linq2GraphQL.CI.slnf --locked-mode→build --no-restore(0 warnings, 0 errors) →test --no-build: 446/446 passedmcr.microsoft.com/dotnet/sdk:10.0(a clean clone, same steps as CI's ubuntu job)../scripts/regenerate-test-clients.ps1 -Check: checked-in test clients match the generator.Notes
CHANGELOG.mdis new in this repo (Keep a Changelog format). Happy to move the notes elsewhere if you prefer.