net/http: documentation: make it clear that the built-in panic recovery in net/http is generally a practice that should be avoided
Related to this discussion in golang-nuts https://groups.google.com/g/golang-nuts/c/zzUfBynUVV8/m/axzz10zPBgAJ
I recently discovered that I had a misconception about how panic recovery works, especially in HTTP handlers. I wrote an article explaining that misunderstanding and suggested using more recover calls for panics in goroutines started from HTTP handler requests. That seemed like a reasonable approach based on the http package 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. (reference)
Reading that, I thought it would be a natural pattern to follow the same logic for goroutines started from HTTP requests. However, the feedback I received from other experienced Go engineers suggested that this is considered a bad practice, and that the built-in recovery mechanism in the HTTP server was a historical mistake that the Go team supposedly regrets.
Therefore, I would like to update net/http documentation to make it more explicit: although the HTTP server has built-in panic recovery, it should not be treated as a best practice pattern. The general guidance should be to let unexpected panics crash the program, unless explicitly recovering from a panic you triggered and fully understand. Clearer documentation would help avoid confusion for engineers who might otherwise assume that, since the standard library does this, it is a good pattern to follow for goroutines started from HTTP handlers as well.
Related Issues
- net/http: document Server recovering panics #8594 (closed)
- proposal: net/http: don't recover from handler panics #25245 (closed)
- doc: Effective Go's "safelyDo" does not demonstrate safe use of recover #19070
- net/http: document panic behavior of `CrossOriginProtection.AddInsecureBypassPattern` #75226 (closed)
- doc: be more explicit about panic / recover & new goroutines #11832 (closed)
- proposal: allow net/http server to fail on panic instead of recovering #16542 (closed)
- net/http: TimeoutHandler hides panic after timeout #34608
- proposal: net/http: optionally disable recover on panic #70292
Related Discussions
(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)
@neild @rsc
I personally think recovering panics automatically in handlers is a mistake.
Others disagree. I don't know that there's a consensus on this.
I think we should do #70292 and document that we strongly recommend turning it on.