http3: reset stream when a handler panics

[WeidiDeng]

When debugging an issue for caddy, I found http3 server handler will not correctly propagate stream error. Code example here.

In this case Content-Length is set, http3 may use that info to infer the stream doesn't terminate properly. But in a streaming environment if there is an error upstream, client should be able to know there is an error at a stream level just like http2 regardless of the presence of Content-Length.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (771d136) 83.77% compared to head (f8f65c3) 83.74%.
Report is 14 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4181      +/-   ##
==========================================
- Coverage   83.77%   83.74%   -0.04%     
==========================================
  Files         150      150              
  Lines       15425    15475      +50     
==========================================
+ Hits        12922    12958      +36     
- Misses       2006     2019      +13     
- Partials      497      498       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[marten-seemann]

If the panic happens before we've written the HTTP header, should we just write the 500 and close the stream?

[WeidiDeng]

I don't think status 500 is necessary in this case. Related pr and issue.

[marten-seemann]

What does HTTP/2 do in both these cases?

[WeidiDeng]

HTTP/2 doesn't differentiate between these two cases. It will reset the stream if there is a panic.

[WeidiDeng]

Per http.Handler documentation:

If ServeHTTP panics, the server (the caller of ServeHTTP) assumes that the effect of the panic was isolated to the active request. It recovers the panic, logs a stack trace to the server error log, and either closes the network connection or sends an HTTP/2 RST_STREAM, depending on the HTTP protocol. To abort a handler so the client sees an interrupted response but the server doesn't log an error, panic with the value ErrAbortHandler.

There is no mention of interpreting the panic and handling it the other way. And it's actually doing that.

[marten-seemann]

Thank you @WeidiDeng, and sorry for the delay in reviewing this PR! This will be part of the next quic-go release.