HttpContext.Abort results in different error code for UseKestrel vs UseHttpSys

[stephentoub]

When the server aborts an HTTP/2 request, a RST_STREAM frame is sent. If UseKestrel is used, this frame contains an INTERNAL_ERROR error code. If UseHttpSys is used, this frame contains a CANCEL error code. CANCEL seems more correct to me, but regardless, seems we should be consistent.

cc: @halter73

[analogrelay]

CANCEL seems more correct to me, but regardless, seems we should be consistent.

Consistency between HttpSysServer and Kestrel is not necessarily a requirement for us, servers differ in behavior all the time. Having said that, it sounds like CANCEL is more correct. Technically a client could be relying on the old behavior and would be broken by using CANCEL, but we think that's a pretty rare case.

Let's change to cancel in 3.0. We can quirk it in a patch if necessary.

[Tratcher]

Kestrel has several abort-ish code paths and does not currently distinguish the error code for them.

• HttpContext.Abort
• App throwing an exception
• Others?

[halter73]

Sure, but it cannot be too difficult to distinguish user-initiated aborts. We were thinking about having an HTTP/2 specific abort API that let you specify the error code anyway for gRPC.

[halter73]

I think I see what you mean though. It would be interesting to see if Http.Sys uses the CANCEL error code for the other cases too, like the app throwing an exception. In that case, I hope even Http.Sys uses the INTERNAL_ERROR error code.

[Tratcher]

Native Http.Sys doesn't expose any granular API there, I expect the error code is always cancel.

Kestrel already has the Reset API that lets you specify the error code.

[stephentoub]

It would be interesting to see if Http.Sys uses the CANCEL error code for the other cases too, like the app throwing an exception

Http.Sys uses CANCEL for both HttpContext.Abort and an exception getting thrown.

[Tratcher]

Yeah, both call HttpCancelHttpRequest. https://msdn.microsoft.com/en-us/windows/desktop/aa814335

[analogrelay]

What seems most appropriate here is to use CANCEL for HttpContext.Abort and INTERNAL_ERROR for app exceptions. Consistency doesn't feel like a significant motivator here to me, and if anything indicates we should be pushing for more control over this in Http.Sys (which I believe we are already working on since gRPC uses the RST_STREAM error code as part of the protocol)

[analogrelay]

The 3.0 door has closed. We still believe this would be a valuable change though, so Backlogging.