From 24730683456d9babf5a7d5b6f64bae3641e9403d Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Thu, 18 Aug 2016 14:20:33 -0600 Subject: [PATCH 01/10] rename errutil to errors --- api/api.go | 32 +++++++-------- api/download_test.go | 4 +- api/upload_test.go | 6 +-- api/v1.go | 4 +- api/verify.go | 6 +-- auth/credentials.go | 5 +-- commands/command_checkout.go | 6 +-- commands/command_clean.go | 6 +-- commands/command_logs.go | 5 +-- commands/command_smudge.go | 8 ++-- commands/commands.go | 8 ++-- commands/uploader.go | 6 +-- {errutil => errors}/errors.go | 60 ++++++++++++++-------------- {errutil => errors}/errors_test.go | 2 +- httputil/request.go | 14 +++---- httputil/request_error_test.go | 6 +-- httputil/response.go | 22 +++++----- lfs/hook.go | 4 +- lfs/pointer.go | 15 ++++--- lfs/pointer_clean.go | 4 +- lfs/pointer_smudge.go | 30 +++++++------- lfs/pointer_test.go | 4 +- lfs/transfer_queue.go | 8 ++-- lfs/upload_queue.go | 6 +-- test/git-lfs-test-server-api/main.go | 4 +- transfer/adapterbase.go | 4 +- transfer/basic_download.go | 5 +-- transfer/basic_upload.go | 10 ++--- transfer/tus_upload.go | 14 +++---- 29 files changed, 153 insertions(+), 155 deletions(-) rename {errutil => errors}/errors.go (96%) rename {errutil => errors}/errors_test.go (98%) diff --git a/api/api.go b/api/api.go index 6e164226..5a6c4bec 100644 --- a/api/api.go +++ b/api/api.go @@ -9,7 +9,7 @@ import ( "strconv" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/git" "github.com/github/git-lfs/httputil" "github.com/github/git-lfs/tools" @@ -27,7 +27,7 @@ func BatchOrLegacy(cfg *config.Configuration, objects []*ObjectResource, operati } objs, adapterName, err := Batch(cfg, objects, operation, transferAdapters) if err != nil { - if errutil.IsNotImplementedError(err) { + if errors.IsNotImplementedError(err) { git.Config.SetLocal("", "lfs.batch", "false") objs, err := Legacy(cfg, objects, operation) return objs, "", err @@ -63,12 +63,12 @@ func Batch(cfg *config.Configuration, objects []*ObjectResource, operation strin o := &batchRequest{Operation: operation, Objects: objects, TransferAdapterNames: transferAdapters} by, err := json.Marshal(o) if err != nil { - return nil, "", errutil.Error(err) + return nil, "", errors.Error(err) } req, err := NewBatchRequest(cfg, operation) if err != nil { - return nil, "", errutil.Error(err) + return nil, "", errors.Error(err) } req.Header.Set("Content-Type", MediaType) @@ -83,14 +83,14 @@ func Batch(cfg *config.Configuration, objects []*ObjectResource, operation strin if err != nil { if res == nil { - return nil, "", errutil.NewRetriableError(err) + return nil, "", errors.NewRetriableError(err) } if res.StatusCode == 0 { - return nil, "", errutil.NewRetriableError(err) + return nil, "", errors.NewRetriableError(err) } - if errutil.IsAuthError(err) { + if errors.IsAuthError(err) { httputil.SetAuthType(cfg, req, res) return Batch(cfg, objects, operation, transferAdapters) } @@ -98,16 +98,16 @@ func Batch(cfg *config.Configuration, objects []*ObjectResource, operation strin switch res.StatusCode { case 404, 410: tracerx.Printf("api: batch not implemented: %d", res.StatusCode) - return nil, "", errutil.NewNotImplementedError(nil) + return nil, "", errors.NewNotImplementedError(nil) } tracerx.Printf("api error: %s", err) - return nil, "", errutil.Error(err) + return nil, "", errors.Error(err) } httputil.LogTransfer(cfg, "lfs.batch", res) if res.StatusCode != 200 { - return nil, "", errutil.Error(fmt.Errorf("Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode)) + return nil, "", errors.Error(fmt.Errorf("Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode)) } return bresp.Objects, bresp.TransferAdapterName, nil @@ -140,7 +140,7 @@ func Legacy(cfg *config.Configuration, objects []*ObjectResource, operation stri func DownloadCheck(cfg *config.Configuration, oid string) (*ObjectResource, error) { req, err := NewRequest(cfg, "GET", oid) if err != nil { - return nil, errutil.Error(err) + return nil, errors.Error(err) } res, obj, err := DoLegacyRequest(cfg, req) @@ -152,7 +152,7 @@ func DownloadCheck(cfg *config.Configuration, oid string) (*ObjectResource, erro _, err = obj.NewRequest("download", "GET") if err != nil { - return nil, errutil.Error(err) + return nil, errors.Error(err) } return obj, nil @@ -167,12 +167,12 @@ func UploadCheck(cfg *config.Configuration, oid string, size int64) (*ObjectReso by, err := json.Marshal(reqObj) if err != nil { - return nil, errutil.Error(err) + return nil, errors.Error(err) } req, err := NewRequest(cfg, "POST", oid) if err != nil { - return nil, errutil.Error(err) + return nil, errors.Error(err) } req.Header.Set("Content-Type", MediaType) @@ -184,12 +184,12 @@ func UploadCheck(cfg *config.Configuration, oid string, size int64) (*ObjectReso res, obj, err := DoLegacyRequest(cfg, req) if err != nil { - if errutil.IsAuthError(err) { + if errors.IsAuthError(err) { httputil.SetAuthType(cfg, req, res) return UploadCheck(cfg, oid, size) } - return nil, errutil.NewRetriableError(err) + return nil, errors.NewRetriableError(err) } httputil.LogTransfer(cfg, "lfs.upload", res) diff --git a/api/download_test.go b/api/download_test.go index ee11845e..593e853f 100644 --- a/api/download_test.go +++ b/api/download_test.go @@ -16,7 +16,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/auth" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/httputil" ) @@ -313,7 +313,7 @@ func TestDownloadAPIError(t *testing.T) { t.Fatal("no error?") } - if errutil.IsFatalError(err) { + if errors.IsFatalError(err) { t.Fatal("should not panic") } diff --git a/api/upload_test.go b/api/upload_test.go index 4b3b0de7..4720effc 100644 --- a/api/upload_test.go +++ b/api/upload_test.go @@ -15,7 +15,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/httputil" "github.com/github/git-lfs/lfs" "github.com/github/git-lfs/test" @@ -448,7 +448,7 @@ func TestUploadApiError(t *testing.T) { t.Fatal(err) } - if errutil.IsFatalError(err) { + if errors.IsFatalError(err) { t.Fatal("should not panic") } @@ -577,7 +577,7 @@ func TestUploadVerifyError(t *testing.T) { t.Fatal("verify should fail") } - if errutil.IsFatalError(err) { + if errors.IsFatalError(err) { t.Fatal("should not panic") } diff --git a/api/v1.go b/api/v1.go index 0b94eaf5..04e76eb6 100644 --- a/api/v1.go +++ b/api/v1.go @@ -7,7 +7,7 @@ import ( "github.com/github/git-lfs/auth" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/httputil" "github.com/rubyist/tracerx" @@ -55,7 +55,7 @@ func DoBatchRequest(cfg *config.Configuration, req *http.Request) (*http.Respons if err != nil { if res != nil && res.StatusCode == 401 { - return res, nil, errutil.NewAuthError(err) + return res, nil, errors.NewAuthError(err) } return res, nil, err } diff --git a/api/verify.go b/api/verify.go index 992e5b9d..c9aff245 100644 --- a/api/verify.go +++ b/api/verify.go @@ -8,7 +8,7 @@ import ( "strconv" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/httputil" ) @@ -21,12 +21,12 @@ func VerifyUpload(cfg *config.Configuration, obj *ObjectResource) error { req, err := obj.NewRequest("verify", "POST") if err != nil { - return errutil.Error(err) + return errors.Error(err) } by, err := json.Marshal(obj) if err != nil { - return errutil.Error(err) + return errors.Error(err) } req.Header.Set("Content-Type", MediaType) diff --git a/auth/credentials.go b/auth/credentials.go index f8970020..6b5973b7 100644 --- a/auth/credentials.go +++ b/auth/credentials.go @@ -3,7 +3,6 @@ package auth import ( "bytes" "encoding/base64" - "errors" "fmt" "net" "net/http" @@ -13,7 +12,7 @@ import ( "strings" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/rubyist/tracerx" ) @@ -35,7 +34,7 @@ func GetCreds(cfg *config.Configuration, req *http.Request) (Creds, error) { credsUrl, err := getCredURLForAPI(cfg, req) if err != nil { - return nil, errutil.Error(err) + return nil, errors.Error(err) } if credsUrl == nil { diff --git a/commands/command_checkout.go b/commands/command_checkout.go index 3a5e1965..599bc00a 100644 --- a/commands/command_checkout.go +++ b/commands/command_checkout.go @@ -7,7 +7,7 @@ import ( "os/exec" "sync" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/git" "github.com/github/git-lfs/lfs" "github.com/github/git-lfs/progress" @@ -174,7 +174,7 @@ func checkoutWithChan(in <-chan *lfs.WrappedPointer) { // Check the content - either missing or still this pointer (not exist is ok) filepointer, err := lfs.DecodePointerFromFile(pointer.Name) if err != nil && !os.IsNotExist(err) { - if errutil.IsNotAPointerError(err) { + if errors.IsNotAPointerError(err) { // File has non-pointer content, leave it alone continue } @@ -193,7 +193,7 @@ func checkoutWithChan(in <-chan *lfs.WrappedPointer) { err = lfs.PointerSmudgeToFile(cwdfilepath, pointer.Pointer, false, manifest, nil) if err != nil { - if errutil.IsDownloadDeclinedError(err) { + if errors.IsDownloadDeclinedError(err) { // acceptable error, data not local (fetch not run or include/exclude) LoggedError(err, "Skipped checkout for %v, content not local. Use fetch to download.", pointer.Name) } else { diff --git a/commands/command_clean.go b/commands/command_clean.go index 30b2d5d6..bbf40e84 100644 --- a/commands/command_clean.go +++ b/commands/command_clean.go @@ -3,7 +3,7 @@ package commands import ( "os" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/lfs" "github.com/github/git-lfs/progress" "github.com/spf13/cobra" @@ -43,8 +43,8 @@ func cleanCommand(cmd *cobra.Command, args []string) { defer cleaned.Teardown() } - if errutil.IsCleanPointerError(err) { - os.Stdout.Write(errutil.ErrorGetContext(err, "bytes").([]byte)) + if errors.IsCleanPointerError(err) { + os.Stdout.Write(errors.ErrorGetContext(err, "bytes").([]byte)) return } diff --git a/commands/command_logs.go b/commands/command_logs.go index e2d7bc47..49917ac5 100644 --- a/commands/command_logs.go +++ b/commands/command_logs.go @@ -1,13 +1,12 @@ package commands import ( - "errors" "io/ioutil" "os" "path/filepath" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/spf13/cobra" ) @@ -54,7 +53,7 @@ func logsClearCommand(cmd *cobra.Command, args []string) { func logsBoomtownCommand(cmd *cobra.Command, args []string) { Debug("Debug message") - err := errutil.Errorf(errors.New("Inner error message!"), "Error!") + err := errors.Errorf(errors.New("Inner error message!"), "Error") Panic(err, "Welcome to Boomtown") Debug("Never seen") } diff --git a/commands/command_smudge.go b/commands/command_smudge.go index 79c1afd5..96f0efb4 100644 --- a/commands/command_smudge.go +++ b/commands/command_smudge.go @@ -6,7 +6,7 @@ import ( "os" "path/filepath" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/lfs" "github.com/spf13/cobra" ) @@ -71,7 +71,7 @@ func smudgeCommand(cmd *cobra.Command, args []string) { if err != nil { ptr.Encode(os.Stdout) // Download declined error is ok to skip if we weren't requesting download - if !(errutil.IsDownloadDeclinedError(err) && !download) { + if !(errors.IsDownloadDeclinedError(err) && !download) { LoggedError(err, "Error downloading object: %s (%s)", filename, ptr.Oid) if !cfg.SkipDownloadErrors() { os.Exit(2) @@ -85,8 +85,8 @@ func smudgeFilename(args []string, err error) string { return args[0] } - if errutil.IsSmudgeError(err) { - return filepath.Base(errutil.ErrorGetContext(err, "FileName").(string)) + if errors.IsSmudgeError(err) { + return filepath.Base(errors.ErrorGetContext(err, "FileName").(string)) } return "" diff --git a/commands/commands.go b/commands/commands.go index 15bdf06c..75f1762c 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -14,7 +14,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/git" "github.com/github/git-lfs/httputil" "github.com/github/git-lfs/lfs" @@ -119,7 +119,7 @@ func FullError(err error) { func errorWith(err error, fatalErrFn func(error, string, ...interface{}), errFn func(string, ...interface{})) { var innermsg string - if inner := errutil.GetInnerError(err); inner != nil { + if inner := errors.GetInnerError(err); inner != nil { innermsg = inner.Error() } @@ -128,7 +128,7 @@ func errorWith(err error, fatalErrFn func(error, string, ...interface{}), errFn Error(innermsg) } - if Debugging || errutil.IsFatalError(err) { + if Debugging || errors.IsFatalError(err) { fatalErrFn(err, errmsg) } else { errFn(errmsg) @@ -269,7 +269,7 @@ func logPanicToWriter(w io.Writer, loggedError error) { } w.Write(err.Stack()) } else { - w.Write(errutil.Stack()) + w.Write(errors.Stack()) } fmt.Fprintln(w, "\nENV:") diff --git a/commands/uploader.go b/commands/uploader.go index da1e1f73..d91ca4dd 100644 --- a/commands/uploader.go +++ b/commands/uploader.go @@ -3,7 +3,7 @@ package commands import ( "os" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/lfs" "github.com/github/git-lfs/tools" ) @@ -129,8 +129,8 @@ func upload(c *uploadContext, unfiltered []*lfs.WrappedPointer) { for _, p := range pointers { u, err := lfs.NewUploadable(p.Oid, p.Name) if err != nil { - if errutil.IsCleanPointerError(err) { - Exit(uploadMissingErr, p.Oid, p.Name, errutil.ErrorGetContext(err, "pointer").(*lfs.Pointer).Oid) + if errors.IsCleanPointerError(err) { + Exit(uploadMissingErr, p.Oid, p.Name, errors.ErrorGetContext(err, "pointer").(*lfs.Pointer).Oid) } else { ExitWithError(err) } diff --git a/errutil/errors.go b/errors/errors.go similarity index 96% rename from errutil/errors.go rename to errors/errors.go index da0a3b76..6db4b7e8 100644 --- a/errutil/errors.go +++ b/errors/errors.go @@ -1,6 +1,6 @@ -// Package errutil provides common error handling tools +// Package errors provides common error handling tools // NOTE: Subject to change, do not rely on this package from outside git-lfs source -package errutil +package errors // The LFS error system provides a simple wrapper around Go errors and the // ability to inspect errors. It is strongly influenced by Dave Cheney's post @@ -41,9 +41,9 @@ package errutil // Example: // // err := lfs.SomeFunction() -// errutil.ErrorSetContext(err, "foo", "bar") -// errutil.ErrorGetContext(err, "foo") // => "bar" -// errutil.ErrorDelContext(err, "foo") +// errors.ErrorSetContext(err, "foo", "bar") +// errors.ErrorGetContext(err, "foo") // => "bar" +// errors.ErrorDelContext(err, "foo") // // Wrapped errors also contain the stack from the point at which they are // called. The stack is accessed via ErrorStack(). Calling ErrorStack() on a @@ -56,6 +56,31 @@ import ( "github.com/pkg/errors" ) +// New returns an error with the supplied message. New also records the stack +// trace at thepoint it was called. +func New(message string) error { + return errors.New(message) +} + +// Error wraps an error with an empty message. +func Error(err error) error { + return Errorf(err, "") +} + +// Errorf wraps an error with an additional formatted message. +func Errorf(err error, format string, args ...interface{}) error { + if err == nil { + err = errors.New("") + } + + message := "" + if len(format) > 0 { + message = fmt.Sprintf(format, args...) + } + + return newWrappedError(err, message) +} + type errorWithCause interface { Error() string Cause() error @@ -232,30 +257,7 @@ func IsRetriableError(err error) bool { } func GetInnerError(err error) error { - if parent := parentOf(err); parent != nil { - return parent - } - - return nil -} - -// Error wraps an error with an empty message. -func Error(err error) error { - return Errorf(err, "") -} - -// Errorf wraps an error with an additional formatted message. -func Errorf(err error, format string, args ...interface{}) error { - if err == nil { - err = errors.New("") - } - - message := "" - if len(format) > 0 { - message = fmt.Sprintf(format, args...) - } - - return newWrappedError(err, message) + return parentOf(err) } // ErrorSetContext sets a value in the error's context. If the error has not diff --git a/errutil/errors_test.go b/errors/errors_test.go similarity index 98% rename from errutil/errors_test.go rename to errors/errors_test.go index 83daab31..4dcc0322 100644 --- a/errutil/errors_test.go +++ b/errors/errors_test.go @@ -1,4 +1,4 @@ -package errutil +package errors import ( "errors" diff --git a/httputil/request.go b/httputil/request.go index e0cec5a6..54f3fac2 100644 --- a/httputil/request.go +++ b/httputil/request.go @@ -10,7 +10,7 @@ import ( "github.com/github/git-lfs/auth" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/rubyist/tracerx" ) @@ -65,11 +65,11 @@ func doHttpRequest(cfg *config.Configuration, req *http.Request, creds auth.Cred } if err != nil { - if errutil.IsAuthError(err) { + if errors.IsAuthError(err) { SetAuthType(cfg, req, res) doHttpRequest(cfg, req, creds) } else { - err = errutil.Error(err) + err = errors.Error(err) } } else { err = handleResponse(cfg, res, creds) @@ -126,7 +126,7 @@ func DoHttpRequestWithRedirects(cfg *config.Configuration, req *http.Request, vi redirectedReq, err := NewHttpRequest(req.Method, redirectTo, nil) if err != nil { - return res, errutil.Errorf(err, err.Error()) + return res, errors.Errorf(err, err.Error()) } via = append(via, req) @@ -139,17 +139,17 @@ func DoHttpRequestWithRedirects(cfg *config.Configuration, req *http.Request, vi seeker, ok := realBody.(io.Seeker) if !ok { - return res, errutil.Errorf(nil, "Request body needs to be an io.Seeker to handle redirects.") + return res, errors.Errorf(nil, "Request body needs to be an io.Seeker to handle redirects.") } if _, err := seeker.Seek(0, 0); err != nil { - return res, errutil.Error(err) + return res, errors.Error(err) } redirectedReq.Body = realBody redirectedReq.ContentLength = req.ContentLength if err = CheckRedirect(redirectedReq, via); err != nil { - return res, errutil.Errorf(err, err.Error()) + return res, errors.Errorf(err, err.Error()) } return DoHttpRequestWithRedirects(cfg, redirectedReq, via, useCreds) diff --git a/httputil/request_error_test.go b/httputil/request_error_test.go index 88d1b80f..cd6d3057 100644 --- a/httputil/request_error_test.go +++ b/httputil/request_error_test.go @@ -11,7 +11,7 @@ import ( "testing" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" ) func TestSuccessStatus(t *testing.T) { @@ -77,7 +77,7 @@ func TestErrorStatusWithCustomMessage(t *testing.T) { continue } - if errutil.IsFatalError(err) == (panicMsg != "panic") { + if errors.IsFatalError(err) == (panicMsg != "panic") { t.Errorf("Error for HTTP %d should %s", status, panicMsg) continue } @@ -140,7 +140,7 @@ func TestErrorStatusWithDefaultMessage(t *testing.T) { continue } - if errutil.IsFatalError(err) == (results[1] != "panic") { + if errors.IsFatalError(err) == (results[1] != "panic") { t.Errorf("Error for HTTP %d should %s", status, results[1]) continue } diff --git a/httputil/response.go b/httputil/response.go index 8267de30..1ef12a5b 100644 --- a/httputil/response.go +++ b/httputil/response.go @@ -10,7 +10,7 @@ import ( "github.com/github/git-lfs/auth" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" ) var ( @@ -41,7 +41,7 @@ func DecodeResponse(res *http.Response, obj interface{}) error { res.Body.Close() if err != nil { - return errutil.Errorf(err, "Unable to parse HTTP response for %s", TraceHttpReq(res.Request)) + return errors.Errorf(err, "Unable to parse HTTP response for %s", TraceHttpReq(res.Request)) } return nil @@ -74,16 +74,16 @@ func handleResponse(cfg *config.Configuration, res *http.Response, creds auth.Cr if len(cliErr.Message) == 0 { err = defaultError(res) } else { - err = errutil.Error(cliErr) + err = errors.Error(cliErr) } } if res.StatusCode == 401 { - return errutil.NewAuthError(err) + return errors.NewAuthError(err) } if res.StatusCode > 499 && res.StatusCode != 501 && res.StatusCode != 509 { - return errutil.NewFatalError(err) + return errors.NewFatalError(err) } return err @@ -100,18 +100,18 @@ func defaultError(res *http.Response) error { msgFmt = defaultErrors[500] + fmt.Sprintf(" from HTTP %d", res.StatusCode) } - return errutil.Error(fmt.Errorf(msgFmt, res.Request.URL)) + return errors.Error(fmt.Errorf(msgFmt, res.Request.URL)) } func SetErrorResponseContext(cfg *config.Configuration, err error, res *http.Response) { - errutil.ErrorSetContext(err, "Status", res.Status) + errors.ErrorSetContext(err, "Status", res.Status) setErrorHeaderContext(err, "Request", res.Header) setErrorRequestContext(cfg, err, res.Request) } func setErrorRequestContext(cfg *config.Configuration, err error, req *http.Request) { - errutil.ErrorSetContext(err, "Endpoint", cfg.Endpoint(auth.GetOperationForRequest(req)).Url) - errutil.ErrorSetContext(err, "URL", TraceHttpReq(req)) + errors.ErrorSetContext(err, "Endpoint", cfg.Endpoint(auth.GetOperationForRequest(req)).Url) + errors.ErrorSetContext(err, "URL", TraceHttpReq(req)) setErrorHeaderContext(err, "Response", req.Header) } @@ -119,9 +119,9 @@ func setErrorHeaderContext(err error, prefix string, head http.Header) { for key, _ := range head { contextKey := fmt.Sprintf("%s:%s", prefix, key) if _, skip := hiddenHeaders[key]; skip { - errutil.ErrorSetContext(err, contextKey, "--") + errors.ErrorSetContext(err, contextKey, "--") } else { - errutil.ErrorSetContext(err, contextKey, head.Get(key)) + errors.ErrorSetContext(err, contextKey, head.Get(key)) } } } diff --git a/lfs/hook.go b/lfs/hook.go index bd6d8793..015cceed 100644 --- a/lfs/hook.go +++ b/lfs/hook.go @@ -9,7 +9,7 @@ import ( "strings" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/git" ) @@ -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 errutil.NewInvalidRepoError(nil) + return errors.NewInvalidRepoError(nil) } match, err := h.matchesCurrent() diff --git a/lfs/pointer.go b/lfs/pointer.go index ee7886d6..a6e1ec94 100644 --- a/lfs/pointer.go +++ b/lfs/pointer.go @@ -11,10 +11,9 @@ import ( "strconv" "strings" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/progress" "github.com/github/git-lfs/transfer" - "github.com/pkg/errors" ) var ( @@ -95,7 +94,7 @@ func DecodePointerFromFile(file string) (*Pointer, error) { return nil, err } if stat.Size() > blobSizeCutoff { - return nil, errutil.NewNotAPointerError(errors.New("file size exceeds lfs pointer size cutoff")) + return nil, errors.NewNotAPointerError(errors.New("file size exceeds lfs pointer size cutoff")) } f, err := os.OpenFile(file, os.O_RDONLY, 0644) if err != nil { @@ -124,7 +123,7 @@ func DecodeFrom(reader io.Reader) ([]byte, *Pointer, error) { func verifyVersion(version string) error { if len(version) == 0 { - return errutil.NewNotAPointerError(errors.New("Missing version")) + return errors.NewNotAPointerError(errors.New("Missing version")) } for _, v := range v1Aliases { @@ -139,8 +138,8 @@ func verifyVersion(version string) error { func decodeKV(data []byte) (*Pointer, error) { kvps, exts, err := decodeKVData(data) if err != nil { - if errutil.IsBadPointerKeyError(err) { - return nil, errutil.StandardizeBadPointerError(err) + if errors.IsBadPointerKeyError(err) { + return nil, errors.StandardizeBadPointerError(err) } return nil, err } @@ -234,7 +233,7 @@ func decodeKVData(data []byte) (kvps map[string]string, exts map[string]string, kvps = make(map[string]string) if !matcherRE.Match(data) { - err = errutil.NewNotAPointerError(errors.New("invalid header")) + err = errors.NewNotAPointerError(errors.New("invalid header")) return } @@ -263,7 +262,7 @@ func decodeKVData(data []byte) (kvps map[string]string, exts map[string]string, if expected := pointerKeys[line]; key != expected { if !extRE.Match([]byte(key)) { - err = errutil.NewBadPointerKeyError(expected, key) + err = errors.NewBadPointerKeyError(expected, key) return } if exts == nil { diff --git a/lfs/pointer_clean.go b/lfs/pointer_clean.go index ba368a03..beae1875 100644 --- a/lfs/pointer_clean.go +++ b/lfs/pointer_clean.go @@ -8,7 +8,7 @@ import ( "os" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/progress" "github.com/github/git-lfs/tools" ) @@ -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 = errutil.NewCleanPointerError(err, ptr, by) + err = errors.NewCleanPointerError(err, ptr, by) return } diff --git a/lfs/pointer_smudge.go b/lfs/pointer_smudge.go index f80683a2..94cdb43f 100644 --- a/lfs/pointer_smudge.go +++ b/lfs/pointer_smudge.go @@ -12,7 +12,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/progress" "github.com/rubyist/tracerx" ) @@ -25,7 +25,7 @@ func PointerSmudgeToFile(filename string, ptr *Pointer, download bool, manifest } defer file.Close() if err := PointerSmudge(file, ptr, filename, download, manifest, cb); err != nil { - if errutil.IsDownloadDeclinedError(err) { + if errors.IsDownloadDeclinedError(err) { // write placeholder data instead file.Seek(0, os.SEEK_SET) ptr.Encode(file) @@ -60,14 +60,14 @@ func PointerSmudge(writer io.Writer, ptr *Pointer, workingfile string, download if download { err = downloadFile(writer, ptr, workingfile, mediafile, manifest, cb) } else { - return errutil.NewDownloadDeclinedError(nil) + return errors.NewDownloadDeclinedError(nil) } } else { err = readLocalFile(writer, ptr, mediafile, workingfile, cb) } if err != nil { - return errutil.NewSmudgeError(err, ptr.Oid, mediafile) + return errors.NewSmudgeError(err, ptr.Oid, mediafile) } return nil @@ -79,7 +79,7 @@ func downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, xfers := manifest.GetDownloadAdapterNames() obj, adapterName, err := api.BatchOrLegacySingle(config.Config, &api.ObjectResource{Oid: ptr.Oid, Size: ptr.Size}, "download", xfers) if err != nil { - return errutil.Errorf(err, "Error downloading %s: %s", filepath.Base(mediafile), err) + return errors.Errorf(err, "Error downloading %s: %s", filepath.Base(mediafile), err) } if ptr.Size == 0 { @@ -104,7 +104,7 @@ func downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, res := <-adapterResultChan if res.Error != nil { - return errutil.Errorf(err, "Error buffering media file: %s", res.Error) + return errors.Errorf(err, "Error buffering media file: %s", res.Error) } return readLocalFile(writer, ptr, mediafile, workingfile, nil) @@ -113,7 +113,7 @@ func downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile string, cb progress.CopyCallback) error { reader, err := os.Open(mediafile) if err != nil { - return errutil.Errorf(err, "Error opening media file.") + return errors.Errorf(err, "Error opening media file.") } defer reader.Close() @@ -130,14 +130,14 @@ func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile ext, ok := registeredExts[ptrExt.Name] if !ok { err := fmt.Errorf("Extension '%s' is not configured.", ptrExt.Name) - return errutil.Error(err) + return errors.Error(err) } ext.Priority = ptrExt.Priority extensions[ext.Name] = ext } exts, err := config.SortExtensions(extensions) if err != nil { - return errutil.Error(err) + return errors.Error(err) } // pipe extensions in reverse order @@ -151,7 +151,7 @@ func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile response, err := pipeExtensions(request) if err != nil { - return errutil.Error(err) + return errors.Error(err) } actualExts := make(map[string]*pipeExtResult) @@ -163,32 +163,32 @@ func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile oid := response.results[0].oidIn if ptr.Oid != oid { err = fmt.Errorf("Actual oid %s during smudge does not match expected %s", oid, ptr.Oid) - return errutil.Error(err) + return errors.Error(err) } for _, expected := range ptr.Extensions { actual := actualExts[expected.Name] if actual.name != expected.Name { err = fmt.Errorf("Actual extension name '%s' does not match expected '%s'", actual.name, expected.Name) - return errutil.Error(err) + return errors.Error(err) } if actual.oidOut != expected.Oid { err = fmt.Errorf("Actual oid %s for extension '%s' does not match expected %s", actual.oidOut, expected.Name, expected.Oid) - return errutil.Error(err) + return errors.Error(err) } } // setup reader reader, err = os.Open(response.file.Name()) if err != nil { - return errutil.Errorf(err, "Error opening smudged file: %s", err) + return errors.Errorf(err, "Error opening smudged file: %s", err) } defer reader.Close() } _, err = tools.CopyWithCallback(writer, reader, ptr.Size, cb) if err != nil { - return errutil.Errorf(err, "Error reading from media file: %s", err) + return errors.Errorf(err, "Error reading from media file: %s", err) } return nil diff --git a/lfs/pointer_test.go b/lfs/pointer_test.go index 78585ff7..246a158f 100644 --- a/lfs/pointer_test.go +++ b/lfs/pointer_test.go @@ -8,7 +8,7 @@ import ( "strings" "testing" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/stretchr/testify/assert" ) @@ -81,7 +81,7 @@ func TestDecodeTinyFile(t *testing.T) { t.Errorf("pointer was decoded: %v", p) } - if !errutil.IsNotAPointerError(err) { + if !errors.IsNotAPointerError(err) { t.Errorf("error is not a NotAPointerError: %s: '%v'", reflect.TypeOf(err), err) } } diff --git a/lfs/transfer_queue.go b/lfs/transfer_queue.go index 29484ca8..53f89d43 100644 --- a/lfs/transfer_queue.go +++ b/lfs/transfer_queue.go @@ -6,7 +6,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/git" "github.com/github/git-lfs/progress" "github.com/github/git-lfs/transfer" @@ -355,7 +355,7 @@ func (q *TransferQueue) batchApiRoutine() { objs, adapterName, err := api.Batch(config.Config, transfers, q.transferKind(), transferAdapterNames) if err != nil { - if errutil.IsNotImplementedError(err) { + if errors.IsNotImplementedError(err) { git.Config.SetLocal("", "lfs.batch", "false") go q.legacyFallback(batch) @@ -379,7 +379,7 @@ func (q *TransferQueue) batchApiRoutine() { for _, o := range objs { if o.Error != nil { - q.errorc <- errutil.Errorf(o.Error, "[%v] %v", o.Oid, o.Error.Message) + q.errorc <- errors.Errorf(o.Error, "[%v] %v", o.Oid, o.Error.Message) q.Skip(o.Size) q.wait.Done() continue @@ -461,7 +461,7 @@ func (q *TransferQueue) retry(t Transferable) { } func (q *TransferQueue) canRetry(err error) bool { - if !errutil.IsRetriableError(err) || atomic.LoadUint32(&q.retrying) == 1 { + if !errors.IsRetriableError(err) || atomic.LoadUint32(&q.retrying) == 1 { return false } diff --git a/lfs/upload_queue.go b/lfs/upload_queue.go index 40a06841..e6f650c5 100644 --- a/lfs/upload_queue.go +++ b/lfs/upload_queue.go @@ -7,7 +7,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/transfer" ) @@ -54,7 +54,7 @@ func (u *Uploadable) LegacyCheck() (*api.ObjectResource, error) { func NewUploadable(oid, filename string) (*Uploadable, error) { localMediaPath, err := LocalMediaPath(oid) if err != nil { - return nil, errutil.Errorf(err, "Error uploading file %s (%s)", filename, oid) + return nil, errors.Errorf(err, "Error uploading file %s (%s)", filename, oid) } if len(filename) > 0 { @@ -65,7 +65,7 @@ func NewUploadable(oid, filename string) (*Uploadable, error) { fi, err := os.Stat(localMediaPath) if err != nil { - return nil, errutil.Errorf(err, "Error uploading file %s (%s)", filename, oid) + return nil, errors.Errorf(err, "Error uploading file %s (%s)", filename, oid) } return &Uploadable{oid: oid, OidPath: localMediaPath, Filename: filename, size: fi.Size()}, nil diff --git a/test/git-lfs-test-server-api/main.go b/test/git-lfs-test-server-api/main.go index 40f5f45a..267574d1 100644 --- a/test/git-lfs-test-server-api/main.go +++ b/test/git-lfs-test-server-api/main.go @@ -12,7 +12,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/lfs" "github.com/github/git-lfs/test" "github.com/spf13/cobra" @@ -170,7 +170,7 @@ func buildTestData() (oidsExist, oidsMissing []TestObject, err error) { uploadQueue.Wait() for _, err := range uploadQueue.Errors() { - if errutil.IsFatalError(err) { + if errors.IsFatalError(err) { exit("Fatal error setting up test data: %s", err) } } diff --git a/transfer/adapterbase.go b/transfer/adapterbase.go index f415cf5c..367e6630 100644 --- a/transfer/adapterbase.go +++ b/transfer/adapterbase.go @@ -5,7 +5,7 @@ import ( "sync" "time" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/rubyist/tracerx" ) @@ -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 = errutil.NewRetriableError(fmt.Errorf("lfs/transfer: object %q has expired", t.Object.Oid)) + err = errors.NewRetriableError(fmt.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_download.go b/transfer/basic_download.go index 8c83b1b5..1a01682a 100644 --- a/transfer/basic_download.go +++ b/transfer/basic_download.go @@ -1,7 +1,6 @@ package transfer import ( - "errors" "fmt" "hash" "io" @@ -11,7 +10,7 @@ import ( "strconv" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/httputil" "github.com/github/git-lfs/localstorage" "github.com/github/git-lfs/tools" @@ -119,7 +118,7 @@ func (a *basicDownloadAdapter) download(t *Transfer, cb TransferProgressCallback os.Remove(dlFile.Name()) return a.download(t, cb, authOkFunc, nil, 0, nil) } - return errutil.NewRetriableError(err) + return errors.NewRetriableError(err) } httputil.LogTransfer(config.Config, "lfs.data.download", res) defer res.Body.Close() diff --git a/transfer/basic_upload.go b/transfer/basic_upload.go index c7de3428..d38ad3db 100644 --- a/transfer/basic_upload.go +++ b/transfer/basic_upload.go @@ -10,7 +10,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/httputil" "github.com/github/git-lfs/progress" ) @@ -69,7 +69,7 @@ func (a *basicUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb Transfe f, err := os.OpenFile(t.Path, os.O_RDONLY, 0644) if err != nil { - return errutil.Error(err) + return errors.Error(err) } defer f.Close() @@ -99,18 +99,18 @@ func (a *basicUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb Transfe res, err := httputil.DoHttpRequest(config.Config, req, t.Object.NeedsAuth()) if err != nil { - return errutil.NewRetriableError(err) + return errors.NewRetriableError(err) } httputil.LogTransfer(config.Config, "lfs.data.upload", res) // 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 { - return errutil.NewRetriableError(err) + return errors.NewRetriableError(err) } if res.StatusCode > 299 { - return errutil.Errorf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) + return errors.Errorf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) } io.Copy(ioutil.Discard, res.Body) diff --git a/transfer/tus_upload.go b/transfer/tus_upload.go index f035e9da..9c8493ea 100644 --- a/transfer/tus_upload.go +++ b/transfer/tus_upload.go @@ -9,7 +9,7 @@ import ( "github.com/github/git-lfs/api" "github.com/github/git-lfs/config" - "github.com/github/git-lfs/errutil" + "github.com/github/git-lfs/errors" "github.com/github/git-lfs/httputil" "github.com/github/git-lfs/progress" "github.com/rubyist/tracerx" @@ -55,7 +55,7 @@ func (a *tusUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb TransferP req.Header.Set("Tus-Resumable", TusVersion) res, err := httputil.DoHttpRequest(config.Config, req, false) if err != nil { - return errutil.NewRetriableError(err) + return errors.NewRetriableError(err) } // Response will contain Upload-Offset if supported @@ -78,7 +78,7 @@ func (a *tusUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb TransferP // Open file for uploading f, err := os.OpenFile(t.Path, os.O_RDONLY, 0644) if err != nil { - return errutil.Error(err) + return errors.Error(err) } defer f.Close() @@ -90,7 +90,7 @@ func (a *tusUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb TransferP advanceCallbackProgress(cb, t, offset) _, err := f.Seek(offset, os.SEEK_CUR) if err != nil { - return errutil.Error(err) + return errors.Error(err) } } @@ -136,18 +136,18 @@ func (a *tusUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb TransferP res, err = httputil.DoHttpRequest(config.Config, req, false) if err != nil { - return errutil.NewRetriableError(err) + return errors.NewRetriableError(err) } httputil.LogTransfer(config.Config, "lfs.data.upload", res) // 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 { - return errutil.NewRetriableError(err) + return errors.NewRetriableError(err) } if res.StatusCode > 299 { - return errutil.Errorf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) + return errors.Errorf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) } io.Copy(ioutil.Discard, res.Body) From 8fd3774c92fbf8ec2dd927fbcb5d597c2e810718 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Thu, 18 Aug 2016 14:35:11 -0600 Subject: [PATCH 02/10] errors: Errorf -> Wrapf --- commands/command_logs.go | 2 +- errors/errors.go | 11 ++++------- httputil/request.go | 6 +++--- httputil/response.go | 2 +- lfs/pointer_smudge.go | 10 +++++----- lfs/transfer_queue.go | 2 +- lfs/upload_queue.go | 4 ++-- transfer/basic_upload.go | 2 +- transfer/tus_upload.go | 2 +- 9 files changed, 19 insertions(+), 22 deletions(-) diff --git a/commands/command_logs.go b/commands/command_logs.go index 49917ac5..84664ae8 100644 --- a/commands/command_logs.go +++ b/commands/command_logs.go @@ -53,7 +53,7 @@ func logsClearCommand(cmd *cobra.Command, args []string) { func logsBoomtownCommand(cmd *cobra.Command, args []string) { Debug("Debug message") - err := errors.Errorf(errors.New("Inner error message!"), "Error") + err := errors.Wrapf(errors.New("Inner error message!"), "Error") Panic(err, "Welcome to Boomtown") Debug("Never seen") } diff --git a/errors/errors.go b/errors/errors.go index 6db4b7e8..835ca979 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -64,19 +64,16 @@ func New(message string) error { // Error wraps an error with an empty message. func Error(err error) error { - return Errorf(err, "") + return Wrapf(err, "") } -// Errorf wraps an error with an additional formatted message. -func Errorf(err error, format string, args ...interface{}) error { +// Wrapf wraps an error with an additional formatted message. +func Wrapf(err error, format string, args ...interface{}) error { if err == nil { err = errors.New("") } - message := "" - if len(format) > 0 { - message = fmt.Sprintf(format, args...) - } + message := fmt.Sprintf(format, args...) return newWrappedError(err, message) } diff --git a/httputil/request.go b/httputil/request.go index 54f3fac2..c59ef333 100644 --- a/httputil/request.go +++ b/httputil/request.go @@ -126,7 +126,7 @@ func DoHttpRequestWithRedirects(cfg *config.Configuration, req *http.Request, vi redirectedReq, err := NewHttpRequest(req.Method, redirectTo, nil) if err != nil { - return res, errors.Errorf(err, err.Error()) + return res, errors.Wrapf(err, err.Error()) } via = append(via, req) @@ -139,7 +139,7 @@ func DoHttpRequestWithRedirects(cfg *config.Configuration, req *http.Request, vi seeker, ok := realBody.(io.Seeker) if !ok { - return res, errors.Errorf(nil, "Request body needs to be an io.Seeker to handle redirects.") + return res, errors.Wrapf(nil, "Request body needs to be an io.Seeker to handle redirects.") } if _, err := seeker.Seek(0, 0); err != nil { @@ -149,7 +149,7 @@ func DoHttpRequestWithRedirects(cfg *config.Configuration, req *http.Request, vi redirectedReq.ContentLength = req.ContentLength if err = CheckRedirect(redirectedReq, via); err != nil { - return res, errors.Errorf(err, err.Error()) + return res, errors.Wrapf(err, err.Error()) } return DoHttpRequestWithRedirects(cfg, redirectedReq, via, useCreds) diff --git a/httputil/response.go b/httputil/response.go index 1ef12a5b..8582ea89 100644 --- a/httputil/response.go +++ b/httputil/response.go @@ -41,7 +41,7 @@ func DecodeResponse(res *http.Response, obj interface{}) error { res.Body.Close() if err != nil { - return errors.Errorf(err, "Unable to parse HTTP response for %s", TraceHttpReq(res.Request)) + return errors.Wrapf(err, "Unable to parse HTTP response for %s", TraceHttpReq(res.Request)) } return nil diff --git a/lfs/pointer_smudge.go b/lfs/pointer_smudge.go index 94cdb43f..6ab0184d 100644 --- a/lfs/pointer_smudge.go +++ b/lfs/pointer_smudge.go @@ -79,7 +79,7 @@ func downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, xfers := manifest.GetDownloadAdapterNames() obj, adapterName, err := api.BatchOrLegacySingle(config.Config, &api.ObjectResource{Oid: ptr.Oid, Size: ptr.Size}, "download", xfers) if err != nil { - return errors.Errorf(err, "Error downloading %s: %s", filepath.Base(mediafile), err) + return errors.Wrapf(err, "Error downloading %s: %s", filepath.Base(mediafile), err) } if ptr.Size == 0 { @@ -104,7 +104,7 @@ func downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, res := <-adapterResultChan if res.Error != nil { - return errors.Errorf(err, "Error buffering media file: %s", res.Error) + return errors.Wrapf(err, "Error buffering media file: %s", res.Error) } return readLocalFile(writer, ptr, mediafile, workingfile, nil) @@ -113,7 +113,7 @@ func downloadFile(writer io.Writer, ptr *Pointer, workingfile, mediafile string, func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile string, cb progress.CopyCallback) error { reader, err := os.Open(mediafile) if err != nil { - return errors.Errorf(err, "Error opening media file.") + return errors.Wrapf(err, "Error opening media file.") } defer reader.Close() @@ -181,14 +181,14 @@ func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile // setup reader reader, err = os.Open(response.file.Name()) if err != nil { - return errors.Errorf(err, "Error opening smudged file: %s", err) + return errors.Wrapf(err, "Error opening smudged file: %s", err) } defer reader.Close() } _, err = tools.CopyWithCallback(writer, reader, ptr.Size, cb) if err != nil { - return errors.Errorf(err, "Error reading from media file: %s", err) + return errors.Wrapf(err, "Error reading from media file: %s", err) } return nil diff --git a/lfs/transfer_queue.go b/lfs/transfer_queue.go index 53f89d43..09fb6766 100644 --- a/lfs/transfer_queue.go +++ b/lfs/transfer_queue.go @@ -379,7 +379,7 @@ func (q *TransferQueue) batchApiRoutine() { for _, o := range objs { if o.Error != nil { - q.errorc <- errors.Errorf(o.Error, "[%v] %v", o.Oid, o.Error.Message) + q.errorc <- errors.Wrapf(o.Error, "[%v] %v", o.Oid, o.Error.Message) q.Skip(o.Size) q.wait.Done() continue diff --git a/lfs/upload_queue.go b/lfs/upload_queue.go index e6f650c5..d328d9d1 100644 --- a/lfs/upload_queue.go +++ b/lfs/upload_queue.go @@ -54,7 +54,7 @@ func (u *Uploadable) LegacyCheck() (*api.ObjectResource, error) { func NewUploadable(oid, filename string) (*Uploadable, error) { localMediaPath, err := LocalMediaPath(oid) if err != nil { - return nil, errors.Errorf(err, "Error uploading file %s (%s)", filename, oid) + return nil, errors.Wrapf(err, "Error uploading file %s (%s)", filename, oid) } if len(filename) > 0 { @@ -65,7 +65,7 @@ func NewUploadable(oid, filename string) (*Uploadable, error) { fi, err := os.Stat(localMediaPath) if err != nil { - return nil, errors.Errorf(err, "Error uploading file %s (%s)", filename, oid) + return nil, errors.Wrapf(err, "Error uploading file %s (%s)", filename, oid) } return &Uploadable{oid: oid, OidPath: localMediaPath, Filename: filename, size: fi.Size()}, nil diff --git a/transfer/basic_upload.go b/transfer/basic_upload.go index d38ad3db..d48a1b70 100644 --- a/transfer/basic_upload.go +++ b/transfer/basic_upload.go @@ -110,7 +110,7 @@ func (a *basicUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb Transfe } if res.StatusCode > 299 { - return errors.Errorf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) + return errors.Wrapf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) } io.Copy(ioutil.Discard, res.Body) diff --git a/transfer/tus_upload.go b/transfer/tus_upload.go index 9c8493ea..c23c37f9 100644 --- a/transfer/tus_upload.go +++ b/transfer/tus_upload.go @@ -147,7 +147,7 @@ func (a *tusUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb TransferP } if res.StatusCode > 299 { - return errors.Errorf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) + return errors.Wrapf(nil, "Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) } io.Copy(ioutil.Discard, res.Body) From 311cc7a2af66540fa26469f0b848111828a09e67 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Thu, 18 Aug 2016 15:02:21 -0600 Subject: [PATCH 03/10] api,auth,errors,httputil,lfs,transfer: wrap errors --- api/api.go | 20 +++++++++----------- api/download_test.go | 2 +- api/upload_test.go | 2 +- api/verify.go | 4 ++-- auth/credentials.go | 2 +- errors/errors.go | 13 ++++++++++--- httputil/request.go | 11 +++++++---- httputil/response.go | 4 ++-- lfs/pointer_smudge.go | 12 ++++++------ transfer/basic_upload.go | 2 +- transfer/tus_upload.go | 4 ++-- 11 files changed, 42 insertions(+), 34 deletions(-) diff --git a/api/api.go b/api/api.go index 5a6c4bec..16a046c2 100644 --- a/api/api.go +++ b/api/api.go @@ -63,12 +63,12 @@ func Batch(cfg *config.Configuration, objects []*ObjectResource, operation strin o := &batchRequest{Operation: operation, Objects: objects, TransferAdapterNames: transferAdapters} by, err := json.Marshal(o) if err != nil { - return nil, "", errors.Error(err) + return nil, "", errors.Wrap(err, "batch request") } req, err := NewBatchRequest(cfg, operation) if err != nil { - return nil, "", errors.Error(err) + return nil, "", errors.Wrap(err, "batch request") } req.Header.Set("Content-Type", MediaType) @@ -81,7 +81,6 @@ func Batch(cfg *config.Configuration, objects []*ObjectResource, operation strin res, bresp, err := DoBatchRequest(cfg, req) if err != nil { - if res == nil { return nil, "", errors.NewRetriableError(err) } @@ -97,17 +96,16 @@ func Batch(cfg *config.Configuration, objects []*ObjectResource, operation strin switch res.StatusCode { case 404, 410: - tracerx.Printf("api: batch not implemented: %d", res.StatusCode) - return nil, "", errors.NewNotImplementedError(nil) + return nil, "", errors.NewNotImplementedError(errors.Errorf("api: batch not implemented: %d", res.StatusCode)) } tracerx.Printf("api error: %s", err) - return nil, "", errors.Error(err) + return nil, "", errors.Wrap(err, "batch response") } httputil.LogTransfer(cfg, "lfs.batch", res) if res.StatusCode != 200 { - return nil, "", errors.Error(fmt.Errorf("Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode)) + return nil, "", errors.Errorf("Invalid status for %s: %d", httputil.TraceHttpReq(req), res.StatusCode) } return bresp.Objects, bresp.TransferAdapterName, nil @@ -140,7 +138,7 @@ func Legacy(cfg *config.Configuration, objects []*ObjectResource, operation stri func DownloadCheck(cfg *config.Configuration, oid string) (*ObjectResource, error) { req, err := NewRequest(cfg, "GET", oid) if err != nil { - return nil, errors.Error(err) + return nil, errors.Wrap(err, "download check") } res, obj, err := DoLegacyRequest(cfg, req) @@ -152,7 +150,7 @@ func DownloadCheck(cfg *config.Configuration, oid string) (*ObjectResource, erro _, err = obj.NewRequest("download", "GET") if err != nil { - return nil, errors.Error(err) + return nil, errors.Wrap(err, "download check") } return obj, nil @@ -167,12 +165,12 @@ func UploadCheck(cfg *config.Configuration, oid string, size int64) (*ObjectReso by, err := json.Marshal(reqObj) if err != nil { - return nil, errors.Error(err) + return nil, errors.Wrap(err, "upload check") } req, err := NewRequest(cfg, "POST", oid) if err != nil { - return nil, errors.Error(err) + return nil, errors.Wrap(err, "upload check") } req.Header.Set("Content-Type", MediaType) diff --git a/api/download_test.go b/api/download_test.go index 593e853f..8ccd59fc 100644 --- a/api/download_test.go +++ b/api/download_test.go @@ -321,7 +321,7 @@ func TestDownloadAPIError(t *testing.T) { return } - expected := "LFS: " + fmt.Sprintf(httputil.GetDefaultError(404), server.URL+"/media/objects/oid") + expected := fmt.Sprintf(httputil.GetDefaultError(404), server.URL+"/media/objects/oid") if err.Error() != expected { t.Fatalf("Expected: %s\nGot: %s", expected, err.Error()) } diff --git a/api/upload_test.go b/api/upload_test.go index 4720effc..09c7cae2 100644 --- a/api/upload_test.go +++ b/api/upload_test.go @@ -581,7 +581,7 @@ func TestUploadVerifyError(t *testing.T) { t.Fatal("should not panic") } - expected := "LFS: " + fmt.Sprintf(httputil.GetDefaultError(404), server.URL+"/verify") + expected := fmt.Sprintf(httputil.GetDefaultError(404), server.URL+"/verify") if err.Error() != expected { t.Fatalf("Expected: %s\nGot: %s", expected, err.Error()) } diff --git a/api/verify.go b/api/verify.go index c9aff245..b0312617 100644 --- a/api/verify.go +++ b/api/verify.go @@ -21,12 +21,12 @@ func VerifyUpload(cfg *config.Configuration, obj *ObjectResource) error { req, err := obj.NewRequest("verify", "POST") if err != nil { - return errors.Error(err) + return errors.Wrap(err, "verify") } by, err := json.Marshal(obj) if err != nil { - return errors.Error(err) + return errors.Wrap(err, "verify") } req.Header.Set("Content-Type", MediaType) diff --git a/auth/credentials.go b/auth/credentials.go index 6b5973b7..328fa50d 100644 --- a/auth/credentials.go +++ b/auth/credentials.go @@ -34,7 +34,7 @@ func GetCreds(cfg *config.Configuration, req *http.Request) (Creds, error) { credsUrl, err := getCredURLForAPI(cfg, req) if err != nil { - return nil, errors.Error(err) + return nil, errors.Wrap(err, "creds") } if credsUrl == nil { diff --git a/errors/errors.go b/errors/errors.go index 835ca979..057feeff 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -62,9 +62,16 @@ func New(message string) error { return errors.New(message) } -// Error wraps an error with an empty message. -func Error(err error) error { - return Wrapf(err, "") +// Errorf formats according to a format specifier and returns the string +// as a value that satisfies error. +// Errorf also records the stack trace at the point it was called. +func Errorf(format string, args ...interface{}) error { + return errors.Errorf(format, args...) +} + +// Wrap wraps an error with an additional message. +func Wrap(err error, msg string) error { + return newWrappedError(err, msg) } // Wrapf wraps an error with an additional formatted message. diff --git a/httputil/request.go b/httputil/request.go index c59ef333..8410823d 100644 --- a/httputil/request.go +++ b/httputil/request.go @@ -45,13 +45,16 @@ func (e *ClientError) Error() string { // Internal http request management func doHttpRequest(cfg *config.Configuration, req *http.Request, creds auth.Creds) (*http.Response, error) { var ( - res *http.Response - err error + res *http.Response + cause string + err error ) if cfg.NtlmAccess(auth.GetOperationForRequest(req)) { + cause = "ntlm" res, err = doNTLMRequest(cfg, req, true) } else { + cause = "http" res, err = NewHttpClient(cfg, req.Host).Do(req) } @@ -69,7 +72,7 @@ func doHttpRequest(cfg *config.Configuration, req *http.Request, creds auth.Cred SetAuthType(cfg, req, res) doHttpRequest(cfg, req, creds) } else { - err = errors.Error(err) + err = errors.Wrap(err, cause) } } else { err = handleResponse(cfg, res, creds) @@ -143,7 +146,7 @@ func DoHttpRequestWithRedirects(cfg *config.Configuration, req *http.Request, vi } if _, err := seeker.Seek(0, 0); err != nil { - return res, errors.Error(err) + return res, errors.Wrap(err, "request retry") } redirectedReq.Body = realBody redirectedReq.ContentLength = req.ContentLength diff --git a/httputil/response.go b/httputil/response.go index 8582ea89..086f1b56 100644 --- a/httputil/response.go +++ b/httputil/response.go @@ -74,7 +74,7 @@ func handleResponse(cfg *config.Configuration, res *http.Response, creds auth.Cr if len(cliErr.Message) == 0 { err = defaultError(res) } else { - err = errors.Error(cliErr) + err = errors.Wrap(cliErr, "http") } } @@ -100,7 +100,7 @@ func defaultError(res *http.Response) error { msgFmt = defaultErrors[500] + fmt.Sprintf(" from HTTP %d", res.StatusCode) } - return errors.Error(fmt.Errorf(msgFmt, res.Request.URL)) + return errors.Errorf(msgFmt, res.Request.URL) } func SetErrorResponseContext(cfg *config.Configuration, err error, res *http.Response) { diff --git a/lfs/pointer_smudge.go b/lfs/pointer_smudge.go index 6ab0184d..60e3dc2b 100644 --- a/lfs/pointer_smudge.go +++ b/lfs/pointer_smudge.go @@ -130,14 +130,14 @@ func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile ext, ok := registeredExts[ptrExt.Name] if !ok { err := fmt.Errorf("Extension '%s' is not configured.", ptrExt.Name) - return errors.Error(err) + return errors.Wrap(err, "smudge") } ext.Priority = ptrExt.Priority extensions[ext.Name] = ext } exts, err := config.SortExtensions(extensions) if err != nil { - return errors.Error(err) + return errors.Wrap(err, "smudge") } // pipe extensions in reverse order @@ -151,7 +151,7 @@ func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile response, err := pipeExtensions(request) if err != nil { - return errors.Error(err) + return errors.Wrap(err, "smudge") } actualExts := make(map[string]*pipeExtResult) @@ -163,18 +163,18 @@ func readLocalFile(writer io.Writer, ptr *Pointer, mediafile string, workingfile oid := response.results[0].oidIn if ptr.Oid != oid { err = fmt.Errorf("Actual oid %s during smudge does not match expected %s", oid, ptr.Oid) - return errors.Error(err) + return errors.Wrap(err, "smudge") } for _, expected := range ptr.Extensions { actual := actualExts[expected.Name] if actual.name != expected.Name { err = fmt.Errorf("Actual extension name '%s' does not match expected '%s'", actual.name, expected.Name) - return errors.Error(err) + return errors.Wrap(err, "smudge") } if actual.oidOut != expected.Oid { err = fmt.Errorf("Actual oid %s for extension '%s' does not match expected %s", actual.oidOut, expected.Name, expected.Oid) - return errors.Error(err) + return errors.Wrap(err, "smudge") } } diff --git a/transfer/basic_upload.go b/transfer/basic_upload.go index d48a1b70..26db52f6 100644 --- a/transfer/basic_upload.go +++ b/transfer/basic_upload.go @@ -69,7 +69,7 @@ func (a *basicUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb Transfe f, err := os.OpenFile(t.Path, os.O_RDONLY, 0644) if err != nil { - return errors.Error(err) + return errors.Wrap(err, "basic upload") } defer f.Close() diff --git a/transfer/tus_upload.go b/transfer/tus_upload.go index c23c37f9..c05f0710 100644 --- a/transfer/tus_upload.go +++ b/transfer/tus_upload.go @@ -78,7 +78,7 @@ func (a *tusUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb TransferP // Open file for uploading f, err := os.OpenFile(t.Path, os.O_RDONLY, 0644) if err != nil { - return errors.Error(err) + return errors.Wrap(err, "tus upload") } defer f.Close() @@ -90,7 +90,7 @@ func (a *tusUploadAdapter) DoTransfer(ctx interface{}, t *Transfer, cb TransferP advanceCallbackProgress(cb, t, offset) _, err := f.Seek(offset, os.SEEK_CUR) if err != nil { - return errors.Error(err) + return errors.Wrap(err, "tus upload") } } From 69acb28d616660c1a8b5f902458886c203976921 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Thu, 18 Aug 2016 15:24:11 -0600 Subject: [PATCH 04/10] 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) } From a3a4e5b78eeb52b3b290a98e6f00ce809911b0fb Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Thu, 18 Aug 2016 15:43:28 -0600 Subject: [PATCH 05/10] errors: remove GetInnerError() and ErrorWithStack --- commands/commands.go | 56 ++++++++++++++++---------------------------- errors/errors.go | 4 ---- lfs/attribute.go | 2 +- 3 files changed, 21 insertions(+), 41 deletions(-) diff --git a/commands/commands.go b/commands/commands.go index 75f1762c..71ce9974 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -85,18 +85,21 @@ func TransferManifest() *transfer.Manifest { // Error prints a formatted message to Stderr. It also gets printed to the // panic log if one is created for this command. func Error(format string, args ...interface{}) { - line := format - if len(args) > 0 { - line = fmt.Sprintf(format, args...) + if len(args) == 0 { + fmt.Fprintln(ErrorWriter, format) + return } - fmt.Fprintln(ErrorWriter, line) + fmt.Fprintf(ErrorWriter, format+"\n", args...) } // Print prints a formatted message to Stdout. It also gets printed to the // panic log if one is created for this command. func Print(format string, args ...interface{}) { - line := fmt.Sprintf(format, args...) - fmt.Fprintln(OutputWriter, line) + if len(args) == 0 { + fmt.Fprintln(OutputWriter, format) + return + } + fmt.Fprintf(OutputWriter, format+"\n", args...) } // Exit prints a formatted message and exits. @@ -118,21 +121,12 @@ func FullError(err error) { } func errorWith(err error, fatalErrFn func(error, string, ...interface{}), errFn func(string, ...interface{})) { - var innermsg string - if inner := errors.GetInnerError(err); inner != nil { - innermsg = inner.Error() - } - - errmsg := err.Error() - if errmsg != innermsg { - Error(innermsg) - } - if Debugging || errors.IsFatalError(err) { - fatalErrFn(err, errmsg) - } else { - errFn(errmsg) + fatalErrFn(err, "") + return } + + errFn("%s", err) } // Debug prints a formatted message if debugging is enabled. The formatted @@ -147,7 +141,9 @@ func Debug(format string, args ...interface{}) { // LoggedError prints a formatted message to Stderr and writes a stack trace for // the error to a log file without exiting. func LoggedError(err error, format string, args ...interface{}) { - Error(format, args...) + if len(format) > 0 { + Error(format, args...) + } file := handlePanic(err) if len(file) > 0 { @@ -260,17 +256,11 @@ func logPanicToWriter(w io.Writer, loggedError error) { w.Write(ErrorBuffer.Bytes()) fmt.Fprintln(w) - fmt.Fprintln(w, loggedError.Error()) - - if err, ok := loggedError.(ErrorWithStack); ok { - fmt.Fprintln(w, err.InnerError()) - for key, value := range err.Context() { - fmt.Fprintf(w, "%s=%s\n", key, value) - } - w.Write(err.Stack()) - } else { - w.Write(errors.Stack()) + fmt.Fprintf(w, "%+v\n", loggedError) + for key, val := range errors.ErrorContext(err) { + fmt.Fprintf(w, "%s=%v\n", key, val) } + fmt.Fprintln(w, "\nENV:") // log the environment @@ -279,12 +269,6 @@ func logPanicToWriter(w io.Writer, loggedError error) { } } -type ErrorWithStack interface { - Context() map[string]string - InnerError() string - Stack() []byte -} - func determineIncludeExcludePaths(config *config.Configuration, includeArg, excludeArg *string) (include, exclude []string) { if includeArg == nil { include = config.FetchIncludePaths() diff --git a/errors/errors.go b/errors/errors.go index 4ded1e37..07c7f015 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -232,10 +232,6 @@ func IsRetriableError(err error) bool { return false } -func GetInnerError(err error) error { - return parentOf(err) -} - // ErrorSetContext sets a value in the error's context. If the error has not // been wrapped, it does nothing. func ErrorSetContext(err error, key string, value interface{}) { diff --git a/lfs/attribute.go b/lfs/attribute.go index cef8d946..e8c99655 100644 --- a/lfs/attribute.go +++ b/lfs/attribute.go @@ -90,7 +90,7 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error { } return err } else if currentValue != value { - return fmt.Errorf("The %s attribute should be \"%s\" but is \"%s\"", + return fmt.Errorf("The %s attribute should be %q but is %q", key, value, currentValue) } From c99920070c4efd95388320fc1e00d999e0161b94 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Thu, 18 Aug 2016 15:55:44 -0600 Subject: [PATCH 06/10] errors: implement fmt.Formatter --- errors/errors.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/errors/errors.go b/errors/errors.go index 07c7f015..3353c36b 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -86,8 +86,10 @@ func Wrapf(err error, format string, args ...interface{}) error { } type errorWithCause interface { - Error() string Cause() error + StackTrace() errors.StackTrace + error + fmt.Formatter } func parentOf(err error) error { From 8ecfd2240322a9392ad97bd35fe49ef503b89d5e Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Thu, 18 Aug 2016 16:02:58 -0600 Subject: [PATCH 07/10] errors: remove unused Stack() func --- errors/errors.go | 12 ++---------- 1 file changed, 2 insertions(+), 10 deletions(-) diff --git a/errors/errors.go b/errors/errors.go index 3353c36b..5ba0dcd4 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -46,12 +46,11 @@ package errors // errors.ErrorDelContext(err, "foo") // // Wrapped errors also contain the stack from the point at which they are -// called. The stack is accessed via ErrorStack(). Calling ErrorStack() on a -// regular Go error will return an empty byte slice. +// called. Use the '%+v' printf verb to display. See the github.com/pkg/errors +// docs for more info: https://godoc.org/github.com/pkg/errors import ( "fmt" - "runtime" "github.com/pkg/errors" ) @@ -460,10 +459,3 @@ func (e retriableError) RetriableError() bool { func NewRetriableError(err error) error { return retriableError{newWrappedError(err, "")} } - -// Stack returns a byte slice containing the runtime.Stack() -func Stack() []byte { - stackBuf := make([]byte, 1024*1024) - written := runtime.Stack(stackBuf, false) - return stackBuf[:written] -} From b5a1c070f95c77ba26de224574bbf8f3c6b338c6 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Thu, 18 Aug 2016 16:15:15 -0600 Subject: [PATCH 08/10] errors: split into multiple go files --- errors/context.go | 42 ++++++ errors/errors.go | 376 ---------------------------------------------- errors/types.go | 340 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 382 insertions(+), 376 deletions(-) create mode 100644 errors/context.go create mode 100644 errors/types.go diff --git a/errors/context.go b/errors/context.go new file mode 100644 index 00000000..f6986edc --- /dev/null +++ b/errors/context.go @@ -0,0 +1,42 @@ +package errors + +type errorWithContext interface { + Set(string, interface{}) + Get(string) interface{} + Del(string) + Context() map[string]interface{} +} + +// ErrorSetContext sets a value in the error's context. If the error has not +// been wrapped, it does nothing. +func ErrorSetContext(err error, key string, value interface{}) { + if e, ok := err.(errorWithContext); ok { + e.Set(key, value) + } +} + +// ErrorGetContext gets a value from the error's context. If the error has not +// been wrapped, it returns an empty string. +func ErrorGetContext(err error, key string) interface{} { + if e, ok := err.(errorWithContext); ok { + return e.Get(key) + } + return "" +} + +// ErrorDelContext removes a value from the error's context. If the error has +// not been wrapped, it does nothing. +func ErrorDelContext(err error, key string) { + if e, ok := err.(errorWithContext); ok { + e.Del(key) + } +} + +// ErrorContext returns the context map for an error if it is a wrappedError. +// If it is not a wrappedError it will return an empty map. +func ErrorContext(err error) map[string]interface{} { + if e, ok := err.(errorWithContext); ok { + return e.Context() + } + return nil +} diff --git a/errors/errors.go b/errors/errors.go index 5ba0dcd4..26f0dab0 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -83,379 +83,3 @@ func Wrapf(err error, format string, args ...interface{}) error { return newWrappedError(err, message) } - -type errorWithCause interface { - Cause() error - StackTrace() errors.StackTrace - error - fmt.Formatter -} - -func parentOf(err error) error { - if c, ok := err.(errorWithCause); ok { - return c.Cause() - } - - return nil -} - -// IsFatalError indicates that the error is fatal and the process should exit -// immediately after handling the error. -func IsFatalError(err error) bool { - if e, ok := err.(interface { - Fatal() bool - }); ok { - return e.Fatal() - } - if parent := parentOf(err); parent != nil { - return IsFatalError(parent) - } - return false -} - -// IsNotImplementedError indicates the client attempted to use a feature the -// server has not implemented (e.g. the batch endpoint). -func IsNotImplementedError(err error) bool { - if e, ok := err.(interface { - NotImplemented() bool - }); ok { - return e.NotImplemented() - } - if parent := parentOf(err); parent != nil { - return IsNotImplementedError(parent) - } - return false -} - -// IsAuthError indicates the client provided a request with invalid or no -// authentication credentials when credentials are required (e.g. HTTP 401). -func IsAuthError(err error) bool { - if e, ok := err.(interface { - AuthError() bool - }); ok { - return e.AuthError() - } - if parent := parentOf(err); parent != nil { - return IsAuthError(parent) - } - return false -} - -// IsSmudgeError indicates an error while smudging a files. -func IsSmudgeError(err error) bool { - if e, ok := err.(interface { - SmudgeError() bool - }); ok { - return e.SmudgeError() - } - if parent := parentOf(err); parent != nil { - return IsSmudgeError(parent) - } - return false -} - -// IsCleanPointerError indicates an error while cleaning a file. -func IsCleanPointerError(err error) bool { - if e, ok := err.(interface { - CleanPointerError() bool - }); ok { - return e.CleanPointerError() - } - if parent := parentOf(err); parent != nil { - return IsCleanPointerError(parent) - } - return false -} - -// IsNotAPointerError indicates the parsed data is not an LFS pointer. -func IsNotAPointerError(err error) bool { - if e, ok := err.(interface { - NotAPointerError() bool - }); ok { - return e.NotAPointerError() - } - if parent := parentOf(err); parent != nil { - return IsNotAPointerError(parent) - } - return false -} - -// IsBadPointerKeyError indicates that the parsed data has an invalid key. -func IsBadPointerKeyError(err error) bool { - if e, ok := err.(interface { - BadPointerKeyError() bool - }); ok { - return e.BadPointerKeyError() - } - - if parent := parentOf(err); parent != nil { - return IsBadPointerKeyError(parent) - } - return false -} - -// If an error is abad pointer error of any type, returns NotAPointerError -func StandardizeBadPointerError(err error) error { - if IsBadPointerKeyError(err) { - badErr := err.(badPointerKeyError) - if badErr.Expected == "version" { - return NewNotAPointerError(err) - } - } - return err -} - -// IsDownloadDeclinedError indicates that the smudge operation should not download. -// TODO: I don't really like using errors to control that flow, it should be refactored. -func IsDownloadDeclinedError(err error) bool { - if e, ok := err.(interface { - DownloadDeclinedError() bool - }); ok { - return e.DownloadDeclinedError() - } - if parent := parentOf(err); parent != nil { - return IsDownloadDeclinedError(parent) - } - return false -} - -// IsRetriableError indicates the low level transfer had an error but the -// caller may retry the operation. -func IsRetriableError(err error) bool { - if e, ok := err.(interface { - RetriableError() bool - }); ok { - return e.RetriableError() - } - if parent := parentOf(err); parent != nil { - return IsRetriableError(parent) - } - return false -} - -// ErrorSetContext sets a value in the error's context. If the error has not -// been wrapped, it does nothing. -func ErrorSetContext(err error, key string, value interface{}) { - if e, ok := err.(errorWrapper); ok { - e.Set(key, value) - } -} - -// ErrorGetContext gets a value from the error's context. If the error has not -// been wrapped, it returns an empty string. -func ErrorGetContext(err error, key string) interface{} { - if e, ok := err.(errorWrapper); ok { - return e.Get(key) - } - return "" -} - -// ErrorDelContext removes a value from the error's context. If the error has -// not been wrapped, it does nothing. -func ErrorDelContext(err error, key string) { - if e, ok := err.(errorWrapper); ok { - e.Del(key) - } -} - -// ErrorContext returns the context map for an error if it is a wrappedError. -// If it is not a wrappedError it will return an empty map. -func ErrorContext(err error) map[string]interface{} { - if e, ok := err.(errorWrapper); ok { - return e.Context() - } - return nil -} - -type errorWrapper interface { - errorWithCause - - Set(string, interface{}) - Get(string) interface{} - Del(string) - Context() map[string]interface{} -} - -// wrappedError is the base error wrapper. It provides a Message string, a -// stack, and a context map around a regular Go error. -type wrappedError struct { - errorWithCause - context map[string]interface{} -} - -// newWrappedError creates a wrappedError. -func newWrappedError(err error, message string) errorWrapper { - if err == nil { - err = errors.New("Error") - } - - var errWithCause errorWithCause - - if len(message) > 0 { - errWithCause = errors.Wrap(err, message).(errorWithCause) - } else if ewc, ok := err.(errorWithCause); ok { - errWithCause = ewc - } else { - errWithCause = errors.Wrap(err, "LFS").(errorWithCause) - } - - return &wrappedError{ - context: make(map[string]interface{}), - errorWithCause: errWithCause, - } -} - -// Set sets the value for the key in the context. -func (e wrappedError) Set(key string, val interface{}) { - e.context[key] = val -} - -// Get gets the value for a key in the context. -func (e wrappedError) Get(key string) interface{} { - return e.context[key] -} - -// Del removes a key from the context. -func (e wrappedError) Del(key string) { - delete(e.context, key) -} - -// Context returns the underlying context. -func (e wrappedError) Context() map[string]interface{} { - return e.context -} - -// Definitions for IsFatalError() - -type fatalError struct { - errorWrapper -} - -func (e fatalError) Fatal() bool { - return true -} - -func NewFatalError(err error) error { - return fatalError{newWrappedError(err, "Fatal error")} -} - -// Definitions for IsNotImplementedError() - -type notImplementedError struct { - errorWrapper -} - -func (e notImplementedError) NotImplemented() bool { - return true -} - -func NewNotImplementedError(err error) error { - return notImplementedError{newWrappedError(err, "Not implemented")} -} - -// Definitions for IsAuthError() - -type authError struct { - errorWrapper -} - -func (e authError) AuthError() bool { - return true -} - -func NewAuthError(err error) error { - return authError{newWrappedError(err, "Authentication required")} -} - -// Definitions for IsSmudgeError() - -type smudgeError struct { - errorWrapper -} - -func (e smudgeError) SmudgeError() bool { - return true -} - -func NewSmudgeError(err error, oid, filename string) error { - e := smudgeError{newWrappedError(err, "Smudge error")} - ErrorSetContext(e, "OID", oid) - ErrorSetContext(e, "FileName", filename) - return e -} - -// Definitions for IsCleanPointerError() - -type cleanPointerError struct { - errorWrapper -} - -func (e cleanPointerError) CleanPointerError() bool { - return true -} - -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 -} - -// Definitions for IsNotAPointerError() - -type notAPointerError struct { - errorWrapper -} - -func (e notAPointerError) NotAPointerError() bool { - return true -} - -func NewNotAPointerError(err error) error { - return notAPointerError{newWrappedError(err, "Pointer file error")} -} - -type badPointerKeyError struct { - Expected string - Actual string - - errorWrapper -} - -func (e badPointerKeyError) BadPointerKeyError() bool { - return true -} - -func NewBadPointerKeyError(expected, actual string) error { - err := Errorf("Expected key %s, got %s", expected, actual) - return badPointerKeyError{expected, actual, newWrappedError(err, "pointer parsing")} -} - -// Definitions for IsDownloadDeclinedError() - -type downloadDeclinedError struct { - errorWrapper -} - -func (e downloadDeclinedError) DownloadDeclinedError() bool { - return true -} - -func NewDownloadDeclinedError(err error, msg string) error { - return downloadDeclinedError{newWrappedError(err, msg)} -} - -// Definitions for IsRetriableError() - -type retriableError struct { - errorWrapper -} - -func (e retriableError) RetriableError() bool { - return true -} - -func NewRetriableError(err error) error { - return retriableError{newWrappedError(err, "")} -} diff --git a/errors/types.go b/errors/types.go new file mode 100644 index 00000000..dfc70745 --- /dev/null +++ b/errors/types.go @@ -0,0 +1,340 @@ +package errors + +import ( + "fmt" + + "github.com/pkg/errors" +) + +// IsFatalError indicates that the error is fatal and the process should exit +// immediately after handling the error. +func IsFatalError(err error) bool { + if e, ok := err.(interface { + Fatal() bool + }); ok { + return e.Fatal() + } + if parent := parentOf(err); parent != nil { + return IsFatalError(parent) + } + return false +} + +// IsNotImplementedError indicates the client attempted to use a feature the +// server has not implemented (e.g. the batch endpoint). +func IsNotImplementedError(err error) bool { + if e, ok := err.(interface { + NotImplemented() bool + }); ok { + return e.NotImplemented() + } + if parent := parentOf(err); parent != nil { + return IsNotImplementedError(parent) + } + return false +} + +// IsAuthError indicates the client provided a request with invalid or no +// authentication credentials when credentials are required (e.g. HTTP 401). +func IsAuthError(err error) bool { + if e, ok := err.(interface { + AuthError() bool + }); ok { + return e.AuthError() + } + if parent := parentOf(err); parent != nil { + return IsAuthError(parent) + } + return false +} + +// IsSmudgeError indicates an error while smudging a files. +func IsSmudgeError(err error) bool { + if e, ok := err.(interface { + SmudgeError() bool + }); ok { + return e.SmudgeError() + } + if parent := parentOf(err); parent != nil { + return IsSmudgeError(parent) + } + return false +} + +// IsCleanPointerError indicates an error while cleaning a file. +func IsCleanPointerError(err error) bool { + if e, ok := err.(interface { + CleanPointerError() bool + }); ok { + return e.CleanPointerError() + } + if parent := parentOf(err); parent != nil { + return IsCleanPointerError(parent) + } + return false +} + +// IsNotAPointerError indicates the parsed data is not an LFS pointer. +func IsNotAPointerError(err error) bool { + if e, ok := err.(interface { + NotAPointerError() bool + }); ok { + return e.NotAPointerError() + } + if parent := parentOf(err); parent != nil { + return IsNotAPointerError(parent) + } + return false +} + +// IsBadPointerKeyError indicates that the parsed data has an invalid key. +func IsBadPointerKeyError(err error) bool { + if e, ok := err.(interface { + BadPointerKeyError() bool + }); ok { + return e.BadPointerKeyError() + } + + if parent := parentOf(err); parent != nil { + return IsBadPointerKeyError(parent) + } + return false +} + +// If an error is abad pointer error of any type, returns NotAPointerError +func StandardizeBadPointerError(err error) error { + if IsBadPointerKeyError(err) { + badErr := err.(badPointerKeyError) + if badErr.Expected == "version" { + return NewNotAPointerError(err) + } + } + return err +} + +// IsDownloadDeclinedError indicates that the smudge operation should not download. +// TODO: I don't really like using errors to control that flow, it should be refactored. +func IsDownloadDeclinedError(err error) bool { + if e, ok := err.(interface { + DownloadDeclinedError() bool + }); ok { + return e.DownloadDeclinedError() + } + if parent := parentOf(err); parent != nil { + return IsDownloadDeclinedError(parent) + } + return false +} + +// IsRetriableError indicates the low level transfer had an error but the +// caller may retry the operation. +func IsRetriableError(err error) bool { + if e, ok := err.(interface { + RetriableError() bool + }); ok { + return e.RetriableError() + } + if parent := parentOf(err); parent != nil { + return IsRetriableError(parent) + } + return false +} + +type errorWithCause interface { + Cause() error + StackTrace() errors.StackTrace + error + fmt.Formatter +} + +// wrappedError is the base error wrapper. It provides a Message string, a +// stack, and a context map around a regular Go error. +type wrappedError struct { + errorWithCause + context map[string]interface{} +} + +// newWrappedError creates a wrappedError. +func newWrappedError(err error, message string) *wrappedError { + if err == nil { + err = errors.New("Error") + } + + var errWithCause errorWithCause + + if len(message) > 0 { + errWithCause = errors.Wrap(err, message).(errorWithCause) + } else if ewc, ok := err.(errorWithCause); ok { + errWithCause = ewc + } else { + errWithCause = errors.Wrap(err, "LFS").(errorWithCause) + } + + return &wrappedError{ + context: make(map[string]interface{}), + errorWithCause: errWithCause, + } +} + +// Set sets the value for the key in the context. +func (e wrappedError) Set(key string, val interface{}) { + e.context[key] = val +} + +// Get gets the value for a key in the context. +func (e wrappedError) Get(key string) interface{} { + return e.context[key] +} + +// Del removes a key from the context. +func (e wrappedError) Del(key string) { + delete(e.context, key) +} + +// Context returns the underlying context. +func (e wrappedError) Context() map[string]interface{} { + return e.context +} + +// Definitions for IsFatalError() + +type fatalError struct { + *wrappedError +} + +func (e fatalError) Fatal() bool { + return true +} + +func NewFatalError(err error) error { + return fatalError{newWrappedError(err, "Fatal error")} +} + +// Definitions for IsNotImplementedError() + +type notImplementedError struct { + *wrappedError +} + +func (e notImplementedError) NotImplemented() bool { + return true +} + +func NewNotImplementedError(err error) error { + return notImplementedError{newWrappedError(err, "Not implemented")} +} + +// Definitions for IsAuthError() + +type authError struct { + *wrappedError +} + +func (e authError) AuthError() bool { + return true +} + +func NewAuthError(err error) error { + return authError{newWrappedError(err, "Authentication required")} +} + +// Definitions for IsSmudgeError() + +type smudgeError struct { + *wrappedError +} + +func (e smudgeError) SmudgeError() bool { + return true +} + +func NewSmudgeError(err error, oid, filename string) error { + e := smudgeError{newWrappedError(err, "Smudge error")} + ErrorSetContext(e, "OID", oid) + ErrorSetContext(e, "FileName", filename) + return e +} + +// Definitions for IsCleanPointerError() + +type cleanPointerError struct { + *wrappedError +} + +func (e cleanPointerError) CleanPointerError() bool { + return true +} + +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 +} + +// Definitions for IsNotAPointerError() + +type notAPointerError struct { + *wrappedError +} + +func (e notAPointerError) NotAPointerError() bool { + return true +} + +func NewNotAPointerError(err error) error { + return notAPointerError{newWrappedError(err, "Pointer file error")} +} + +type badPointerKeyError struct { + Expected string + Actual string + + *wrappedError +} + +func (e badPointerKeyError) BadPointerKeyError() bool { + return true +} + +func NewBadPointerKeyError(expected, actual string) error { + err := Errorf("Expected key %s, got %s", expected, actual) + return badPointerKeyError{expected, actual, newWrappedError(err, "pointer parsing")} +} + +// Definitions for IsDownloadDeclinedError() + +type downloadDeclinedError struct { + *wrappedError +} + +func (e downloadDeclinedError) DownloadDeclinedError() bool { + return true +} + +func NewDownloadDeclinedError(err error, msg string) error { + return downloadDeclinedError{newWrappedError(err, msg)} +} + +// Definitions for IsRetriableError() + +type retriableError struct { + *wrappedError +} + +func (e retriableError) RetriableError() bool { + return true +} + +func NewRetriableError(err error) error { + return retriableError{newWrappedError(err, "")} +} + +func parentOf(err error) error { + if c, ok := err.(errorWithCause); ok { + return c.Cause() + } + + return nil +} From 8624a873286173a763c507f3323e679dd0bd26e6 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Fri, 19 Aug 2016 12:30:53 -0600 Subject: [PATCH 09/10] fix stacktrace logging --- commands/commands.go | 6 +++++- errors/errors.go | 17 +++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/commands/commands.go b/commands/commands.go index 71ce9974..acda2cc0 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -256,7 +256,11 @@ func logPanicToWriter(w io.Writer, loggedError error) { w.Write(ErrorBuffer.Bytes()) fmt.Fprintln(w) - fmt.Fprintf(w, "%+v\n", loggedError) + fmt.Fprintf(w, "%s\n", loggedError) + for _, stackline := range errors.StackTrace(loggedError) { + fmt.Fprintln(w, stackline) + } + for key, val := range errors.ErrorContext(err) { fmt.Fprintf(w, "%s=%v\n", key, val) } diff --git a/errors/errors.go b/errors/errors.go index 26f0dab0..4a98394a 100644 --- a/errors/errors.go +++ b/errors/errors.go @@ -83,3 +83,20 @@ func Wrapf(err error, format string, args ...interface{}) error { return newWrappedError(err, message) } + +func StackTrace(err error) []string { + type stacktrace interface { + StackTrace() errors.StackTrace + } + + if err, ok := err.(stacktrace); ok { + frames := err.StackTrace() + lines := make([]string, len(frames)) + for i, f := range frames { + lines[i] = fmt.Sprintf("%+v", f) + } + return lines + } + + return nil +} From 831bcc4358bebd9d12b34ee4d4812fe7d2994194 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Fri, 19 Aug 2016 14:03:39 -0600 Subject: [PATCH 10/10] errors: remove Error prefix --- commands/command_clean.go | 2 +- commands/command_smudge.go | 2 +- commands/commands.go | 2 +- commands/uploader.go | 2 +- errors/context.go | 18 +++++++++--------- errors/errors_test.go | 14 +++++++------- errors/types.go | 8 ++++---- httputil/response.go | 10 +++++----- 8 files changed, 29 insertions(+), 29 deletions(-) diff --git a/commands/command_clean.go b/commands/command_clean.go index bbf40e84..5aad9ba6 100644 --- a/commands/command_clean.go +++ b/commands/command_clean.go @@ -44,7 +44,7 @@ func cleanCommand(cmd *cobra.Command, args []string) { } if errors.IsCleanPointerError(err) { - os.Stdout.Write(errors.ErrorGetContext(err, "bytes").([]byte)) + os.Stdout.Write(errors.GetContext(err, "bytes").([]byte)) return } diff --git a/commands/command_smudge.go b/commands/command_smudge.go index 96f0efb4..2d09ec91 100644 --- a/commands/command_smudge.go +++ b/commands/command_smudge.go @@ -86,7 +86,7 @@ func smudgeFilename(args []string, err error) string { } if errors.IsSmudgeError(err) { - return filepath.Base(errors.ErrorGetContext(err, "FileName").(string)) + return filepath.Base(errors.GetContext(err, "FileName").(string)) } return "" diff --git a/commands/commands.go b/commands/commands.go index acda2cc0..ea22a4a6 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -261,7 +261,7 @@ func logPanicToWriter(w io.Writer, loggedError error) { fmt.Fprintln(w, stackline) } - for key, val := range errors.ErrorContext(err) { + for key, val := range errors.Context(err) { fmt.Fprintf(w, "%s=%v\n", key, val) } diff --git a/commands/uploader.go b/commands/uploader.go index d91ca4dd..92a3447d 100644 --- a/commands/uploader.go +++ b/commands/uploader.go @@ -130,7 +130,7 @@ func upload(c *uploadContext, unfiltered []*lfs.WrappedPointer) { u, err := lfs.NewUploadable(p.Oid, p.Name) if err != nil { if errors.IsCleanPointerError(err) { - Exit(uploadMissingErr, p.Oid, p.Name, errors.ErrorGetContext(err, "pointer").(*lfs.Pointer).Oid) + Exit(uploadMissingErr, p.Oid, p.Name, errors.GetContext(err, "pointer").(*lfs.Pointer).Oid) } else { ExitWithError(err) } diff --git a/errors/context.go b/errors/context.go index f6986edc..af81d8de 100644 --- a/errors/context.go +++ b/errors/context.go @@ -1,6 +1,6 @@ package errors -type errorWithContext interface { +type withContext interface { Set(string, interface{}) Get(string) interface{} Del(string) @@ -9,16 +9,16 @@ type errorWithContext interface { // ErrorSetContext sets a value in the error's context. If the error has not // been wrapped, it does nothing. -func ErrorSetContext(err error, key string, value interface{}) { - if e, ok := err.(errorWithContext); ok { +func SetContext(err error, key string, value interface{}) { + if e, ok := err.(withContext); ok { e.Set(key, value) } } // ErrorGetContext gets a value from the error's context. If the error has not // been wrapped, it returns an empty string. -func ErrorGetContext(err error, key string) interface{} { - if e, ok := err.(errorWithContext); ok { +func GetContext(err error, key string) interface{} { + if e, ok := err.(withContext); ok { return e.Get(key) } return "" @@ -26,16 +26,16 @@ func ErrorGetContext(err error, key string) interface{} { // ErrorDelContext removes a value from the error's context. If the error has // not been wrapped, it does nothing. -func ErrorDelContext(err error, key string) { - if e, ok := err.(errorWithContext); ok { +func DelContext(err error, key string) { + if e, ok := err.(withContext); ok { e.Del(key) } } // ErrorContext returns the context map for an error if it is a wrappedError. // If it is not a wrappedError it will return an empty map. -func ErrorContext(err error) map[string]interface{} { - if e, ok := err.(errorWithContext); ok { +func Context(err error) map[string]interface{} { + if e, ok := err.(withContext); ok { return e.Context() } return nil diff --git a/errors/errors_test.go b/errors/errors_test.go index 4dcc0322..37c6fc8a 100644 --- a/errors/errors_test.go +++ b/errors/errors_test.go @@ -45,9 +45,9 @@ func TestBehaviorWraps(t *testing.T) { func TestContextOnGoErrors(t *testing.T) { err := errors.New("Go error") - ErrorSetContext(err, "foo", "bar") + SetContext(err, "foo", "bar") - v := ErrorGetContext(err, "foo") + v := GetContext(err, "foo") if v == "bar" { t.Error("expected empty context on go error") } @@ -56,20 +56,20 @@ func TestContextOnGoErrors(t *testing.T) { func TestContextOnWrappedErrors(t *testing.T) { err := NewFatalError(errors.New("Go error")) - ErrorSetContext(err, "foo", "bar") + SetContext(err, "foo", "bar") - if v := ErrorGetContext(err, "foo"); v != "bar" { + if v := GetContext(err, "foo"); v != "bar" { t.Error("expected to be able to use context on wrapped errors") } - ctxt := ErrorContext(err) + ctxt := Context(err) if ctxt["foo"] != "bar" { t.Error("expected to get the context of an error") } - ErrorDelContext(err, "foo") + DelContext(err, "foo") - if v := ErrorGetContext(err, "foo"); v == "bar" { + if v := GetContext(err, "foo"); v == "bar" { t.Errorf("expected to delete from error context") } } diff --git a/errors/types.go b/errors/types.go index dfc70745..e4fafd85 100644 --- a/errors/types.go +++ b/errors/types.go @@ -250,8 +250,8 @@ func (e smudgeError) SmudgeError() bool { func NewSmudgeError(err error, oid, filename string) error { e := smudgeError{newWrappedError(err, "Smudge error")} - ErrorSetContext(e, "OID", oid) - ErrorSetContext(e, "FileName", filename) + SetContext(e, "OID", oid) + SetContext(e, "FileName", filename) return e } @@ -268,8 +268,8 @@ func (e cleanPointerError) CleanPointerError() bool { func NewCleanPointerError(pointer interface{}, bytes []byte) error { err := New("pointer error") e := cleanPointerError{newWrappedError(err, "clean")} - ErrorSetContext(e, "pointer", pointer) - ErrorSetContext(e, "bytes", bytes) + SetContext(e, "pointer", pointer) + SetContext(e, "bytes", bytes) return e } diff --git a/httputil/response.go b/httputil/response.go index f82f5fa9..b01b0db6 100644 --- a/httputil/response.go +++ b/httputil/response.go @@ -110,14 +110,14 @@ func defaultError(res *http.Response) error { } func SetErrorResponseContext(cfg *config.Configuration, err error, res *http.Response) { - errors.ErrorSetContext(err, "Status", res.Status) + errors.SetContext(err, "Status", res.Status) setErrorHeaderContext(err, "Request", res.Header) setErrorRequestContext(cfg, err, res.Request) } func setErrorRequestContext(cfg *config.Configuration, err error, req *http.Request) { - errors.ErrorSetContext(err, "Endpoint", cfg.Endpoint(auth.GetOperationForRequest(req)).Url) - errors.ErrorSetContext(err, "URL", TraceHttpReq(req)) + errors.SetContext(err, "Endpoint", cfg.Endpoint(auth.GetOperationForRequest(req)).Url) + errors.SetContext(err, "URL", TraceHttpReq(req)) setErrorHeaderContext(err, "Response", req.Header) } @@ -125,9 +125,9 @@ func setErrorHeaderContext(err error, prefix string, head http.Header) { for key, _ := range head { contextKey := fmt.Sprintf("%s:%s", prefix, key) if _, skip := hiddenHeaders[key]; skip { - errors.ErrorSetContext(err, contextKey, "--") + errors.SetContext(err, contextKey, "--") } else { - errors.ErrorSetContext(err, contextKey, head.Get(key)) + errors.SetContext(err, contextKey, head.Get(key)) } } }