proposal: allow net/http server to fail on panic instead of recovering

[HasAusten]

I would like to have the ability to turn off net/http server's current behavior to recover on panic.

My proposal is to add a new package variable http.ServerRecoverOnPanic that has a default value of True which is then checked in server.go before the recover is issued. By making the default value of http.ServerRecoverOnPanic True, the current behavior to recover would be maintained for backwards compatibility.

It is my feeling that the software designer should be able to determine when, and when not to recover from a panic, to allow for existing tools like supervisord to handle service crashes and to provide visibility and notification when a panic occurs.

[cespare]

There's no way to have a default value of true so it would need to be DisableRecoverOnPanic instead.

Otherwise this sounds like a good idea to me.

[minux]

For a package level variable, its initialization value could be true. However, I think this should be a http.Server specific setting instead.

[cespare]

@minux @HasAusten sorry, I didn't notice that you were proposing a package-level variable. I agree that it should be a Server-level configuration setting.

[rakyll]

/cc @bradfitz

[bradfitz]

If we did this (per-Server), we would still need a way to retain the ability for handlers to abort a handler and send an intentional stream break to the peer. Currently the defacto way to do that is to panic, which works in both http2 (by sending a RST_STREAM) and http1 (by closing the TCP connection which breaks any chunked encoding and the peer never sees the EOF 0-byte final chunk).

I would almost say that adding the knob is a backwards incompatible change, since Handlers might then have a new meaning of panic, but the panic-to-close-stream behavior is not very documented or used, and this would be opt-in, so maybe it's okay.

But we'd need to bless & document an official way to break streams.

[HasAusten]

Agreed, server configuration makes more sense.

[rgeronimi]

+1
The current behavior of managing panics does not follow the traditional Go intuition about how things work

[adg]

While I agree that this would be useful, I agree with @bradfitz that it is difficult to see how it might be done without breaking backward compatibility.

If you want your program to crash when a panic happens, you can wrap your handlers (or even the root ServeMux) in something like the panicHandler type I have included below:

package main

import (
        "fmt"
        "log"
        "net/http"
        "os"
        "runtime"
)       

type panicHandler struct {      
        http.Handler
}       

func (h panicHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
        defer func() { 
                err := recover()
                if err != nil {
                        buf := make([]byte, 1<<20)
                        n := runtime.Stack(buf, true)
                        fmt.Fprintf(os.Stderr, "panic: %v\n\n%s", err, buf[:n])
                        os.Exit(1)
                }
        }()
        h.Handler.ServeHTTP(w, r)
}       

func handler(w http.ResponseWriter, r *http.Request) {
        panic("oh no!")
}       

func main() {
        http.Handle("/", panicHandler{http.HandlerFunc(handler)})
        log.Fatal(http.ListenAndServe("localhost:8080", nil))
}

Given the workaround and the difficulty of implementing the change, I am going to decline this proposal. Thanks≥