From 04b8155701f1c27c0138b341907e2f4665375a12 Mon Sep 17 00:00:00 2001 From: Antoine Tollenaere Date: Fri, 16 Jun 2023 11:50:33 +0200 Subject: [PATCH] internal/retry: Never return nil from GRPCStatus() An error's GRPCStatus() method should never returns nil, which maps to a status with Code.OK (https://github.com/grpc/grpc-go/blob/642dd63a85275a96d172f446911fd04111e2c74c/internal/status/status.go#L71-L76). Instead it should return an actual error code, such as Status.Unknown. This addresses https://github.com/googleapis/google-cloud-go/issues/8127. --- internal/retry.go | 3 ++- internal/retry_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/internal/retry.go b/internal/retry.go index 2943a6d0b457..fce0c48ed21a 100644 --- a/internal/retry.go +++ b/internal/retry.go @@ -17,6 +17,7 @@ package internal import ( "context" "fmt" + "google.golang.org/grpc/codes" "time" gax "github.com/googleapis/gax-go/v2" @@ -81,5 +82,5 @@ func (e wrappedCallErr) GRPCStatus() *status.Status { if s, ok := status.FromError(e.wrappedErr); ok { return s } - return nil + return status.New(codes.Unknown, e.Error()) } diff --git a/internal/retry_test.go b/internal/retry_test.go index 771cb26ffca4..1c26dcbcc394 100644 --- a/internal/retry_test.go +++ b/internal/retry_test.go @@ -93,3 +93,29 @@ func TestRetryPreserveError(t *testing.T) { t.Errorf("got message %q, want %q", g, w) } } + +func TestRetryWrapsErrorWithStatusUnknown(t *testing.T) { + // When retrying on an error that is not a grpc error, make sure to return + // a valid gRPC status. + err := retry(context.Background(), gax.Backoff{}, + func() (bool, error) { + return false, errors.New("test error") + }, + func(context.Context, time.Duration) error { + return context.DeadlineExceeded + }) + if err == nil { + t.Fatalf("unexpectedly got nil error") + } + wantError := "retry failed with context deadline exceeded; last error: test error" + if g, w := err.Error(), wantError; g != w { + t.Errorf("got error %q, want %q", g, w) + } + got, ok := status.FromError(err) + if !ok { + t.Fatal("expected error to implement a gRPC status") + } + if g, w := got.Code(), codes.Unknown; g != w { + t.Errorf("got code %v, want %v", g, w) + } +}