proposal: net/http: return an error instead of panicking on http.ServeMux registration errors

[matttproud]

This will fall into the Go 2 category bucket, because as proposed it is a breaking behavioral change for Go 1.

Proposal

Instead of panicking on invalid http.Handler registrations, (*http.ServeMux).Handle should return errors. This is germane in when the user attempts to register multiple http.Handler against the same path as demonstrated with http.DefaultServeMux:

http.Handle("/path", h)
http.Handle("/path", h)

// Output:
panic: http: multiple registrations for /path

Instead, I propose (*http.ServeMux).Handle gets a new method signature similar to this:

func (*http.ServeMux) Handle(string, Handler) error { ... }

In the case above, a sentinel or structured error value could be returned by package http to enable callers to handle the error:

package http

var ErrMultipleRegistrations = errors.New("http: more than one handler registered against the path")

// Or:

type MultipleRegistrationError struct {
  Path string
}

func (err *MultipleRegistrationError) Error() string { ... }

Motivation

Why is this panic worth handling at all? Shouldn't the user know about all registrations and be able to prevent multiple?

I believe the latter question is the first to start with. Based on our team's experience in offering a server framework used internally by several thousand customer teams, we got to see first-hand how the multiple registration behavior played in a gigantic monorepo, especially when combined with code that registered default debugging handlers using http.Handle in func init bodies. It played out terribly. This was because there were two registration pathways:

1. through the server framework directly, which could catch duplicates of thing registered through it
2. things registered directly by other infrastructure providers in func init on http.DefaultServeMux using http.Handle and similar

We could only control no. 1. Auditing no. 2 was a boil-the-ocean affair, particularly because there was no wisdom nor enforcement of avoiding mutating package state in registrations. Attempting to convert no. 2 to support per-http.ServeMux registration was a good compromise, but we could never catch up with the codebase.

In effect, this means that it's infeasible in large codebases to ensure that there is central control over who is registering what. We tried with a per-server http.ServeMux that falls back to http.DefaultServeMux for paths it doesn't recognize, but the result and behavior is not pretty. It's also error-prone.

Returning an error sentinel or structured error value would enable intermediate frameworks and middleware to more gracefully handle such cases, which is a huge win for developer experience.

Other Thoughts and Speculation

An API for unregistering existing handlers might be interesting to consider. A server framework could de-register a duplicate. Or as a twist on this, last-one-wins semantic for registration? I haven't thought about these super closely, but I can say the panic should go.

Perhaps a structured error return (as suggested above) could return information about the registrations: what package, what line, identifier type information, or interface value of the pre-existing http.Handler.

[ianlancetaylor]

CC @neild @bradfitz

[neild]

We aren't making breaking changes to the standard library, so this is moot.

That said, I'm extremely dubious about the value of doing this. If two pieces of code attempt to register the same path, your choices largely come down to not starting the program, or picking a winner. Picking a winner is a poor choice; there's a fault in the program logic and we have no way to tell how serious it is. It isn't difficult to construct scenarios where papering over the error causes a significant outage or security vulnerability.

Panicking immediately surfaces the issue in a clear fashion, generally at program start time.

If panics are a problem due to too many sources registering handlers on DefaultServeMux, I don't see how returning an error helps: Instead of a panic, you either get effectively random behavior over which handler is registered, or something looks at the error and panics anyway.

[bsiegert]

Returning an error would be an opportunity to handle it in an orderly fashion. For instance, a framework could take that error and export it to a monitoring system.

In fact, the most common way I have seen this happen is in a test:

func TestA(t *testing.T) {
  http.Handle("/", &MyHandler{})
  // run test
}

func TestB(t *testing.T) {
  http.Handle("/", &SomeOtherHandler{})  // panic
  // run test
}

We have users that test that their initialization routine works, but as soon as there is a HTTP handler registration, the routine can no longer be idempotent.

[neild]

Tests should be using a test-specific mux, not the global DefaultServeMux. Panicking in this case seems better than returning an error that the test might ignore.

[komuw]

I'm not supportive of this proposal, for reasons that have already been enumerated already.

However, I see an opportunity for improving the panic message.

Auditing no. 2 was a boil-the-ocean affair, particularly because there was no wisdom nor enforcement of avoiding mutating package state in registrations.

The http panic message could be improved so that it reads something like:

http: multiple registrations for /path. conflicting handlers are:
   logingHandler - /home/main.go:312
   shoppingCartHandler - /home/shop.go:17

The exact message can be up to debate, but the general idea is that the panic will tell you exactly where the conflicting handlers are defined.

This is similar to the superfluous response.WriteHeader call where the http library provides this sort of info:

go/src/net/http/server.go

Lines 1146 to 1147 in 4a4127b

	caller := relevantCaller()
	w.conn.server.logf("http: superfluous response.WriteHeader call from %s (%s:%d)", caller.Function, path.Base(caller.File), caller.Line)

I know that in the case of superfluous it is a log message instead of a panic message. But still debugging superfluous is much easier than debugging multiple registrations and the callers info makes it that easy.

2 cents.

[Merovius]

I like the idea of annotating the panic message with extra line numbers. We do something similar with an internal config library extracting values from the environment, where we want to guarantee (for maintainability) that only one code location ever looks at any given variable.

Returning an error would be an opportunity to handle it in an orderly fashion. For instance, a framework could take that error and export it to a monitoring system.

AFAIK Handle{,Func} only ever panics on duplicate registration (correct me if I'm wrong) so ISTM this would work

func FrameworksHandle(pattern string, h http.Handler) (err error) {
    defer func() {
        if recover() != nil {
            err = fmt.Errorf("duplicate registration of %q", pattern)
        }
    }()
    http.Handle(pattern, h)
}

Personally, I tend to agree with @neild though, there doesn't seem to really be anything useful to do with that error, in general.