Skip to content

Commit

Permalink
fix(storage): retry net.OpError on connection reset (googleapis#10154)
Browse files Browse the repository at this point in the history
We are seeing these errors surfaced via net.OpError as well as
url.Error. Update the ShouldRetry function accordingly.

Also, use net.ErrClosed sentinel over string matching.

Fixes googleapis#9478
  • Loading branch information
tritone committed May 13, 2024
1 parent 7c52978 commit 54fab10
Show file tree
Hide file tree
Showing 2 changed files with 11 additions and 9 deletions.
10 changes: 4 additions & 6 deletions storage/invoke.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,16 @@ func ShouldRetry(err error) bool {
if errors.Is(err, io.ErrUnexpectedEOF) {
return true
}
if errors.Is(err, net.ErrClosed) {
return true
}

switch e := err.(type) {
case *net.OpError:
if strings.Contains(e.Error(), "use of closed network connection") {
// TODO: check against net.ErrClosed (go 1.16+) instead of string
return true
}
case *googleapi.Error:
// Retry on 408, 429, and 5xx, according to
// https://cloud.google.com/storage/docs/exponential-backoff.
return e.Code == 408 || e.Code == 429 || (e.Code >= 500 && e.Code < 600)
case *url.Error:
case *net.OpError, *url.Error:
// Retry socket-level errors ECONNREFUSED and ECONNRESET (from syscall).
// Unfortunately the error type is unexported, so we resort to string
// matching.
Expand Down
10 changes: 7 additions & 3 deletions storage/invoke_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,11 @@ func TestShouldRetry(t *testing.T) {
inputErr: &url.Error{Op: "blah", URL: "blah", Err: errors.New("connection refused")},
shouldRetry: true,
},
{
desc: "net.OpError{Err: errors.New(\"connection reset by peer\")}",
inputErr: &net.OpError{Op: "blah", Net: "tcp", Err: errors.New("connection reset by peer")},
shouldRetry: true,
},
{
desc: "io.ErrUnexpectedEOF",
inputErr: io.ErrUnexpectedEOF,
Expand Down Expand Up @@ -382,9 +387,8 @@ func TestShouldRetry(t *testing.T) {
shouldRetry: false,
},
{
desc: "wrapped ErrClosed text",
// TODO: check directly against wrapped net.ErrClosed (go 1.16+)
inputErr: &net.OpError{Op: "write", Err: errors.New("use of closed network connection")},
desc: "wrapped net.ErrClosed",
inputErr: &net.OpError{Err: net.ErrClosed},
shouldRetry: true,
},
} {
Expand Down

0 comments on commit 54fab10

Please sign in to comment.