doc: document quic stopSending() and resetStream() - #64888
Open
theSnackOverflow wants to merge 2 commits into
Open
doc: document quic stopSending() and resetStream()#64888theSnackOverflow wants to merge 2 commits into
theSnackOverflow wants to merge 2 commits into
Conversation
`QuicStream` exposes `stopSending()` and `resetStream()`, but neither appeared in the QuicStream API reference. Both matter when half-closing a stream, which protocols such as WebTransport rely on. Document the two methods and list them in the "Aborting a stream" summary, which previously covered only `writer.fail()` and `stream.destroy()`. Unlike those, both send the given code as-is rather than deriving a wire code from an error. Fixes: nodejs#63680 Signed-off-by: Ji Hoon Kang <ivory.ma9ic@gmail.com>
Collaborator
|
Review requested:
|
The reference added earlier in this pull request described only the bidirectional happy path. Three behaviours were missing: * `resetStream()` discards any data still queued for sending. A reset stream is never acknowledged, so that queue can no longer drain. * `resetStream()` is a no-op once the stream has already been reset. * Neither method sends a frame on the unidirectional stream that lacks the side it aborts β `stopSending()` on a locally-initiated one, `resetStream()` on a remote-initiated one. Both fail silently there. The wording for stream directionality follows `stream.destroy()`, which already states that the readable side exists on bidirectional and remote-initiated unidirectional streams. Refs: nodejs#63680 Signed-off-by: Ji Hoon Kang <ivory.ma9ic@gmail.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
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.
Fixes: #63680
QuicStream.prototype.stopSending()andQuicStream.prototype.resetStream()are part of the public surface but were missing from the QuicStream reference.
The wording follows the JSDoc already on the implementation:
node/lib/internal/quic/quic.js
Lines 2411 to 2435 in 8a1ca0f
I also added both to the Aborting a stream section, which listed only
writer.fail()andstream.destroy(). The distinction seemed worth callingout: those two derive the wire code from an error (falling back to the
protocol's "internal error" code), while
stopSending()andresetStream()send the given
codeas-is.added:is set tov23.8.0, matching the rest of the file β both methods wereintroduced in 062ae6f, the same commit that created
doc/api/quic.md, andwere simply omitted from it. Happy to change this if a different version is
more accurate.
Verification
doc/api/quic.mdis inskip_apidoc_files(Makefile), so it is not part of theHTML/JSON doc build and
make doc-onlydoes not exercise it. Validation wastherefore limited to:
node tools/lint-md/lint-md.mjs doc/api/quic.mdβ cleanpython3 tools/test.py doctoolβ 3/3 passingstream.setPriority([options])β
#streamsetpriorityoptions)Note
This supersedes #63681, which covered the same issue but was closed without
landing.
Update
Added a second commit covering behaviour the first one missed:
resetStream()discards any data still queued for sending, and is a no-oponce the stream has already been reset.
it aborts β
stopSending()on a locally-initiated one,resetStream()on aremote-initiated one. Both fail silently there.
Verified against
Stream::DoStreamResetandStream::SendStopSendinginsrc/quic/streams.cc. The directionality wording followsstream.destroy(),which already frames the readable side as existing on bidirectional and
remote-initiated unidirectional streams.
I left the valid range of
codeundocumented on purpose: the binding reads itwith
Uint64Value()and discards the lossless flag, so out-of-range values aresilently truncated rather than rejected. Documenting a 62-bit limit would state
a contract the implementation does not currently enforce. Happy to open that
separately if it's worth tracking.