Remove all meta from wire errors

connectrpc · Mar 14, 2024 · b10aaca · b10aaca
1 parent 418009b
commit b10aaca
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 5 deletions.
diff --git a/connect_ext_test.go b/connect_ext_test.go
@@ -558,7 +558,10 @@ func TestErrorHeaderPropagation(t *testing.T) {
 			err.Meta().Set("Content-Type", "application/xml")
 			err.Meta().Set("Accept-Encoding", "bogus")
 			err.Meta().Set("Date", "Thu, 01 Jan 1970 00:00:00 GMT")
-			err.Meta().Set("X-Test", "test") // Allow custom headers.
+			err.Meta().Set("Grpc-Status", "0")
+			// Allow custom headers.
+			err.Meta()["x-test-case"] = []string{"test"}
+			err.Meta().Set("X-Test", request.Header().Get("X-Test"))
 			return nil, err
 		},
 	}
@@ -599,9 +602,77 @@ func TestErrorHeaderPropagation(t *testing.T) {
 		assert.NotEqual(t, meta.Values("Content-Type"), []string{"application/xml"})
 		assert.NotEqual(t, meta.Values("Content-Length"), []string{"1337"})
 		assert.NotEqual(t, meta.Values("Date"), []string{"Thu, 01 Jan 1970 00:00:00 GMT"})
-		assert.Equal(t, meta.Values("X-Test"), []string{"test"})
+		assert.Equal(t, meta.Values("x-test-case"), []string{"test"})
+		assert.Equal(t, meta.Values("X-Test"), []string{t.Name()})
 	}
+	t.Run("connect", func(t *testing.T) {
+		t.Parallel()
+		client := pingv1connect.NewPingServiceClient(server.Client(), server.URL())
+		testPing(t, client)
+	})
+	t.Run("grpc", func(t *testing.T) {
+		t.Parallel()
+		client := pingv1connect.NewPingServiceClient(server.Client(), server.URL(), connect.WithGRPC())
+		testPing(t, client)
+	})
+	t.Run("grpc-web", func(t *testing.T) {
+		t.Parallel()
+		client := pingv1connect.NewPingServiceClient(server.Client(), server.URL(), connect.WithGRPCWeb())
+		testPing(t, client)
+	})
+}
 
+func TestWireErrorHeaderPropagation(t *testing.T) {
+	t.Parallel()
+	pingServer := &pluggablePingServer{
+		ping: func(ctx context.Context, request *connect.Request[pingv1.PingRequest]) (*connect.Response[pingv1.PingResponse], error) {
+			err := connect.NewWireError(connect.CodeInvalidArgument, errors.New("ping error"))
+			msgDetail := &wrapperspb.StringValue{Value: "server details"}
+			errDetail, derr := connect.NewErrorDetail(msgDetail)
+			if derr != nil {
+				return nil, derr
+			}
+			err.AddDetail(errDetail)
+			err.Meta().Set("X-Test", request.Header().Get("X-Test"))
+			return nil, err
+		},
+	}
+	mux := http.NewServeMux()
+	mux.Handle(pingv1connect.NewPingServiceHandler(pingServer))
+	server := memhttptest.NewServer(t, mux)
+
+	testPing := func(t *testing.T, client pingv1connect.PingServiceClient) {
+		t.Helper()
+		request := connect.NewRequest(&pingv1.PingRequest{})
+		request.Header().Set("X-Test", t.Name())
+		_, err := client.Ping(context.Background(), request)
+		if !assert.NotNil(t, err) {
+			return
+		}
+		assert.Equal(t, connect.CodeOf(err), connect.CodeInvalidArgument)
+		var connectErr *connect.Error
+		if !assert.True(t, errors.As(err, &connectErr)) {
+			return
+		}
+		assert.Equal(t, connectErr.Message(), "ping error")
+
+		details := connectErr.Details()
+		if assert.Equal(t, len(details), 1) {
+			detailMsg, err := details[0].Value()
+			if !assert.Nil(t, err) {
+				return
+			}
+			serverDetails, ok := detailMsg.(*wrapperspb.StringValue)
+			if !assert.True(t, ok) {
+				return
+			}
+			assert.Equal(t, serverDetails.Value, "server details")
+		}
+		meta := connectErr.Meta()
+		assert.NotNil(t, meta.Values("Content-Type"))
+		assert.NotNil(t, meta.Values("Date"))
+		assert.Equal(t, meta.Values("X-Test"), nil) // Wire errors don't propagate meta.
+	}
 	t.Run("connect", func(t *testing.T) {
 		t.Parallel()
 		client := pingv1connect.NewPingServiceClient(server.Client(), server.URL())

diff --git a/error_writer.go b/error_writer.go
@@ -127,7 +127,7 @@ func (w *ErrorWriter) Write(response http.ResponseWriter, request *http.Request,
 }
 
 func (w *ErrorWriter) writeConnectUnary(response http.ResponseWriter, err error) error {
-	if connectErr, ok := asError(err); ok {
+	if connectErr, ok := asError(err); ok && !connectErr.wireErr {
 		mergeMetadataHeaders(response.Header(), connectErr.meta)
 	}
 	response.WriteHeader(connectCodeToHTTP(CodeOf(err)))

diff --git a/protocol_connect.go b/protocol_connect.go
@@ -766,7 +766,7 @@ func (hc *connectUnaryHandlerConn) writeResponseHeader(err error) {
 		header[headerVary] = append(header[headerVary], connectUnaryHeaderAcceptCompression)
 	}
 	if err != nil {
-		if connectErr, ok := asError(err); ok {
+		if connectErr, ok := asError(err); ok && !connectErr.wireErr {
 			mergeMetadataHeaders(header, connectErr.meta)
 		}
 	}

diff --git a/protocol_grpc.go b/protocol_grpc.go
@@ -859,7 +859,7 @@ func grpcErrorToTrailer(trailer http.Header, protobuf Codec, err error) {
 		)
 		return
 	}
-	if connectErr, ok := asError(err); ok {
+	if connectErr, ok := asError(err); ok && !connectErr.wireErr {
 		mergeMetadataHeaders(trailer, connectErr.meta)
 	}
 	setHeaderCanonical(trailer, grpcHeaderStatus, code)