Factor common error handling into fs module

amazonclouddrive/amazonclouddrive.go

@@ -16,9 +16,7 @@ import (
 	"fmt"
 	"io"
 	"log"
-	"net"
 	"net/http"
-	"net/url"
 	"regexp"
 	"strings"
 	"sync"
@@ -134,28 +132,7 @@ var retryErrorCodes = []int{
 // shouldRetry returns a boolean as to whether this resp and err
 // deserve to be retried.  It returns the err as a convenience
 func shouldRetry(resp *http.Response, err error) (bool, error) {
-	// See if HTTP error code is to be retried
+	return fs.ShouldRetry(err) || fs.ShouldRetryHTTP(resp, retryErrorCodes), err
-	if err != nil && resp != nil {
-		for _, e := range retryErrorCodes {
-			if resp.StatusCode == e {
-				return true, err
-			}
-		}
-	}
-
-	// Allow retry if request times out. Adapted from
-	// http://stackoverflow.com/questions/23494950/specifically-check-for-timeout-error
-	switch err := err.(type) {
-	case *url.Error:
-		if err, ok := err.Err.(net.Error); ok && err.Timeout() {
-			return true, err
-		}
-	case net.Error:
-		if err.Timeout() {
-			return true, err
-		}
-	}
-	return false, err
 }
 
 // NewFs constructs an FsAcd from the path, container:path

drive/drive.go

@@ -124,29 +124,20 @@ func (f *FsDrive) String() string {
 // shouldRetry determines whehter a given err rates being retried
 func shouldRetry(err error) (again bool, errOut error) {
 	again = false
-	errOut = err
 	if err != nil {
-		// Check for net error Timeout()
+		if fs.ShouldRetry(err) {
-		if x, ok := err.(interface {
-			Timeout() bool
-		}); ok && x.Timeout() {
 			again = true
-		}
+		} else {
-		// Check for net error Temporary()
+			switch gerr := err.(type) {
-		if x, ok := err.(interface {
+			case *googleapi.Error:
-			Temporary() bool
+				if gerr.Code >= 500 && gerr.Code < 600 {
-		}); ok && x.Temporary() {
+					// All 5xx errors should be retried
-			again = true
-		}
-		switch gerr := err.(type) {
-		case *googleapi.Error:
-			if gerr.Code >= 500 && gerr.Code < 600 {
-				// All 5xx errors should be retried
-				again = true
-			} else if len(gerr.Errors) > 0 {
-				reason := gerr.Errors[0].Reason
-				if reason == "rateLimitExceeded" || reason == "userRateLimitExceeded" {
 					again = true
+				} else if len(gerr.Errors) > 0 {
+					reason := gerr.Errors[0].Reason
+					if reason == "rateLimitExceeded" || reason == "userRateLimitExceeded" {
+						again = true
+					}
 				}
 			}
 		}

fs/error.go

@@ -0,0 +1,102 @@
+// Errors and error handling
+
+package fs
+
+import (
+	"fmt"
+	"net/http"
+	"net/url"
+)
+
+// Retry is an optional interface for error as to whether the
+// operation should be retried at a high level.
+//
+// This should be returned from Update or Put methods as required
+type Retry interface {
+	error
+	Retry() bool
+}
+
+// retryError is a type of error
+type retryError string
+
+// Error interface
+func (r retryError) Error() string {
+	return string(r)
+}
+
+// Retry interface
+func (r retryError) Retry() bool {
+	return true
+}
+
+// Check interface
+var _ Retry = retryError("")
+
+// RetryErrorf makes an error which indicates it would like to be retried
+func RetryErrorf(format string, a ...interface{}) error {
+	return retryError(fmt.Sprintf(format, a...))
+}
+
+// PlainRetryError is an error wrapped so it will retry
+type plainRetryError struct {
+	error
+}
+
+// Retry interface
+func (err plainRetryError) Retry() bool {
+	return true
+}
+
+// Check interface
+var _ Retry = plainRetryError{(error)(nil)}
+
+// RetryError makes an error which indicates it would like to be retried
+func RetryError(err error) error {
+	return plainRetryError{err}
+}
+
+// ShouldRetry looks at an error and tries to work out if retrying the
+// operation that caused it would be a good idea. It returns true if
+// the error implements Timeout() or Temporary() and it returns true.
+func ShouldRetry(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	// Unwrap url.Error
+	if urlErr, ok := err.(*url.Error); ok {
+		err = urlErr.Err
+	}
+
+	// Check for net error Timeout()
+	if x, ok := err.(interface {
+		Timeout() bool
+	}); ok && x.Timeout() {
+		return true
+	}
+
+	// Check for net error Temporary()
+	if x, ok := err.(interface {
+		Temporary() bool
+	}); ok && x.Temporary() {
+		return true
+	}
+
+	return false
+}
+
+// ShouldRetryHTTP returns a boolean as to whether this resp deserves.
+// It checks to see if the HTTP response code is in the slice
+// retryErrorCodes.
+func ShouldRetryHTTP(resp *http.Response, retryErrorCodes []int) bool {
+	if resp == nil {
+		return false
+	}
+	for _, e := range retryErrorCodes {
+		if resp.StatusCode == e {
+			return true
+		}
+	}
+	return false
+}

fs/fs.go

@@ -197,54 +197,6 @@ type DirMover interface {
 	DirMove(src Fs) error
 }
 
-// Retry is optional interface for error as to whether the operation
-// should be retried at a high level.
-//
-// This should be returned from Update or Put methods as required
-type Retry interface {
-	error
-	Retry() bool
-}
-
-// retryError is a type of error
-type retryError string
-
-// Error interface
-func (r retryError) Error() string {
-	return string(r)
-}
-
-// Retry interface
-func (r retryError) Retry() bool {
-	return true
-}
-
-// Check interface
-var _ Retry = retryError("")
-
-// RetryErrorf makes an error which indicates it would like to be retried
-func RetryErrorf(format string, a ...interface{}) error {
-	return retryError(fmt.Sprintf(format, a...))
-}
-
-// PlainRetryError is an error wrapped so it will retry
-type plainRetryError struct {
-	error
-}
-
-// Retry interface
-func (err plainRetryError) Retry() bool {
-	return true
-}
-
-// Check interface
-var _ Retry = plainRetryError{(error)(nil)}
-
-// RetryError makes an error which indicates it would like to be retried
-func RetryError(err error) error {
-	return plainRetryError{err}
-}
-
 // ObjectsChan is a channel of Objects
 type ObjectsChan chan Object