From 69acb28d616660c1a8b5f902458886c203976921 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Thu, 18 Aug 2016 15:24:11 -0600 Subject: [PATCH] lean into pkg/errors philosophy on wrappin' --- errors/errors.go | 69 ++++------------------------------------ httputil/response.go | 6 ++++ lfs/hook.go | 2 +- lfs/pointer_clean.go | 2 +- lfs/pointer_smudge.go | 3 +- transfer/adapterbase.go | 2 +- transfer/basic_upload.go | 1 + transfer/tus_upload.go | 1 + 8 files changed, 19 insertions(+), 67 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index 057feeff..4ded1e37 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -140,34 +140,6 @@ func IsAuthError(err error) bool { return false } -// IsInvalidPointerError indicates an attempt to parse data that was not a -// valid pointer. -func IsInvalidPointerError(err error) bool { - if e, ok := err.(interface { - InvalidPointer() bool - }); ok { - return e.InvalidPointer() - } - if parent := parentOf(err); parent != nil { - return IsInvalidPointerError(parent) - } - return false -} - -// IsInvalidRepoError indicates an operation was attempted from outside a git -// repository. -func IsInvalidRepoError(err error) bool { - if e, ok := err.(interface { - InvalidRepo() bool - }); ok { - return e.InvalidRepo() - } - if parent := parentOf(err); parent != nil { - return IsInvalidRepoError(parent) - } - return false -} - // IsSmudgeError indicates an error while smudging a files. func IsSmudgeError(err error) bool { if e, ok := err.(interface { @@ -398,34 +370,6 @@ func NewAuthError(err error) error { return authError{newWrappedError(err, "Authentication required")} } -// Definitions for IsInvalidPointerError() - -type invalidPointerError struct { - errorWrapper -} - -func (e invalidPointerError) InvalidPointer() bool { - return true -} - -func NewInvalidPointerError(err error) error { - return invalidPointerError{newWrappedError(err, "Invalid pointer")} -} - -// Definitions for IsInvalidRepoError() - -type invalidRepoError struct { - errorWrapper -} - -func (e invalidRepoError) InvalidRepo() bool { - return true -} - -func NewInvalidRepoError(err error) error { - return invalidRepoError{newWrappedError(err, "Not in a git repository")} -} - // Definitions for IsSmudgeError() type smudgeError struct { @@ -453,8 +397,9 @@ func (e cleanPointerError) CleanPointerError() bool { return true } -func NewCleanPointerError(err error, pointer interface{}, bytes []byte) error { - e := cleanPointerError{newWrappedError(err, "Clean pointer error")} +func NewCleanPointerError(pointer interface{}, bytes []byte) error { + err := New("pointer error") + e := cleanPointerError{newWrappedError(err, "clean")} ErrorSetContext(e, "pointer", pointer) ErrorSetContext(e, "bytes", bytes) return e @@ -486,8 +431,8 @@ func (e badPointerKeyError) BadPointerKeyError() bool { } func NewBadPointerKeyError(expected, actual string) error { - err := fmt.Errorf("Error parsing LFS Pointer. Expected key %s, got %s", expected, actual) - return badPointerKeyError{expected, actual, newWrappedError(err, "")} + err := Errorf("Expected key %s, got %s", expected, actual) + return badPointerKeyError{expected, actual, newWrappedError(err, "pointer parsing")} } // Definitions for IsDownloadDeclinedError() @@ -500,8 +445,8 @@ func (e downloadDeclinedError) DownloadDeclinedError() bool { return true } -func NewDownloadDeclinedError(err error) error { - return downloadDeclinedError{newWrappedError(err, "File missing and download is not allowed")} +func NewDownloadDeclinedError(err error, msg string) error { + return downloadDeclinedError{newWrappedError(err, msg)} } // Definitions for IsRetriableError() diff --git a/httputil/response.go b/httputil/response.go index 086f1b56..f82f5fa9 100644 --- a/httputil/response.go +++ b/httputil/response.go @@ -79,10 +79,16 @@ func handleResponse(cfg *config.Configuration, res *http.Response, creds auth.Cr } if res.StatusCode == 401 { + if err == nil { + err = errors.New("api: received status 401") + } return errors.NewAuthError(err) } if res.StatusCode > 499 && res.StatusCode != 501 && res.StatusCode != 509 { + if err == nil { + err = errors.Errorf("api: received status %d", res.StatusCode) + } return errors.NewFatalError(err) } diff --git a/lfs/hook.go b/lfs/hook.go index 015cceed..b1e374a0 100644 --- a/lfs/hook.go +++ b/lfs/hook.go @@ -90,7 +90,7 @@ func (h *Hook) Upgrade() error { // or any of the past versions of this hook. func (h *Hook) Uninstall() error { if !InRepo() { - return errors.NewInvalidRepoError(nil) + return errors.New("Not in a git repository") } match, err := h.matchesCurrent() diff --git a/lfs/pointer_clean.go b/lfs/pointer_clean.go index beae1875..0208a14b 100644 --- a/lfs/pointer_clean.go +++ b/lfs/pointer_clean.go @@ -78,7 +78,7 @@ func copyToTemp(reader io.Reader, fileSize int64, cb progress.CopyCallback) (oid by, ptr, err := DecodeFrom(reader) if err == nil && len(by) < 512 { - err = errors.NewCleanPointerError(err, ptr, by) + err = errors.NewCleanPointerError(ptr, by) return } diff --git a/lfs/pointer_smudge.go b/lfs/pointer_smudge.go index 60e3dc2b..55fe0d0b 100644 --- a/lfs/pointer_smudge.go +++ b/lfs/pointer_smudge.go @@ -46,7 +46,6 @@ func PointerSmudge(writer io.Writer, ptr *Pointer, workingfile string, download LinkOrCopyFromReference(ptr.Oid, ptr.Size) stat, statErr := os.Stat(mediafile) - if statErr == nil && stat != nil { fileSize := stat.Size() if fileSize == 0 || fileSize != ptr.Size { @@ -60,7 +59,7 @@ func PointerSmudge(writer io.Writer, ptr *Pointer, workingfile string, download if download { err = downloadFile(writer, ptr, workingfile, mediafile, manifest, cb) } else { - return errors.NewDownloadDeclinedError(nil) + return errors.NewDownloadDeclinedError(statErr, "smudge") } } else { err = readLocalFile(writer, ptr, mediafile, workingfile, cb) diff --git a/transfer/adapterbase.go b/transfer/adapterbase.go index 367e6630..432c9e94 100644 --- a/transfer/adapterbase.go +++ b/transfer/adapterbase.go @@ -128,7 +128,7 @@ func (a *adapterBase) worker(workerNum int, ctx interface{}) { var err error if t.Object.IsExpired(time.Now().Add(objectExpirationGracePeriod)) { tracerx.Printf("xfer: adapter %q worker %d found job for %q expired, retrying...", a.Name(), workerNum, t.Object.Oid) - err = errors.NewRetriableError(fmt.Errorf("lfs/transfer: object %q has expired", t.Object.Oid)) + err = errors.NewRetriableError(errors.Errorf("lfs/transfer: object %q has expired", t.Object.Oid)) } else if t.Object.Size < 0 { tracerx.Printf("xfer: adapter %q worker %d found invalid size for %q (got: %d), retrying...", a.Name(), workerNum, t.Object.Oid, t.Object.Size) err = fmt.Errorf("Git LFS: object %q has invalid size (got: %d)", t.Object.Oid, t.Object.Size) diff --git a/transfer/basic_upload.go b/transfer/basic_upload.go index 26db52f6..a5f88ea0 100644 --- a/transfer/basic_upload.go +++ b/transfer/basic_upload.go @@ -106,6 +106,7 @@ func (a *basicUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb Transfe // A status code of 403 likely means that an authentication token for the // upload has expired. This can be safely retried. if res.StatusCode == 403 { + err = errors.New("http: received status 403") return errors.NewRetriableError(err) } diff --git a/transfer/tus_upload.go b/transfer/tus_upload.go index c05f0710..8f33f803 100644 --- a/transfer/tus_upload.go +++ b/transfer/tus_upload.go @@ -143,6 +143,7 @@ func (a *tusUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb TransferP // A status code of 403 likely means that an authentication token for the // upload has expired. This can be safely retried. if res.StatusCode == 403 { + err = errors.New("http: received status 403") return errors.NewRetriableError(err) }