Add reset stream API - #104
Conversation
|
Related apple/swift-nio-http3#14 |
| /// | ||
| /// - Parameter code: The `RST_STREAM` error code to send. | ||
| public consuming func reset(code: HTTP2ErrorCode) { | ||
| // The `HTTP2FramePayloadToHTTPServerCodec` on the stream channel translates this event into an `RST_STREAM` |
There was a problem hiding this comment.
nit:
| // The `HTTP2FramePayloadToHTTPServerCodec` on the stream channel translates this event into an `RST_STREAM` | |
| // The `HTTP2FramePayloadToHTTPServerCodec` on the stream channel translates this event into a `RST_STREAM` |
| /// Stream resets only exists over HTTP/2 and HTTP/3. The only abrupt tear-down mechanism available | ||
| /// for HTTP/1.1 is closing the connection. | ||
| @nonexhaustive | ||
| public enum StreamReset: ~Copyable { |
There was a problem hiding this comment.
I am not very happy that this surfaces different APIs for H/2 and H/3. In the end both are just wrappers around Int which is treated as an application error code right? Do we need to surface this difference really?
There was a problem hiding this comment.
There were two reasons why I decided to go this way, but basically it boils down to request handlers potentially wanting to do different things depending on the HTTP version they're running on (to fail with different error codes for example). The first reason is because the errors that can be surfaced aren't the same for H2 and H3; I considered having a set of common errors that could be mapped across each version, but not many of them actually overlap, and having some "raw" code where most of the errors would have to live felt weird.
The second and perhaps more importantly is that H1 does not support resets at all, so handlers may want to do close the connection instead or do something different altogether.
| /// Sends a `RST_STREAM` frame for this stream with the provided error code. | ||
| /// | ||
| /// - Parameter code: The `RST_STREAM` error code to send. | ||
| public consuming func reset(code: HTTP2ErrorCode) { |
There was a problem hiding this comment.
So this method executes I/O and it must be async and throws.
| /// | ||
| /// - Parameter body: A closure that is provided a ``NIOHTTPServer/StreamReset`` instance from which the request | ||
| /// stream can be reset with a transport-specific error code. | ||
| public consuming func reset(_ body: (consuming NIOHTTPServer.StreamReset) throws -> Void) throws { |
There was a problem hiding this comment.
I like that this is a method on the response sender but I am not sure we need the closure yet. Wouldn't it suffice if we just made this take an Int as the application error code? It probably should also consume the reader otherwise the reader is left in a bad state.
There was a problem hiding this comment.
We could potentially return the StreamReset instead of having the closure, yeah.
As to consuming the reader, I played with that, but for H3 where only the sending direction is closed, we may still have bytes to read.
Until now, a request handler had no way to abort the stream carrying a request.
This PR adds a reset operation to the response side:
ResponseSender.reset(_:): abort before any response head is sent.Writer.reset(_:): abort a response that has already started.Because a reset is inherently protocol-specific, reset hands the handler a protocol-discriminated
NIOHTTPServer.StreamResetto switch over:.http2→ sends an HTTP/2 RST_STREAM with a chosenHTTP2ErrorCode..http3→ sends an HTTP/3 RESET_STREAM with a chosen QUIC application error code..unavailable→ HTTP/1.1, which has no per-stream reset.resetconsumes the sender/writer, so users are prevented from writing anything further once the stream has been reset.An example scenario using
CONNECT: