Errors in response streams do not go through the registered error handler

[jhump]

I am using runtime.WithProtoErrorHandler to provide custom error handling in my service. In particular, my service discards the error message (and provides a default based on the gRPC code) unless the error includes details that specify a "safe" message (e.g. no stack trace or other internal details leaked). I also have a slightly modified mapping of gRPC codes to HTTP codes.

But if I have a bidi- or server-streaming endpoint, the error will get converted into a StreamError message in a way that is not pluggable/configurable. That means the err.Error() or stat.Message() is rendered in the response chunk with no ability to customize it (such as stripping away any inappropriate details that are present in the message).

I understand that the existing ProtoErrorHandlerFunc type is not necessarily appropriate for use with a streaming response, since it assumes it can set a response code and response headers. So I think a new ServerMuxOption is necessary for this.

Proposal (two-pronged):

1. Use ProtoErrorHandlerFunc when possible -- e.g. when no response messages have actually been sent back.
2. Add new ServerMuxOption for rendering errors, maybe a signature like so:
   func(context.Context, error) *StreamError

That second part could even be extended to customizing the envelope that is used for streaming response messages. Instead of a function, it could be an interface:

type StreamChunkHandler interface {
    HandleResponse(context.Context, proto.Message) interface{}
    HandleError(context.Context, error) interface{}
}

This allows for using a custom envelope as well as using a custom error representation, too. The returned item from these methods would then be passed to the configured Marshaler for rendering to the HTTP stream.

[tmc]

I think this proposal is sound and I like the flexibility it provides. Should the return types be proto.Message?
@achew22 @tamalsaha thoughts?

[jhump]

Should the return types be proto.Message?

Currently, the ProtoErrorHandlerFunc can do whatever it likes, including using a non-proto as the response body. So I'm inclined to leave the same flexibility -- especially since the envelope itself certainly need not be a proto (even if it contains a payload that is a proto).

I did realize though that customizing the envelope and error type means that we'd no longer be able to generate an accurate swagger/OpenAPI spec, since the plugin would not know what the envelope/error types look like. (FWIW, the generated spec is already currently wrong since it describes the response as if it were single instance of the response type and does not attempt to describe the envelope or error schema.)

So I think the first suggestion is the better way to go: just have StreamErrorHandlerFunc with a signature of func(context.Context, error) *StreamError.