cannot use multiple different error handlers in one gateway binary

[jgiles]

Fill in the following sections with explanations of what's gone wrong.

Steps you follow to reproduce the error:

1. Create ServeMux without a custom error handler. Register it to handle prefix "/v1"
2. Create a ServeMux with a custom error handler using runtime.WithProtoErrorHandler. Register it to handle prefix "/v2".
3. Make calls resulting in errors to a mix of v1 and v2 endpoints.

Observe that the v2 error handler runs for:

• errors returned from the backends on both the v1 and v2 endpoints
• errors returned from within runtime.ServeMux code (e.g. Unimplemented on unknown URIs) on the v2 endpoints

Errors from within runtime.ServeMux code (like unknown URIs) under v1 result in unexpected use of OtherErrorHandler errors.

What did you expect to happen instead:

• v1 endpoints use the default error handler
• v2 endpoints use the configured custom error handler.

What's your theory on why it isn't working:

• The WithProtoErrorHandler option overwrites a global handler HTTPError and configures the global OtherErrorHandler to return "unexpected use" errors:

  grpc-gateway/runtime/mux.go

  Line 169 in 9aff355

  HTTPError = serveMux.protoErrorHandler

• Generated gateway handler code calls the global HTTPError regardless of the protoErrorHandler configured on the current ServeMux instance:

  grpc-gateway/protoc-gen-grpc-gateway/gengateway/template.go

  Line 571 in 9aff355

  runtime.HTTPError(ctx, mux, outboundMarshaler, w, req, err)

• Without a configured protoErrorHandler, ServeMux falls back to the global OtherErrorHandler (which has been overwritten if any mux in the process has a protoErrorHandler configured).

An initial patch to fix this issue can be seen here: master...paxosglobal:per-mux-error-handler

However, since this does change the generated code to rely on the updated runtime code, I'd appreciate guidance on the process for handling/merging this kind of breaking change.

[jgiles]

I guess a less-breaking alternative patch would be to modify DefaultHTTPError to look and see if there is a configured protoErrorHandler (and stop overwriting HTTPError when configuring protoErrorHandler). Then the current generated code continues to operate.

[jgiles]

Actually, that would lead to trouble if someone tried to explicitly configure DefaultHTTPError as the proto error handler. It also means clients can't use HTTPError a custom "global fallback" error handler.

We could initialize HTTPError = muxOrGlobalErrorHandler, retain the current DefaultErrorHandler, and set var GlobalErrorHandler = DefaultErrorHandler. That would make the current generated code work, preserve the options people currently have for directly modifying HTTPError, and allow setting an alternative global fallback.

[johanbrandhorst]

The latter option sounds good to me, please put together a PR if you can find the time.