[release/6.0] HTTP/3: Support canceling requests that aren't reading a body #35823
Conversation
JamesNK
requested review from
Tratcher,
wtgodbe and
adityamandaleeka
and removed request for
Tratcher and
wtgodbe
| @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Connections.Features | |||
| public interface IProtocolErrorCodeFeature | |||
| { | |||
| /// <summary> | |||
| /// Gets or sets the error code. | |||
| /// Gets or sets the error code. The property returns -1 if the error code hasn't been set. | |||
There was a problem hiding this comment.
IMO IProtocolErrorCodeFeature.Error should have been nullable, but the API ship sailed in .NET 5.
QUIC and HTTP/3 errors are variable-length integers that are always positive so -1 can be used to represent unset.
| var errorCode = stream._errorCodeFeature.Error; | ||
| if (errorCode >= 0) | ||
| { | ||
| stream.Abort(new ConnectionAbortedException(CoreStrings.Http2StreamResetByClient), (Http3ErrorCode)errorCode); |
There was a problem hiding this comment.
This will be updated to call AbortCore when #35764 is merged.
src/Servers/Kestrel/Transport.Quic/test/QuicConnectionListenerTests.cs
Outdated
Show resolved
Hide resolved
src/Servers/Kestrel/Transport.Quic/src/Internal/QuicStreamContext.cs
Outdated
Show resolved
Hide resolved
| { | ||
| // An error code value other than -1 indicates a value was set and the request didn't gracefully complete. | ||
| var errorCode = stream._errorCodeFeature.Error; | ||
| if (errorCode >= 0) |
There was a problem hiding this comment.
What does an error code of 0 indicate?
There was a problem hiding this comment.
It means the stream was aborted with error code of 0. That doesn't have any meaning in HTTP/3, but it's a valid error code for QUIC.
| // TODO: Consider a better way to provide this notification. For perf we want to | ||
| // make the ConnectionClosed CTS pay-for-play, and change this event to use | ||
| // something that is more lightweight than a CTS. |
There was a problem hiding this comment.
Can this be created lazily from the RequestAborted property?
There was a problem hiding this comment.
I think we still want to stop and complete the output pipes in this situation. So, we always want this event raised, even if someone hasn't subscribed to RequestAborted.
JamesNK
force-pushed
the
jamesnk/http3-waitwritescomplete
branch
from
c6bf2e7
to
8b32f03
Compare
JamesNK
force-pushed
the
jamesnk/http3-waitwritescomplete
branch
from
8b32f03
to
1e942b2
Compare
|
Replaced with #36109 |
amcasey
added
area-networking
React to dotnet/runtime#58236.