Skip to content

Commit

Permalink
feat(storage): returned wrapped error for timeouts
Browse files Browse the repository at this point in the history
This piggybacks on googleapis#4797 to allow storage to expose wrapped
service errors when a call retries without success until timeout
or cancellation.

I also updated all checks for context sentinel errors in storage
to use xerrors.Is instead of strict equality. Users of this
package should do the same. I'll update the documentation on
errors from this package in a subsequent PR.

We will have to bump the dependency on the root module before
merging this change I believe.

Fixes googleapis#4197
  • Loading branch information
tritone committed Sep 23, 2021
1 parent ce5f4db commit 45947e1
Show file tree
Hide file tree
Showing 3 changed files with 6 additions and 7 deletions.
8 changes: 3 additions & 5 deletions storage/integration_test.go
Expand Up @@ -3206,14 +3206,12 @@ func TestIntegration_CancelWrite(t *testing.T) {
cancel()
// The next Write should return context.Canceled.
_, err = w.Write(buf)
// TODO: Once we drop support for Go versions < 1.13, use errors.Is() to
// check for context cancellation instead.
if err != context.Canceled && !strings.Contains(err.Error(), "context canceled") {
if !xerrors.Is(err, context.Canceled) {
t.Fatalf("got %v, wanted context.Canceled", err)
}
// The Close should too.
err = w.Close()
if err != context.Canceled && !strings.Contains(err.Error(), "context canceled") {
if !xerrors.Is(err, context.Canceled) {
t.Fatalf("got %v, wanted context.Canceled", err)
}
}
Expand Down Expand Up @@ -3865,7 +3863,7 @@ func TestIntegration_ReaderCancel(t *testing.T) {
buf := make([]byte, 1000)
_, readErr = r.Read(buf)
if readErr != nil {
if readErr == context.Canceled {
if xerrors.Is(readErr, context.Canceled) {
return
}
break
Expand Down
2 changes: 1 addition & 1 deletion storage/invoke.go
Expand Up @@ -36,7 +36,7 @@ func runWithRetry(ctx context.Context, call func() error) error {
return true, nil
}
if shouldRetry(err) {
return false, nil
return false, err
}
return true, err
})
Expand Down
3 changes: 2 additions & 1 deletion storage/writer.go
Expand Up @@ -24,6 +24,7 @@ import (
"unicode/utf8"

"github.com/golang/protobuf/proto"
"golang.org/x/xerrors"
"google.golang.org/api/googleapi"
raw "google.golang.org/api/storage/v1"
storagepb "google.golang.org/genproto/googleapis/storage/v2"
Expand Down Expand Up @@ -217,7 +218,7 @@ func (w *Writer) Write(p []byte) (n int, err error) {
// Preserve existing functionality that when context is canceled, Write will return
// context.Canceled instead of "io: read/write on closed pipe". This hides the
// pipe implementation detail from users and makes Write seem as though it's an RPC.
if werr == context.Canceled || werr == context.DeadlineExceeded {
if xerrors.Is(werr, context.Canceled) || xerrors.Is(werr, context.DeadlineExceeded){
return n, werr
}
}
Expand Down

0 comments on commit 45947e1

Please sign in to comment.