middleware: recoverer ignores http.ErrAbortHandler

[ChristophPech]

The http.ErrAbortHandler panic usually causes a connection close. The recoverer just ignores the panic instead of reraising it causing connection leaks.

[pkieltyka]

I'm a bit confused.. how should we handle http.ErrAbortHandler then..?

the current middleware: https://github.com/go-chi/chi/blob/master/middleware/recoverer.go#L22

looks like so:

func Recoverer(next http.Handler) http.Handler {
	fn := func(w http.ResponseWriter, r *http.Request) {
		defer func() {
			if rvr := recover(); rvr != nil && rvr != http.ErrAbortHandler {

				logEntry := GetLogEntry(r)
				if logEntry != nil {
					logEntry.Panic(rvr, debug.Stack())
				} else {
					PrintPrettyStack(rvr)
				}

				w.WriteHeader(http.StatusInternalServerError)
			}
		}()

		next.ServeHTTP(w, r)
	}

	return http.HandlerFunc(fn)
}

suggestion is to change it to....?

func Recoverer(next http.Handler) http.Handler {
	fn := func(w http.ResponseWriter, r *http.Request) {
		defer func() {
			if rvr := recover(); rvr != nil {

				logEntry := GetLogEntry(r)
				if logEntry != nil {
					logEntry.Panic(rvr, debug.Stack())
				} else {
					PrintPrettyStack(rvr)
				}

				if rvr == http.ErrAbortHandler {
					panic(rvr)
				}

				w.WriteHeader(http.StatusInternalServerError)
			}
		}()

		next.ServeHTTP(w, r)
	}

	return http.HandlerFunc(fn)
}

so, we log and re-raise the panic..? or something else is suggested here..?

[drakkan]

please take a look at #624, the panic should not be logged

https://go.googlesource.com/go/+/master/src/net/http/server.go#1793

[pkieltyka]

I get it now, thank you. Do you mind rebasing your PR and I will then merge it?

[drakkan]

I get it now, thank you. Do you mind rebasing your PR and I will then merge it?

Dome, thank you