HandlerOption for ErrorWriter to fallback to JSON if no header is provided

[weeco]

Is your feature request related to a problem? Please describe.
I have an API exposed via different protocols (connect, grpc and connect-gateway/http). This API is auth protected (Bearer token in header or via Cookie-based session) and I'm using middlewares for that. I'd like to return helpful errors (missing, invalid, expired bearer token for example) for all of these APIs. Thus, the ErrorWriter is almost the perfect tool.

One thing I do not really want to enforce is setting an apropriate Content-Type in the request - for example if someone just wants to test an API endpoint via the Browser and the content-type is not set the ErrorWriter would not work because it's missing the Content-Type header with the protocol information.

Describe the solution you'd like

I would like to have a little bit more control (e.g. an extra HandlerOption) when constructing a new ErrorWriter, so that I could configure the ErrorWriter to fallback to writing JSON if no header is set.

Describe alternatives you've considered
I could inject the Content-Type header when no Content-Type header is set. It is a bit silly to do so, because the sole purpose would be to trick the ErrorWriter to write JSON.

Additional context

// In my middleware somewhere...
// This condition will evaluate to false if no Content-Type is set (e.g. request from the Browser), thus no error will be written by my ErrorWriter
if errorWriter.IsSupported(r) {
	err := apierrors.NewConnectError(
		connect.CodeUnauthenticated,
		errors.New("no bearer token found in authorization header"),
		apierrors.NewErrorInfo(commonv1alpha1.Reason_REASON_NO_AUTHENTICATION_TOKEN.String()),
	)
	_ = errorWriter.Write(w, r, err)
	return
}

errorWriter.Write() will only work if a known Content-Type header is set. Maybe there's a good reason why this should always be enforced, but I wouldn't know what reason that is.

[mattrobenolt]

In my experience implementing this, since I do very similar (auth with an http middleware), I chose to fall back to plain text as the default:

Something like:

if errorWriter.IsSupported(r) {
	errorWriter.Write(w, r, connect.NewError(connect.CodeUnauthenticated, err))
} else {
	w.WriteHeader(http.StatusUnauthorized)
	w.Write([]byte(err.Error()))
}

Obviously this behavior is subjective for what's right/wrong, but to me, it felt if the client isn't requesting JSON, there's no reason to try to return JSON. We can return some human readable text as text/plain.

To me, based on what you're saying, you want to allow and assume a default Content-Type on the requests as well. I think overall that's a design decision of your API and can be injected in a middleware if there is no Content-Type set first, like you proposed. I don't personally think this is very hacky, and would enable a "json by default" for your entire API, and not just error responses. It's something I've personally considered doing since it's a tad cumbersome when using curl to need to overly specify the content type. :)

I hope that helps, curious of other opinions tho.

[weeco]

I think even a configurable HandlerOption that tells the ErrorWriter to fallback to plaintext would be helpful to you in that case right? In my case I just want the fallback option to be JSON instead.

[emcfarlane]

@weeco this issue should be fixed by #654 and released in 1.14.