Skip to content

Commit e8b224a

Browse files
authored
fix(internal/retry): Simplify gRPC status code mapping of retry error (#8196)
gRPC v1.55.0 that this project depends on came with a change in semantic for status.FromError such that the gRPC status of wrapped errors is returned. The implementation of GRPCStatus in internal/retry code was doing just that. Removing GRPCStatus entirely makes the code much easier to understand: at the moment, status.FromError calls GRPCStatus which in turns calls status.FromError and it's not immediately obvious why this can't result in infinite recursion. A previous change I made to this code made sure this method never returns Status.OK (https://github.com/googleapis/google-cloud-go/pull/8128), but I failed to realize that since this project depends on gRPC 1.55 that already handles wrapped errors in status.FromError, we can simply remove the implementation of `GRPCStatus` and let gRPC status.FromError handle unwrapping. Note that I barely had to change the tests, but there *IS* a slight change in behavior: the message of the wrapping error is included in the gRPC error. I think it's fine, bet let me know if you think otherwise (for the same reasons discussed in https://github.com/grpc/grpc-go/pull/6150).
1 parent afdf772 commit e8b224a

File tree

2 files changed

+2
-15
lines changed

2 files changed

+2
-15
lines changed

internal/retry.go

-10
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,6 @@ import (
2020
"time"
2121

2222
gax "github.com/googleapis/gax-go/v2"
23-
"google.golang.org/grpc/codes"
24-
"google.golang.org/grpc/status"
2523
)
2624

2725
// Retry calls the supplied function f repeatedly according to the provided
@@ -76,11 +74,3 @@ func (e wrappedCallErr) Unwrap() error {
7674
func (e wrappedCallErr) Is(err error) bool {
7775
return e.ctxErr == err || e.wrappedErr == err
7876
}
79-
80-
// GRPCStatus allows the wrapped error to be used with status.FromError.
81-
func (e wrappedCallErr) GRPCStatus() *status.Status {
82-
if s, ok := status.FromError(e.wrappedErr); ok {
83-
return s
84-
}
85-
return status.New(codes.Unknown, e.Error())
86-
}

internal/retry_test.go

+2-5
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ func TestRetryPreserveError(t *testing.T) {
8888
if g, w := got.Code(), codes.NotFound; g != w {
8989
t.Errorf("got code %v, want %v", g, w)
9090
}
91-
wantMessage := "not found"
91+
wantMessage := "retry failed with context deadline exceeded; last error: rpc error: code = NotFound desc = not found"
9292
if g, w := got.Message(), wantMessage; g != w {
9393
t.Errorf("got message %q, want %q", g, w)
9494
}
@@ -111,10 +111,7 @@ func TestRetryWrapsErrorWithStatusUnknown(t *testing.T) {
111111
if g, w := err.Error(), wantError; g != w {
112112
t.Errorf("got error %q, want %q", g, w)
113113
}
114-
got, ok := status.FromError(err)
115-
if !ok {
116-
t.Fatal("expected error to implement a gRPC status")
117-
}
114+
got, _ := status.FromError(err)
118115
if g, w := got.Code(), codes.Unknown; g != w {
119116
t.Errorf("got code %v, want %v", g, w)
120117
}

0 commit comments

Comments
 (0)