Draft-20 backlog: session-layer request-stream rules - #116
Merged
Merged
Conversation
…R that answers no REQUEST_UPDATE (§10.9) On a request stream, a REQUEST_OK or REQUEST_ERROR can only answer a REQUEST_UPDATE this side sent (§10.9). Otherwise it answers nothing, and the session is now closed with PROTOCOL_VIOLATION: - on a request this side sent, it is a second response (§5.1, §5.2, §6.2). RequestBroker already closed on this for SUBSCRIBE and PUBLISH; it now does for FETCH and the namespace requests too; - on a stream this side answered, only the requester sends REQUEST_UPDATE, bar a PUBLISH's subscriber (§10.9). The draft names no rule for a response from the requester there. Closing is the maintainer's decision, and reverses cc1a999's TestResponderStreamResponseKeepsSession, which passed such a response to Serve's callback. The same rule covers every broker, accept-side ones included, and the relay's readRequestStream: the relay sends no REQUEST_UPDATE on the SUBSCRIBE, FETCH, namespace and forwarded-PUBLISH streams it reads there. That supersedes the forwarded-PUBLISH-only check in readSubscribeUpdates and its `forwarded` parameter. NewRequestBroker's doc now states that a request's own response must be read before a broker attaches to its stream. Tests, each verified red first: TestResponderStreamStrayResponseCloses (an accepted SUBSCRIBE and PUBLISH), TestRequesterStrayResponseCloses (this side's FETCH and SUBSCRIBE_NAMESPACE), and TestRelay_StrayResponseCloses (SUBSCRIBE, FETCH, PUBLISH, PUBLISH_NAMESPACE, SUBSCRIBE_NAMESPACE and SUBSCRIBE_TRACKS, each with both messages). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… (§10.4)
"A GOAWAY MAY also be sent on a request stream to initiate migration of that
individual request" (§10.4), before the request's response too. The
requester failed the request instead ("unexpected GOAWAY in … response").
awaitRequestResponse now:
- checks such a GOAWAY as any on the stream is. A New Session URI sent to
the server, or a second GOAWAY before the response, closes the session
with PROTOCOL_VIOLATION;
- keeps reading for the response;
- puts the GOAWAY back at the front of the stream (replayStream), so the
stream's next reader, a broker's Serve or a direct message.Parse, gets it
as its first follow-up, just as one sent after the response. A later
second GOAWAY is still caught there.
Not for PUBLISH_NAMESPACE, SUBSCRIBE_NAMESPACE or SUBSCRIBE_TRACKS, whose
response MUST be "the first message" on the stream (§6.1, §6.2). Those keep
their existing handling: the request fails, and for the two subscription
requests the session also closes.
No new API; a handle's Stream is the wrapper after an early GOAWAY.
Tests, written first and seen red: TestEarlyRequestGoawayThenResponse
(SUBSCRIBE through its broker, FETCH by direct read),
...ThenRejection, ...ViolationCloses (two before the response, one before
and one after, a URI to the server) and ...OnPublishNamespace.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
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
1 participant
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.
The session-layer items of the draft-20 backlog. There is one commit per item, and each was reviewed by moqt-reviewer. Every test was seen failing before its fix.
Commits
8f8cbecClose the session on a REQUEST_OK or REQUEST_ERROR that answers no REQUEST_UPDATE (§10.9).RequestBroker, including accept-side ones, and to the relay'sreadRequestStream. That supersedes the forwarded-PUBLISH-only check.NewRequestBroker's doc now says a request's own response must be read before a broker attaches.6fdf6bcKeep awaiting a request's response past an early GOAWAY (§10.4).Decisions and interpretations
Verification
go test ./...andgolangci-lint runpass.go test -racepasses for./pkg/moqt/session/...and./pkg/relay/....make bench-quickshowsControlRoundTripallocs/op unchanged.🤖 Generated with Claude Code