Skip to content

Commit

Permalink
storage: fix retry strategy to unwrap errors (#4019)
Browse files Browse the repository at this point in the history
I updated retry strategy in google-cloud-go to follow what [google-api-go-client](https://github.com/googleapis/google-api-go-client/blob/e8cf7f70a1e0160405dd0f789a006e6b6074fbdb/internal/gensupport/resumable.go#L233-L258) by unwrapping errors which can be unwrapped. 

For testing, I followed wrapping guidance in [godocs](https://golang.org/pkg/errors/#pkg-overview) to verify the change works as expected.

Thank you in advance for your review.

Fixes: #3963
  • Loading branch information
frankyn committed May 3, 2021
1 parent a297278 commit c3cb0d4
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 4 deletions.
1 change: 1 addition & 0 deletions storage/go.mod
Expand Up @@ -8,6 +8,7 @@ require (
github.com/google/go-cmp v0.5.5
github.com/googleapis/gax-go/v2 v2.0.5
golang.org/x/oauth2 v0.0.0-20210413134643-5e61552d6c78
golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1
google.golang.org/api v0.45.0
google.golang.org/genproto v0.0.0-20210427215850-f767ed18ee4d
google.golang.org/grpc v1.37.0
Expand Down
12 changes: 8 additions & 4 deletions storage/go110.go
Expand Up @@ -43,10 +43,14 @@ func shouldRetry(err error) bool {
return true
}
}
return false
case interface{ Temporary() bool }:
return e.Temporary()
default:
return false
if e.Temporary() {
return true
}
}
// Unwrap is only supported in go1.13.x+
if e, ok := err.(interface{ Unwrap() error }); ok {
return shouldRetry(e.Unwrap())
}
return false
}
4 changes: 4 additions & 0 deletions storage/go110_test.go
Expand Up @@ -23,6 +23,8 @@ import (
"net/url"
"testing"

"golang.org/x/xerrors"

"google.golang.org/api/googleapi"
)

Expand All @@ -45,6 +47,8 @@ func TestInvoke(t *testing.T) {
{2, &googleapi.Error{Code: 599}, &googleapi.Error{Code: 428}},
{1, &url.Error{Op: "blah", URL: "blah", Err: errors.New("connection refused")}, nil},
{1, io.ErrUnexpectedEOF, nil},
{1, xerrors.Errorf("Test unwrapping of a temporary error: %w", &googleapi.Error{Code: 500}), nil},
{0, xerrors.Errorf("Test unwrapping of a non-retriable error: %w", &googleapi.Error{Code: 400}), &googleapi.Error{Code: 400}},
} {
counter := 0
call := func() error {
Expand Down

0 comments on commit c3cb0d4

Please sign in to comment.