net/http: Add Error Callbacks

[theory]

I've written a little REST server on net/http that's supposed to only ever return JSON contents. At least, that's what the parts I wrote do. However, there are certain errors that net/http encounters before my code is ever called. In those situations, I have no access to any code to determine what the content type and message should be for a given error. For example, take this stupid simple implementation:

package main

import (
	"encoding/json"
	"net/http"
)

type Server struct{}

func (server *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) {
	w.Header().Set("Content-Type", "application/json; charset=UTF-8")
	w.WriteHeader(http.StatusOK)
	if err := json.NewEncoder(w).Encode(http.StatusText(http.StatusOK)); err != nil {
		panic(err)
	}
}

func main() {
	http.ListenAndServe(":8080", &Server{})
}

If I submit a request with borked headers (this one has a wayward newline in the Authorization header), my ServeHTTP() method is never called, presumably because there's an error when parsing the request to create the Request object:

> curl -i -H 'Authorization: Basic hi
there' 'http://localhost:8080/'
HTTP/1.1 400 Bad Request
Content-Type: text/plain; charset=utf-8
Connection: close

400 Bad Request

This makes complete sense to me, but if I'm creating an API with a contract to only ever return JSON, I'd like to have some way to plug into this error response and set the content-type and the body. I think the same challenge exists for Not Found errors. Perhaps there could be special handlers to configure these situations, similar to how Gorilla mux provides an assignable NotFoundHandler field in its Router object?

I'm not sure exactly what a generalized interface would look like for such error handlers (methods on Handler protocol?), but some way to hook in and get some control over net/http-initiated error responses would be very useful for guaranteeing the content-type contract of a web API.

[artyom]

@theory consider this: practically there's big enough chance that API would be running behind some kind of load balancer that may send error replies on its own in some scenarios (including malformed client requests). This would violate your assumption about getting replies that only have json payload in their bodies.

[theory]

That's often true, @artyom, but not an assumption I'd always want to make. In order to ensure the integrity of the interface I provide, I'd like to have some ability to determine output formats for errors from http itself, regardless of what the operational deployment ultimately looks like. Helps me avoid QA questions, too.

[artyom]

This one is a potential duplicate of #10123.

[theory]

Thanks @artyom, I think you're right.

[ianlancetaylor]

#10123 is closed, so should we close this one as well? Or with more experience does this seem like a real problem that net/http ought to address?

[ghost]

I don't think it's a dup of #10123. Bypassing the handler altogether is a different thing.

[tombergan]

I believe this is a dup of #18997 (via #18095). Please reopen if you disagree.

#18997 is a long thread. The tl;dr is here:
#18997 (comment)

[tombergan]

Sorry, reopening, it's not a dup. You don't just want to log, you want to return a custom response. I would not consider it a dup of #10123 either, because #10123 was about overriding http.Error, while this is about returning a custom response for internal errors that user code currently has no control over.

I don't know how to fix this cleanly. The quick hack is to add a field http.Server.InternalErrorResponseWriter, but that's ugly. Another approach is to have the server create an actual request, but somehow mark that request "bad". I'm not sure how to do that without breaking existing code.

[theory]

Not beautiful, but the simplest solution is to add a method one can override something like:

type ErrorHandler interface {
    HandleError func(w http.ResponseWriter, code int)
}

Then check to see if the Handler implements it.

Pretty similar to Error(), I guess.

[maxmoehl]

I have a similar use-case where the stdlib server will usually tell the client that it made a mistake without the handler ever getting a chance to interact with the request. In our case there are two aspects to this:

• We have a contract where we expose certain errors via a special header which is (obviously) not done by the stdlib.
• We log every request to be able to support our users with troubleshooting when their clients misbehave, for some error categories there is no log on the server at all, for example this switch statement is something I've came across a few times when investigating problems.

This is probably somewhat closer to #10123, but still different: I'm looking to respond to errors which are never exposed to a handler compared to being able to replace existing default handlers (e.g. 404). I'm happy to open a different issue if you think this does not fit this issue.

The suggestion from @theory is already pretty close, but I'd prefer an approach closer to the existing ServeHTTP function:

type ErrorHandler interface {
	ServeError(w http.ResponseWriter, r *http.Request, err error)
}

The ServeError function should be prepared to handle broken response writers as well as incomplete requests. At least the RemoteAddr should still be guaranteed to exist. It will give us the chance to log as much of the information as is available. The stdlib would not handle such cases on its own if such a handler is provided.

The handler would also be called for some of the cases where currently Server.ErrorLog is called: TLS handshake errors, panics and probably some additional places which I missed. Those are currently emitted as a log but it only contains very little data and the format does not align with the rest of our logs, being able to adjust that would be a huge plus.

[theory]

@maxmoehl Looks nice. Is there a way to know what the response code was intended to be?

[maxmoehl]

Right, it would probably be a good idea to expose that as well, so maybe the interface should be extended to include the encountered error, I've adjusted my original comment (I wanted to do that from the get go but somehow forgot it 😄). That error could also carry the status code which is "proposed" by the stdlib.