Add connection timeout retries to item retry handler (#4706)

We were missing a retry for connection timeout issues. Adding it.

---

#### Does this PR need a docs update or release note?

- [x] ✅ Yes, it's included
- [ ] 🕐 Yes, but in a later PR
- [ ] ⛔ No

#### Type of change

<!--- Please check the type of change your PR introduces: --->
- [ ] 🌻 Feature
- [x] 🐛 Bugfix
- [ ] 🗺️ Documentation
- [ ] 🤖 Supportability/Tests
- [ ] 💻 CI/Deployment
- [ ] 🧹 Tech Debt/Cleanup

#### Issue(s)

<!-- Can reference multiple issues. Use one of the following "magic words" - "closes, fixes" to auto-close the Github issue. -->
* #<issue>

#### Test Plan

<!-- How will this be tested prior to merging.-->
- [ ] 💪 Manual
- [x] ⚡ Unit test
- [ ] 💚 E2E

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
 

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) {

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",
+			name: "Chained Errors With No Retriables",
-			err:    clues.Stack(assert.AnError, clues.New("another error")),
+			err: func() error {
+				return clues.Stack(assert.AnError, clues.New("another error"))
+			},
 			expect: false,
 		},
 		{
-			name:   "Chained Errors With ECONNRESET",
+			name: "Chained Errors With ECONNRESET",
-			err:    clues.Stack(assert.AnError, syscall.ECONNRESET, assert.AnError),
+			err: func() error {
+				return clues.Stack(assert.AnError, syscall.ECONNRESET, assert.AnError)
+			},
 			expect: true,
 		},
 		{
-			name:   "Chained Errors With ErrUnexpectedEOF",
+			name: "Chained Errors With ErrUnexpectedEOF",
-			err:    clues.Stack(assert.AnError, io.ErrUnexpectedEOF, assert.AnError),
+			err: func() error {
+				return clues.Stack(assert.AnError, io.ErrUnexpectedEOF, assert.AnError)
+			},
 			expect: true,
 		},
 		{
-			name:   "Wrapped ECONNRESET Error",
+			name: "Wrapped ECONNRESET Error",
-			err:    clues.Wrap(syscall.ECONNRESET, "wrapped error"),
+			err: func() error {
+				return clues.Wrap(syscall.ECONNRESET, "wrapped error")
+			},
 			expect: true,
 		},
 		{
-			name:   "Wrapped ErrUnexpectedEOF Error",
+			name: "Wrapped ErrUnexpectedEOF Error",
-			err:    clues.Wrap(io.ErrUnexpectedEOF, "wrapped 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()))
 		})
 	}
 }