From 5d542d398f2c7e32c121798d35cbf2914dbd1a20 Mon Sep 17 00:00:00 2001 From: Jakub Mikians Date: Sun, 11 Jun 2017 09:55:14 +0200 Subject: [PATCH] Move retry logic to errors.IsRetrialbeError Also added unit tests. --- errors/types.go | 4 ++++ errors/types_test.go | 46 ++++++++++++++++++++++++++++++++++++++++++++ tq/transfer_queue.go | 8 +------- 3 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 errors/types_test.go diff --git a/errors/types.go b/errors/types.go index e4fafd85..efeca36c 100644 --- a/errors/types.go +++ b/errors/types.go @@ -2,6 +2,7 @@ package errors import ( "fmt" + "net/url" "github.com/pkg/errors" ) @@ -134,6 +135,9 @@ func IsRetriableError(err error) bool { }); ok { return e.RetriableError() } + if cause, ok := Cause(err).(*url.Error); ok { + return cause.Temporary() || cause.Timeout() + } if parent := parentOf(err); parent != nil { return IsRetriableError(parent) } diff --git a/errors/types_test.go b/errors/types_test.go new file mode 100644 index 00000000..a71ab9f1 --- /dev/null +++ b/errors/types_test.go @@ -0,0 +1,46 @@ +package errors_test + +import ( + "net/url" + "testing" + + "github.com/git-lfs/git-lfs/errors" + "github.com/stretchr/testify/assert" +) + +type TemporaryError struct { +} + +func (e TemporaryError) Error() string { + return "" +} + +func (e TemporaryError) Temporary() bool { + return true +} + +type TimeoutError struct { +} + +func (e TimeoutError) Error() string { + return "" +} + +func (e TimeoutError) Timeout() bool { + return true +} + +func TestCanRetryOnTemporaryError(t *testing.T) { + err := &url.Error{Err: TemporaryError{}} + assert.True(t, errors.IsRetriableError(err)) +} + +func TestCanRetryOnTimeoutError(t *testing.T) { + err := &url.Error{Err: TimeoutError{}} + assert.True(t, errors.IsRetriableError(err)) +} + +func TestCannotRetryOnGenericUrlError(t *testing.T) { + err := &url.Error{Err: errors.New("")} + assert.False(t, errors.IsRetriableError(err)) +} diff --git a/tq/transfer_queue.go b/tq/transfer_queue.go index 89af6aef..3180d030 100644 --- a/tq/transfer_queue.go +++ b/tq/transfer_queue.go @@ -1,7 +1,6 @@ package tq import ( - "net/url" "os" "sort" "sync" @@ -634,12 +633,7 @@ func (q *TransferQueue) run() { // canRetry returns whether or not the given error "err" is retriable. func (q *TransferQueue) canRetry(err error) bool { - switch cause := errors.Cause(err).(type) { - case *url.Error: - return cause.Temporary() || cause.Timeout() - default: - return errors.IsRetriableError(err) - } + return errors.IsRetriableError(err) } // canRetryObject returns whether the given error is retriable for the object