From 4520f0d0a6848cb5aef9e0fe6530fb890a65a86a Mon Sep 17 00:00:00 2001 From: Yuri Nikolic Date: Fri, 27 Oct 2023 16:33:39 +0200 Subject: [PATCH 1/4] httpgrpc: correct handling of non-loggable errors Signed-off-by: Yuri Nikolic --- httpgrpc/httpgrpc.go | 27 +++++- httpgrpc/httpgrpc_test.go | 147 +++++++++++++++++++++++++++++++++ httpgrpc/server/server.go | 14 +++- httpgrpc/server/server_test.go | 34 ++++++++ middleware/grpc_logging.go | 11 +++ 5 files changed, 230 insertions(+), 3 deletions(-) diff --git a/httpgrpc/httpgrpc.go b/httpgrpc/httpgrpc.go index 3012edd42..83ea023f3 100644 --- a/httpgrpc/httpgrpc.go +++ b/httpgrpc/httpgrpc.go @@ -6,10 +6,12 @@ package httpgrpc import ( "context" + "errors" "fmt" "github.com/go-kit/log/level" "google.golang.org/grpc/metadata" + grpcstatus "google.golang.org/grpc/status" spb "github.com/gogo/googleapis/google/rpc" "github.com/gogo/protobuf/types" @@ -44,7 +46,7 @@ func ErrorFromHTTPResponse(resp *HTTPResponse) error { // HTTPResponseFromError converts a grpc error into an HTTP response func HTTPResponseFromError(err error) (*HTTPResponse, bool) { - s, ok := status.FromError(err) + s, ok := statusFromError(err) if !ok { return nil, false } @@ -63,6 +65,29 @@ func HTTPResponseFromError(err error) (*HTTPResponse, bool) { return &resp, true } +// statusFromError tries to cast the given error into status.Status. +// If the given error, or any error from its tree are a status.Status, +// that status.Status and the outcome true are returned. +// Otherwise, nil and the outcome false are returned. +// This implementation differs from status.FromError() because the +// latter checks only if the given error can be cast to status.Status, +// and doesn't check other errors in the given error's tree. +func statusFromError(err error) (*status.Status, bool) { + if err == nil { + return nil, false + } + type grpcStatus interface{ GRPCStatus() *grpcstatus.Status } + var gs grpcStatus + if errors.As(err, &gs) { + st := gs.GRPCStatus() + if st == nil { + return nil, false + } + return status.FromGRPCStatus(st), true + } + return nil, false +} + const ( MetadataMethod = "httpgrpc-method" MetadataURL = "httpgrpc-url" diff --git a/httpgrpc/httpgrpc_test.go b/httpgrpc/httpgrpc_test.go index 1331f9753..7f47775f2 100644 --- a/httpgrpc/httpgrpc_test.go +++ b/httpgrpc/httpgrpc_test.go @@ -2,10 +2,14 @@ package httpgrpc import ( "context" + "fmt" "testing" + "github.com/gogo/status" "github.com/stretchr/testify/require" + "google.golang.org/grpc/codes" "google.golang.org/grpc/metadata" + grpcstatus "google.golang.org/grpc/status" ) func TestAppendMessageSizeToOutgoingContext(t *testing.T) { @@ -24,3 +28,146 @@ func TestAppendMessageSizeToOutgoingContext(t *testing.T) { require.Equal(t, []string{"GET"}, md.Get(MetadataMethod)) require.Equal(t, []string{"/test"}, md.Get(MetadataURL)) } + +func TestErrorf(t *testing.T) { + code := 400 + errMsg := "this is an error" + expectedHTTPResponse := &HTTPResponse{ + Code: int32(code), + Body: []byte(errMsg), + } + err := Errorf(code, errMsg) + stat, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, code, int(stat.Code())) + require.Equal(t, errMsg, stat.Message()) + checkDetailAsHTTPResponse(t, expectedHTTPResponse, stat) +} + +func TestErrorFromHTTPResponse(t *testing.T) { + var code int32 = 400 + errMsg := "this is an error" + headers := []*Header{{Key: "X-Header", Values: []string{"a", "b", "c"}}} + resp := &HTTPResponse{ + Code: code, + Headers: headers, + Body: []byte(errMsg), + } + err := ErrorFromHTTPResponse(resp) + require.Error(t, err) + stat, ok := status.FromError(err) + require.True(t, ok) + require.Equal(t, code, int32(stat.Code())) + require.Equal(t, errMsg, stat.Message()) + checkDetailAsHTTPResponse(t, resp, stat) +} + +func TestHTTPResponseFromError(t *testing.T) { + msgErr := "this is an error" + testCases := map[string]struct { + err error + isGRPCError bool + isHTTPGRCPError bool + expectedHTTPResponse *HTTPResponse + }{ + "no error cannot be parsed to an HTTPResponse": { + err: nil, + }, + "a random error cannot be parsed to an HTTPResponse": { + err: fmt.Errorf(msgErr), + }, + "a gRPC error built by gogo/status cannot be parsed to an HTTPResponse": { + err: status.Error(codes.Internal, msgErr), + }, + "a gRPC error built by grpc/status cannot be parsed to an HTTPResponse": { + err: grpcstatus.Error(codes.Internal, msgErr), + }, + "a gRPC error built by httpgrpc can be parsed to an HTTPResponse": { + err: Errorf(400, msgErr), + expectedHTTPResponse: &HTTPResponse{Code: 400, Body: []byte(msgErr)}, + }, + "a wrapped gRPC error built by httpgrpc can be parsed to an HTTPResponse": { + err: fmt.Errorf("wrapped: %w", Errorf(400, msgErr)), + expectedHTTPResponse: &HTTPResponse{Code: 400, Body: []byte(msgErr)}, + }, + } + for testName, testData := range testCases { + t.Run(testName, func(t *testing.T) { + resp, ok := HTTPResponseFromError(testData.err) + if testData.expectedHTTPResponse == nil { + require.False(t, ok) + require.Nil(t, resp) + } else { + require.True(t, ok) + + } + }) + } +} + +func TestStatusFromError(t *testing.T) { + msgErr := "this is an error" + testCases := map[string]struct { + err error + expectedStatus *status.Status + }{ + "no error cannot be cast to status.Status": { + err: nil, + }, + "a random error cannot be cast to status.Status": { + err: fmt.Errorf(msgErr), + }, + "a wrapped error of a random error cannot be cast to status.Status": { + err: fmt.Errorf("wrapped: %w", fmt.Errorf(msgErr)), + }, + "a gRPC error built by gogo/status can be cast to status.Status": { + err: status.Error(codes.Internal, msgErr), + expectedStatus: status.New(codes.Internal, msgErr), + }, + "a wrapped error of a gRPC error built by gogo/status can be cast to status.Status": { + err: fmt.Errorf("wrapped: %w", status.Error(codes.Internal, msgErr)), + expectedStatus: status.New(codes.Internal, msgErr), + }, + "a gRPC error built by grpc/status can be cast to status.Status": { + err: grpcstatus.Error(codes.Internal, msgErr), + expectedStatus: status.New(codes.Internal, msgErr), + }, + "a wrapped error of a gRPC error built by grpc/status can be cast to status.Status": { + err: fmt.Errorf("wrapped: %w", grpcstatus.Error(codes.Internal, msgErr)), + expectedStatus: status.New(codes.Internal, msgErr), + }, + "a gRPC error built by httpgrpc can be cast to status.Status": { + err: Errorf(400, msgErr), + expectedStatus: status.New(400, msgErr), + }, + "a wrapped gRPC error built by httpgrpc can be cast to status.Status": { + err: fmt.Errorf("wrapped: %w", Errorf(400, msgErr)), + expectedStatus: status.New(400, msgErr), + }, + } + for testName, testData := range testCases { + t.Run(testName, func(t *testing.T) { + stat, ok := statusFromError(testData.err) + if testData.expectedStatus == nil { + require.False(t, ok) + require.Nil(t, stat) + } else { + require.True(t, ok) + require.NotNil(t, stat) + require.Equal(t, testData.expectedStatus.Code(), stat.Code()) + require.Equal(t, testData.expectedStatus.Message(), stat.Message()) + } + }) + } +} + +func checkDetailAsHTTPResponse(t *testing.T, httpResponse *HTTPResponse, stat *status.Status) { + details := stat.Details() + require.Len(t, details, 1) + respDetails, ok := details[0].(*HTTPResponse) + require.True(t, ok) + require.NotNil(t, respDetails) + require.Equal(t, httpResponse.Code, respDetails.Code) + require.Equal(t, httpResponse.Headers, respDetails.Headers) + require.Equal(t, httpResponse.Body, respDetails.Body) +} diff --git a/httpgrpc/server/server.go b/httpgrpc/server/server.go index b0d808b7b..5dc547680 100644 --- a/httpgrpc/server/server.go +++ b/httpgrpc/server/server.go @@ -62,13 +62,18 @@ func (s Server) Handle(ctx context.Context, r *httpgrpc.HTTPRequest) (*httpgrpc. recorder := httptest.NewRecorder() s.handler.ServeHTTP(recorder, req) + header := recorder.Header() resp := &httpgrpc.HTTPResponse{ Code: int32(recorder.Code), - Headers: fromHeader(recorder.Header()), + Headers: fromHeader(header), Body: recorder.Body.Bytes(), } if recorder.Code/100 == 5 { - return nil, httpgrpc.ErrorFromHTTPResponse(resp) + err := httpgrpc.ErrorFromHTTPResponse(resp) + if containsDoNotLogErrorKey(header) { + return nil, middleware.DoNotLogError{Err: err} + } + return nil, err } return resp, nil } @@ -234,3 +239,8 @@ func fromHeader(hs http.Header) []*httpgrpc.Header { } return result } + +func containsDoNotLogErrorKey(hs http.Header) bool { + _, ok := hs[middleware.DoNotLogErrorHeader] + return ok +} diff --git a/httpgrpc/server/server_test.go b/httpgrpc/server/server_test.go index 1c32a1a0c..5dae049f1 100644 --- a/httpgrpc/server/server_test.go +++ b/httpgrpc/server/server_test.go @@ -151,3 +151,37 @@ func TestTracePropagation(t *testing.T) { assert.Equal(t, "world", recorder.Body.String()) assert.Equal(t, 200, recorder.Code) } + +func TestContainsDoNotLogErrorKey(t *testing.T) { + testCases := map[string]struct { + header http.Header + expectedOutcome bool + }{ + "if headers do not contain X-DoNotLogError, return false": { + header: http.Header{ + "X-First": []string{"a", "b", "c"}, + "X-Second": []string{"1", "2"}, + }, + expectedOutcome: false, + }, + "if headers contain X-DoNotLogError with a value, return true": { + header: http.Header{ + "X-First": []string{"a", "b", "c"}, + "X-DoNotLogError": []string{"true"}, + }, + expectedOutcome: true, + }, + "if headers contain X-DoNotLogError without a value, return true": { + header: http.Header{ + "X-First": []string{"a", "b", "c"}, + "X-DoNotLogError": nil, + }, + expectedOutcome: true, + }, + } + for testName, testData := range testCases { + t.Run(testName, func(t *testing.T) { + require.Equal(t, testData.expectedOutcome, containsDoNotLogErrorKey(testData.header)) + }) + } +} diff --git a/middleware/grpc_logging.go b/middleware/grpc_logging.go index 7f5db7725..0f9ba8a4f 100644 --- a/middleware/grpc_logging.go +++ b/middleware/grpc_logging.go @@ -7,6 +7,7 @@ package middleware import ( "context" "errors" + "net/http" "time" "github.com/go-kit/log" @@ -24,11 +25,21 @@ const ( gRPC = "gRPC" ) +var ( + DoNotLogErrorHeader = http.CanonicalHeaderKey("X-DoNotLogError") +) + // An error can implement ShouldLog() to control whether GRPCServerLog will log. type OptionalLogging interface { ShouldLog(ctx context.Context, duration time.Duration) bool } +type DoNotLogError struct{ Err error } + +func (i DoNotLogError) Error() string { return i.Err.Error() } +func (i DoNotLogError) Unwrap() error { return i.Err } +func (i DoNotLogError) ShouldLog(_ context.Context, _ time.Duration) bool { return false } + // GRPCServerLog logs grpc requests, errors, and latency. type GRPCServerLog struct { Log log.Logger From 00bb08e31615a1c2073aa370f99795d0041347a1 Mon Sep 17 00:00:00 2001 From: Yuri Nikolic Date: Sun, 29 Oct 2023 16:13:27 +0100 Subject: [PATCH 2/4] Fixing review findings Signed-off-by: Yuri Nikolic --- httpgrpc/server/server.go | 13 +++--- httpgrpc/server/server_test.go | 78 +++++++++++++--------------------- middleware/grpc_logging.go | 5 --- 3 files changed, 35 insertions(+), 61 deletions(-) diff --git a/httpgrpc/server/server.go b/httpgrpc/server/server.go index 5dc547680..34c4f1b6e 100644 --- a/httpgrpc/server/server.go +++ b/httpgrpc/server/server.go @@ -27,6 +27,10 @@ import ( "github.com/grafana/dskit/middleware" ) +var ( + DoNotLogErrorHeaderKey = http.CanonicalHeaderKey("X-DoNotLogError") +) + // Server implements HTTPServer. HTTPServer is a generated interface that gRPC // servers must implement. type Server struct { @@ -70,8 +74,8 @@ func (s Server) Handle(ctx context.Context, r *httpgrpc.HTTPRequest) (*httpgrpc. } if recorder.Code/100 == 5 { err := httpgrpc.ErrorFromHTTPResponse(resp) - if containsDoNotLogErrorKey(header) { - return nil, middleware.DoNotLogError{Err: err} + if _, ok := header[DoNotLogErrorHeaderKey]; ok { + err = middleware.DoNotLogError{Err: err} } return nil, err } @@ -239,8 +243,3 @@ func fromHeader(hs http.Header) []*httpgrpc.Header { } return result } - -func containsDoNotLogErrorKey(hs http.Header) bool { - _, ok := hs[middleware.DoNotLogErrorHeader] - return ok -} diff --git a/httpgrpc/server/server_test.go b/httpgrpc/server/server_test.go index 5dae049f1..3bc7f72de 100644 --- a/httpgrpc/server/server_test.go +++ b/httpgrpc/server/server_test.go @@ -73,25 +73,39 @@ func TestBasic(t *testing.T) { } func TestError(t *testing.T) { - server, err := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // Does a Fprintln, injecting a newline. - http.Error(w, "foo", http.StatusInternalServerError) - })) - require.NoError(t, err) - defer server.grpcServer.GracefulStop() + for _, doNotLog := range []bool{true, false} { + var stat string + if !doNotLog { + stat = "not " + } + t.Run(fmt.Sprintf("test header when DoNotLogErrorHeaderKey is %spresent", stat), func(t *testing.T) { + server, err := newTestServer(t, http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if doNotLog { + w.Header().Set(DoNotLogErrorHeaderKey, "true") + } + // Does a Fprintln, injecting a newline. + http.Error(w, "foo", http.StatusInternalServerError) + })) + require.NoError(t, err) + defer server.grpcServer.GracefulStop() - client, err := NewClient(server.URL) - require.NoError(t, err) + client, err := NewClient(server.URL) + require.NoError(t, err) - req, err := http.NewRequest("GET", "/hello", &bytes.Buffer{}) - require.NoError(t, err) + req, err := http.NewRequest("GET", "/hello", &bytes.Buffer{}) + require.NoError(t, err) - req = req.WithContext(user.InjectOrgID(context.Background(), "1")) - recorder := httptest.NewRecorder() - client.ServeHTTP(recorder, req) + req = req.WithContext(user.InjectOrgID(context.Background(), "1")) + recorder := httptest.NewRecorder() + client.ServeHTTP(recorder, req) - assert.Equal(t, "foo\n", recorder.Body.String()) - assert.Equal(t, 500, recorder.Code) + assert.Equal(t, "foo\n", recorder.Body.String()) + assert.Equal(t, 500, recorder.Code) + if doNotLog { + assert.Equal(t, recorder.Header().Get(DoNotLogErrorHeaderKey), "true") + } + }) + } } func TestParseURL(t *testing.T) { @@ -151,37 +165,3 @@ func TestTracePropagation(t *testing.T) { assert.Equal(t, "world", recorder.Body.String()) assert.Equal(t, 200, recorder.Code) } - -func TestContainsDoNotLogErrorKey(t *testing.T) { - testCases := map[string]struct { - header http.Header - expectedOutcome bool - }{ - "if headers do not contain X-DoNotLogError, return false": { - header: http.Header{ - "X-First": []string{"a", "b", "c"}, - "X-Second": []string{"1", "2"}, - }, - expectedOutcome: false, - }, - "if headers contain X-DoNotLogError with a value, return true": { - header: http.Header{ - "X-First": []string{"a", "b", "c"}, - "X-DoNotLogError": []string{"true"}, - }, - expectedOutcome: true, - }, - "if headers contain X-DoNotLogError without a value, return true": { - header: http.Header{ - "X-First": []string{"a", "b", "c"}, - "X-DoNotLogError": nil, - }, - expectedOutcome: true, - }, - } - for testName, testData := range testCases { - t.Run(testName, func(t *testing.T) { - require.Equal(t, testData.expectedOutcome, containsDoNotLogErrorKey(testData.header)) - }) - } -} diff --git a/middleware/grpc_logging.go b/middleware/grpc_logging.go index 0f9ba8a4f..feab36474 100644 --- a/middleware/grpc_logging.go +++ b/middleware/grpc_logging.go @@ -7,7 +7,6 @@ package middleware import ( "context" "errors" - "net/http" "time" "github.com/go-kit/log" @@ -25,10 +24,6 @@ const ( gRPC = "gRPC" ) -var ( - DoNotLogErrorHeader = http.CanonicalHeaderKey("X-DoNotLogError") -) - // An error can implement ShouldLog() to control whether GRPCServerLog will log. type OptionalLogging interface { ShouldLog(ctx context.Context, duration time.Duration) bool From 3ec93e8b535b45e351994edcc0c58b5d4e1e5e17 Mon Sep 17 00:00:00 2001 From: Yuri Nikolic Date: Tue, 31 Oct 2023 12:07:05 +0100 Subject: [PATCH 3/4] Fixing review findings 2 Signed-off-by: Yuri Nikolic --- CHANGELOG.md | 1 + httpgrpc/server/server.go | 7 +++ httpgrpc/server/server_test.go | 79 +++++++++++++++++++++++++++++++++- 3 files changed, 85 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f66087b43..b47c7d396 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -154,6 +154,7 @@ * [ENHANCEMENT] Ring: add support for aborting early if a terminal error is returned by a request initiated by `DoUntilQuorum`. #404 #413 * [ENHANCEMENT] Memcached: allow to configure write and read buffer size (in bytes). #414 * [ENHANCEMENT] Server: Add `-server.http-read-header-timeout` option to specify timeout for reading HTTP request header. It defaults to 0, in which case reading of headers can take up to `-server.http-read-timeout`, leaving no time for reading body, if there's any. #423 +* [ENHANCEMENT] Make httpgrpc.Server produce non-loggable errors when a header with key `httpgrpc.DoNotLogErrorHeaderKey` and any value is present in the HTTP response. #421 * [BUGFIX] spanlogger: Support multiple tenant IDs. #59 * [BUGFIX] Memberlist: fixed corrupted packets when sending compound messages with more than 255 messages or messages bigger than 64KB. #85 * [BUGFIX] Ring: `ring_member_ownership_percent` and `ring_tokens_owned` metrics are not updated on scale down. #109 diff --git a/httpgrpc/server/server.go b/httpgrpc/server/server.go index 34c4f1b6e..c2ee26517 100644 --- a/httpgrpc/server/server.go +++ b/httpgrpc/server/server.go @@ -28,6 +28,9 @@ import ( ) var ( + // DoNotLogErrorHeaderKey is a header key used for marking non-loggable errors. More precisely, if an HTTP response + // has a status code 5xx, and contains a header with key DoNotLogErrorHeaderKey and any values, the generated error + // will be marked as non-loggable. DoNotLogErrorHeaderKey = http.CanonicalHeaderKey("X-DoNotLogError") ) @@ -75,6 +78,7 @@ func (s Server) Handle(ctx context.Context, r *httpgrpc.HTTPRequest) (*httpgrpc. if recorder.Code/100 == 5 { err := httpgrpc.ErrorFromHTTPResponse(resp) if _, ok := header[DoNotLogErrorHeaderKey]; ok { + recorder.Header().Del(DoNotLogErrorHeaderKey) err = middleware.DoNotLogError{Err: err} } return nil, err @@ -236,6 +240,9 @@ func toHeader(hs []*httpgrpc.Header, header http.Header) { func fromHeader(hs http.Header) []*httpgrpc.Header { result := make([]*httpgrpc.Header, 0, len(hs)) for k, vs := range hs { + if k == DoNotLogErrorHeaderKey { + continue + } result = append(result, &httpgrpc.Header{ Key: k, Values: vs, diff --git a/httpgrpc/server/server_test.go b/httpgrpc/server/server_test.go index 3bc7f72de..6ec8d5bdf 100644 --- a/httpgrpc/server/server_test.go +++ b/httpgrpc/server/server_test.go @@ -7,6 +7,7 @@ package server import ( "bytes" "context" + "errors" "fmt" "net" "net/http" @@ -101,13 +102,87 @@ func TestError(t *testing.T) { assert.Equal(t, "foo\n", recorder.Body.String()) assert.Equal(t, 500, recorder.Code) - if doNotLog { - assert.Equal(t, recorder.Header().Get(DoNotLogErrorHeaderKey), "true") + assert.NotContains(t, recorder.Header(), DoNotLogErrorHeaderKey) + }) + } +} + +func TestServerHandleDoNotLogError(t *testing.T) { + testCases := map[string]struct { + errorCode int + doNotLogError bool + expectedError bool + }{ + "HTTPResponse with code 5xx and with DoNotLogError header should return a non-loggable error": { + errorCode: http.StatusInternalServerError, + doNotLogError: true, + expectedError: true, + }, + "HTTPResponse with code 5xx and without DoNotLogError header should return a loggable error": { + errorCode: http.StatusInternalServerError, + expectedError: true, + }, + "HTTPResponse with code different from 5xx and with DoNotLogError header should not return an error": { + errorCode: http.StatusBadRequest, + doNotLogError: true, + expectedError: false, + }, + "HTTPResponse with code different from 5xx and without DoNotLogError header should not return an error": { + errorCode: http.StatusBadRequest, + expectedError: false, + }, + } + errMsg := "this is an error" + for testName, testData := range testCases { + t.Run(testName, func(t *testing.T) { + h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if testData.doNotLogError { + w.Header().Set(DoNotLogErrorHeaderKey, "true") + } + http.Error(w, errMsg, testData.errorCode) + }) + + s := NewServer(h) + req := &httpgrpc.HTTPRequest{ + Method: "GET", + Url: "/test", + } + resp, err := s.Handle(context.Background(), req) + if testData.expectedError { + require.Error(t, err) + require.Nil(t, resp) + var optional middleware.OptionalLogging + if testData.doNotLogError { + require.ErrorAs(t, err, &optional) + require.False(t, optional.ShouldLog(context.Background(), 0)) + } else { + require.False(t, errors.As(err, &optional)) + } + checkError(t, err, testData.errorCode, errMsg) + } else { + require.NoError(t, err) + require.NotNil(t, resp) + checkHTTPResponse(t, resp, testData.errorCode, errMsg) } }) } } +func checkError(t *testing.T, err error, expectedCode int, expectedMessage string) { + resp, ok := httpgrpc.HTTPResponseFromError(err) + require.True(t, ok) + checkHTTPResponse(t, resp, expectedCode, expectedMessage) +} + +func checkHTTPResponse(t *testing.T, resp *httpgrpc.HTTPResponse, expectedCode int, expectedBody string) { + require.Equal(t, int32(expectedCode), resp.GetCode()) + require.Equal(t, fmt.Sprintf("%s\n", expectedBody), string(resp.GetBody())) + hs := resp.GetHeaders() + for _, h := range hs { + require.NotEqual(t, DoNotLogErrorHeaderKey, h.Key) + } +} + func TestParseURL(t *testing.T) { for _, tc := range []struct { input string From 9145a20d1f3e571e1c054bd534a2e5a8d62fc2e0 Mon Sep 17 00:00:00 2001 From: Yuri Nikolic Date: Tue, 31 Oct 2023 13:48:46 +0100 Subject: [PATCH 4/4] Fixing review findings 3 Signed-off-by: Yuri Nikolic --- httpgrpc/server/server.go | 1 - 1 file changed, 1 deletion(-) diff --git a/httpgrpc/server/server.go b/httpgrpc/server/server.go index c2ee26517..35656c943 100644 --- a/httpgrpc/server/server.go +++ b/httpgrpc/server/server.go @@ -78,7 +78,6 @@ func (s Server) Handle(ctx context.Context, r *httpgrpc.HTTPRequest) (*httpgrpc. if recorder.Code/100 == 5 { err := httpgrpc.ErrorFromHTTPResponse(resp) if _, ok := header[DoNotLogErrorHeaderKey]; ok { - recorder.Header().Del(DoNotLogErrorHeaderKey) err = middleware.DoNotLogError{Err: err} } return nil, err