You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The net/http.conn.serve method is equipped to handle a special case for when panic is called with http.ErrAbortHandler by downstream http.Handler implementations where it silently recovers and prevents the program from crashing.
When chaining http.Handler implementations, if an http.Handler uses golang.org/x/sync/singleflight.Group, and an http.Handler downstream of that panics with http.ErrAbortHandler, the singleflight package recovers from the panic first, wraps the panic value in its own private error type, and re-panics with its private type. When the new panic gets to net/http.conn.serve, a direct comparison is made to see if err != ErrAbortHandler. Since technically err is not ErrAbortHandler, just another error type that wraps ErrAbortHandler, net/http.conn.serve does not handle the special case and allows the program to crash.
Example where net/http.conn.serve handles the panic and the program does not crash:
When using golang.org/x/sync/singleflight.Group in a chain of http.Handler implementations where a panic(http.ErrAbortHandler is called downstream, net/http.conn.serve is able to unwrap errors to determine if the error is actually http.ErrAbortHandler and handle the panic without allowing the program to crash.
What did you see instead?
The program crashes with the message panic: net/http: abort Handler.
The text was updated successfully, but these errors were encountered:
Since technically err is not ErrAbortHandler, just another error type that wraps ErrAbortHandler, net/http.conn.serve does not handle the special case and allows the program to crash.
net/http swallows all panics from handlers. ErrAbortHandler just controls whether it logs an error or not.If the program is crashing, something else is going on.
The net/http change in https://go.dev/cl/526418 seems reasonable to me in isolation--we're checking for ErrAbortHandler, I don't see a good reason not to check for errors which wrap ErrAbortHandler.
singleflight's abstraction is leaky for panics: If the function passed to singleflight.Group.Do panics, then Do propagates the panic, but it hides the original panic value in a type which adds a stack trace. There is no way for the caller to recover the original panic.
singleflight does not document how it handles panics.
https://go.dev/cl/526171 proposes changing singleflight to wrap the original panic value, when that value is an error. This will allow recovering the original panic value, but only when that value is an error.
I'm uncertain about the special handling of error values; what if the panic value isn't an error, but the caller wants it anyway? On the other hand, it seems useful for singleflight to add stack information for the source of the original panic, and when the original value is an error using error wrapping to augment that error is fairly sensible.