Skip to content

Commit

Permalink
fix(internal/retry): never return nil from GRPCStatus() (#8128)
Browse files Browse the repository at this point in the history
An error's GRPCStatus() method should never returns nil, which maps to a status with Code.OK (https://togithub.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://togithub.com/googleapis/google-cloud-go/issues/8127.
  • Loading branch information
atollena authored Jun 17, 2023
1 parent b429aa1 commit 005d2df
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 1 deletion.
3 changes: 2 additions & 1 deletion internal/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"time"

gax "github.com/googleapis/gax-go/v2"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"
)

Expand Down Expand Up @@ -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())
}
26 changes: 26 additions & 0 deletions internal/retry_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

0 comments on commit 005d2df

Please sign in to comment.