envoyproxy / envoy

Cloud-native high-performance edge/middle/service proxy
https://www.envoyproxy.io
Apache License 2.0
24.74k stars 4.75k forks source link

grpc_http1_reverse_bridge filter: support text errors from upstream #12953

Open mpuncel opened 4 years ago

mpuncel commented 4 years ago

Title: grpc_http1_reverse_bridge filter: support text errors from upstream

Description: Currently, the grpc_http1_reverse_bridge filter is configured with a single content_type that the upstream must respond with, or the filter stands in its own error (saying that the content type was wrong).

However, gRPC supports sending text error responses (see "Standard error model") here https://grpc.io/docs/guides/error/. This is exactly how the filter stands in its current error without needing to have knowledge of the response proto message.

It seems like it would make sense to allow configuring the filter to support passing text error message from the upstream to the downstream by using the grpc-message trailer as it currently does.

Proposal:

Add a pass_through_plaintext_errors bool to the filter's config. When set:

if upstream responds with anything other than 4xx or 5xx, and content type doesn't match the configured content-type, keep the behavior the same.

If the upstream responds with 4xx or 5xx, and the content type is something like text/plain, put it into grpc-message. (Note that i'm not knowledgeable about MIME types so probably that's not an exhaustive list of what should be included, and maybe the exact allowable types should be configurable as well).

mattklein123 commented 4 years ago

At a high level this makes sense to me.

If the upstream responds with 4xx or 5xx, and the content type is something like text/plain, put it into grpc-message. (Note that i'm not knowledgeable about MIME types so probably that's not an exhaustive list of what should be included, and maybe the exact allowable types should be configurable as well).

This probably needs more thinking. I would potentially chat with the gRPC folks about this. cc @markdroth

altexdim commented 3 years ago

There's workaround to use a lua script to pass over the headers you need

          - name: envoy.filters.http.lua
            typed_config:
              "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua
              inline_code: |
                function envoy_on_response(response_handle)
                  if (response_handle:headers() and response_handle:trailers())
                  then
                    if (response_handle:headers():get("grpc-status")) 
                    then
                      response_handle:trailers():replace("grpc-status", response_handle:headers():get("grpc-status"))
                    end
                    if (response_handle:headers():get("grpc-message")) 
                    then
                      response_handle:trailers():replace("grpc-message", response_handle:headers():get("grpc-message"))
                    end
                    if (response_handle:headers():get("grpc-status-details-bin")) 
                    then
                      response_handle:trailers():replace("grpc-status-details-bin", response_handle:headers():get("grpc-status-details-bin"))
                    end
                  end
                end