[H/3] Fix code passed into QuicConnection.CloseAsync and QuicStream.Abort#55282
[H/3] Fix code passed into QuicConnection.CloseAsync and QuicStream.Abort#55282amcasey merged 11 commits intodotnet:mainfrom
Conversation
amcasey
left a comment
There was a problem hiding this comment.
I think it would be nice to flesh out the test a little, but I'm okay merging as-is if this unblocks you.
src/Servers/Kestrel/Transport.Quic/src/Internal/QuicConnectionContext.cs
Outdated
Show resolved
Hide resolved
src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs
Outdated
Show resolved
Hide resolved
|
My new check is failing CI. The test fails for me locally, but not until after the log message check succeeds, so I'm not sure why that's happening. I'll investigate. Edit: it was flaky before my change. |
|
The windows failure is known CI flakiness |
|
Okay, looking more closely, the failure I'm seeing here is the same one I'm seeing locally and isn't related to the logging check. I think the test is flaky in that it won't reliably cause the connection to be closed on the server side. |
src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs
Outdated
Show resolved
Hide resolved
src/Servers/Kestrel/Transport.Quic/src/Internal/QuicStreamContext.cs
Outdated
Show resolved
Hide resolved
I worked backwards from calls to `QuicConnection.CloseAsync` and `QuicStream.Abort`. TODO: Update tests
|
I believe the PR is ready to go. |
src/Servers/Kestrel/test/Interop.FunctionalTests/Http3/Http3RequestTests.cs
Outdated
Show resolved
Hide resolved
|
Ah, it was auto-merge from the previous design and my sign-off counts because it's technically Mana's PR. Well, shout if you don't like the exceptions @halter73. |
Fixes the code passed into
QuicConnection.CloseAsyncandQuicStream.Abortwhich was by default -1. Quic supports only error codes up to 62 bits non-negative integer values: https://www.rfc-editor.org/rfc/rfc9000.html#integer-encoding.Also added test which crashes the process (if running with DEBUG build of MsQuic) before this change and runs to completion afterwards.
Let me know if you want me to shuffle the code around, I'm not much familiar with the structure of your code base.
cc @amcasey
Fixes #55196
Also this will be followed up in S.N.Quic with some checks (dotnet/runtime#101385).