From ff16e0f6dfe23a69561ddf00cd88270abc7fd271 Mon Sep 17 00:00:00 2001 From: Nick Craig-Wood Date: Wed, 14 Oct 2015 17:37:53 +0100 Subject: [PATCH] Factor common error handling into fs module --- amazonclouddrive/amazonclouddrive.go | 25 +------ drive/drive.go | 31 +++----- fs/error.go | 102 +++++++++++++++++++++++++++ fs/fs.go | 48 ------------- 4 files changed, 114 insertions(+), 92 deletions(-) create mode 100644 fs/error.go diff --git a/amazonclouddrive/amazonclouddrive.go b/amazonclouddrive/amazonclouddrive.go index 5687f9a36..a4b665336 100644 --- a/amazonclouddrive/amazonclouddrive.go +++ b/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 - 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 + return fs.ShouldRetry(err) || fs.ShouldRetryHTTP(resp, retryErrorCodes), err } // NewFs constructs an FsAcd from the path, container:path diff --git a/drive/drive.go b/drive/drive.go index 7281f14ea..1cf55a4b1 100644 --- a/drive/drive.go +++ b/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 x, ok := err.(interface { - Timeout() bool - }); ok && x.Timeout() { + if fs.ShouldRetry(err) { again = true - } - // Check for net error Temporary() - if x, ok := err.(interface { - Temporary() bool - }); ok && x.Temporary() { - 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" { + } else { + 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 + } } } } diff --git a/fs/error.go b/fs/error.go new file mode 100644 index 000000000..820dd64b0 --- /dev/null +++ b/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 +} diff --git a/fs/fs.go b/fs/fs.go index 639b0ba34..1d755ba62 100644 --- a/fs/fs.go +++ b/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