diff --git a/CHANGELOG.md b/CHANGELOG.md index 39171e240..3256569d3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Handle OneDrive folders being deleted and recreated midway through a backup - Automatically re-run a full delta query on incremental if the prior backup is found to have malformed prior-state information. - Retry drive item permission downloads during long-running backups after the jwt token expires and refreshes. +- Retry item downloads during connection timeouts. ## [v0.15.0] (beta) - 2023-10-31 diff --git a/src/internal/common/readers/retry_handler.go b/src/internal/common/readers/retry_handler.go index cf660d580..a10629502 100644 --- a/src/internal/common/readers/retry_handler.go +++ b/src/internal/common/readers/retry_handler.go @@ -5,6 +5,9 @@ import ( "errors" "fmt" "io" + "net/http" + "net/url" + "os" "syscall" "time" @@ -31,6 +34,18 @@ var retryErrs = []error{ io.ErrUnexpectedEOF, } +// TODO(pandeyabs): Consolidate error funcs with retryErrs slice. +func isTimeoutErr(err error) bool { + switch err := err.(type) { + case *url.Error: + return err.Timeout() + } + + return errors.Is(err, os.ErrDeadlineExceeded) || + errors.Is(err, http.ErrHandlerTimeout) || + os.IsTimeout(err) +} + type Getter interface { // SupportsRange returns true if this Getter supports adding Range headers to // the Get call. Otherwise returns false. @@ -91,7 +106,8 @@ func isRetriable(err error) bool { } } - return false + // Retry on read connection timeouts. + return isTimeoutErr(err) } func (rrh *resetRetryHandler) Read(p []byte) (int, error) { diff --git a/src/internal/common/readers/retry_handler_test.go b/src/internal/common/readers/retry_handler_test.go index 89376c22f..8f74455f1 100644 --- a/src/internal/common/readers/retry_handler_test.go +++ b/src/internal/common/readers/retry_handler_test.go @@ -4,6 +4,10 @@ import ( "bytes" "context" "io" + "net" + "net/http" + "net/url" + "os" "syscall" "testing" @@ -510,52 +514,91 @@ func (suite *ResetRetryHandlerUnitSuite) TestResetRetryHandler() { func (suite *ResetRetryHandlerUnitSuite) TestIsRetriable() { table := []struct { name string - err error + err func() error expect bool }{ { name: "nil", - err: nil, + err: func() error { return nil }, expect: false, }, { name: "Connection Reset Error", - err: syscall.ECONNRESET, + err: func() error { return syscall.ECONNRESET }, expect: true, }, { name: "Unexpected EOF Error", - err: io.ErrUnexpectedEOF, + err: func() error { return io.ErrUnexpectedEOF }, expect: true, }, { name: "Not Retriable Error", - err: assert.AnError, + err: func() error { return assert.AnError }, expect: false, }, { - name: "Chained Errors With No Retriables", - err: clues.Stack(assert.AnError, clues.New("another error")), + name: "Chained Errors With No Retriables", + err: func() error { + return clues.Stack(assert.AnError, clues.New("another error")) + }, expect: false, }, { - name: "Chained Errors With ECONNRESET", - err: clues.Stack(assert.AnError, syscall.ECONNRESET, assert.AnError), + name: "Chained Errors With ECONNRESET", + err: func() error { + return clues.Stack(assert.AnError, syscall.ECONNRESET, assert.AnError) + }, expect: true, }, { - name: "Chained Errors With ErrUnexpectedEOF", - err: clues.Stack(assert.AnError, io.ErrUnexpectedEOF, assert.AnError), + name: "Chained Errors With ErrUnexpectedEOF", + err: func() error { + return clues.Stack(assert.AnError, io.ErrUnexpectedEOF, assert.AnError) + }, expect: true, }, { - name: "Wrapped ECONNRESET Error", - err: clues.Wrap(syscall.ECONNRESET, "wrapped error"), + name: "Wrapped ECONNRESET Error", + err: func() error { + return clues.Wrap(syscall.ECONNRESET, "wrapped error") + }, expect: true, }, { - name: "Wrapped ErrUnexpectedEOF Error", - err: clues.Wrap(io.ErrUnexpectedEOF, "wrapped error"), + name: "Wrapped ErrUnexpectedEOF Error", + err: func() error { + return clues.Wrap(io.ErrUnexpectedEOF, "wrapped error") + }, + expect: true, + }, + { + name: "Timeout - http timeout", + err: func() error { return http.ErrHandlerTimeout }, + expect: true, + }, + { + name: "Timeout - url error", + err: func() error { + return &url.Error{Err: os.ErrDeadlineExceeded} + }, + expect: true, + }, + { + name: "Timeout - OS timeout", + err: func() error { + return &os.PathError{Err: os.ErrDeadlineExceeded} + }, + expect: true, + }, + { + name: "Timeout - net timeout", + err: func() error { + return &net.OpError{ + Op: "read", + Err: &os.PathError{Err: os.ErrDeadlineExceeded}, + } + }, expect: true, }, } @@ -564,7 +607,7 @@ func (suite *ResetRetryHandlerUnitSuite) TestIsRetriable() { suite.Run(test.name, func() { t := suite.T() - assert.Equal(t, test.expect, isRetriable(test.err)) + assert.Equal(t, test.expect, isRetriable(test.err())) }) } }