Skip to content

Commit

Permalink
internal/retry: Never return nil from GRPCStatus()
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://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 #8127.
  • Loading branch information
atollena committed Jun 16, 2023
1 parent 20573e2 commit 04b8155
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 @@ -17,6 +17,7 @@ package internal
import (
"context"
"fmt"
"google.golang.org/grpc/codes"
"time"

gax "github.com/googleapis/gax-go/v2"
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 04b8155

Please sign in to comment.