pkg/errors

Simple error handling primitives

errors's Issues

Cause() is broken

Hi,

Looking at the following code snippet:

func Cause(err error) error {
    type causer interface {
        Cause() error
    }

    for err != nil {
        cause, ok := err.(causer)
        if !ok {
            break
        }
        err = cause.Cause()
    }
    return err
}

How is err ever going to be nil? I am getting panic: interface conversion: error is errors.wrapper, not *net.OpError after calling errors.Cause...

go-stack/stack and pkg/errors should support the same formatting verbs

At the moment, errors.Frame supports a subset of the verbs that stack.Call supports.

Obtaining only message

Is it possible to get the message passed to errors.Wrap(err, message)? What I specifically mean is message itself, without anything from err.

My use case is that I wrap underlying errors, and pass in a human friendly message, that I would like to expose to customers via our APIs. Currently I'm parsing the message out by splitting by : , which is brittle.

goimports miss

For many files it'll be enough to use just errors.New(), which result in goimports incorrectly import "errors" instead of "github.com/pkg/errors". This mistake will be hard to spot manually while code review and it'll show itself only when one of these errors will be output into log without stack trace.

Not sure is you can do anything with this - maybe just note this subtle trap in package documentation?

WithMessage appends message after associated stack traces instead of before

After amending an error using errors.WithMessage, printing the error with %+v prints the extra information after the stack trace instead of with the original message. For example:

package main

import (
	"log"

	"github.com/pkg/errors"
)

func a() error {
	return errors.New("error details")
}

func b() error {
	if err := a(); err != nil {
		return errors.WithMessage(err, "additional details")
	}
	return nil
}

func main() {
	if err := b(); err != nil {
		log.Fatalf("%+v", err)
	}
}

This prints:

2017/02/06 14:34:13 error details
main.a
	.../example.go:10
main.b
	.../example.go:14
main.main
	.../example.go:21
runtime.main
	/usr/local/Cellar/go/1.7.4_2/libexec/src/runtime/proc.go:183
runtime.goexit
	/usr/local/Cellar/go/1.7.4_2/libexec/src/runtime/asm_amd64.s:2086
additional details
exit status 1

As mentioned in #75, it seems like it would be more helpful to include the additional information with original error message. When using %v with the same example, it prints messages in this way now:

2017/02/06 14:33:32 additional details: error details
exit status 1

[Feature request] Add fields

First of all, thanks for creating and sharing a wonderful library!

I think a stack trace with an error message is a big help for debugging, but I also want variable values at the stack where the error occurred. I would like to log those values at the higher stack where I received the error.

So how about adding something like NewWithFields or WrapWithFields?

I implemented a proof of concept at Comparing pkg:a2d6902c6d2a2f194eb3fb474981ab7867c81505...hnakamur:add_fields · pkg/errors

Could you tell me what do you think?
Thanks!

Help to print only one stack trace when multiple wrap

When I return an error I wrap because i'm never sure if it's already wrapped or not.
Then when i print the error with "%+v" it print all the stacks.
How can I do to only print the last stack but also all the messages ?

package main

import (
	"fmt"

	"github.com/pkg/errors"
)

func three() error {
	return errors.New("three")
}
func two() error {
	return errors.Wrap(three(), "two")
}
func one() error {
	return errors.Wrap(two(), "one")
}
func main() {
	err := one()
	fmt.Printf("%+v", err)
}

It print

three
main.three
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:10
main.two
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:13
main.one
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:16
main.main
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:19
runtime.main
	/usr/local/go/src/runtime/proc.go:183
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:2086
two
main.two
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:13
main.one
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:16
main.main
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:19
runtime.main
	/usr/local/go/src/runtime/proc.go:183
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:2086
one
main.one
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:16
main.main
	/home/wilk/projets/logics/medicoop/src/medicoop/t/t.go:19
runtime.main
	/usr/local/go/src/runtime/proc.go:183
runtime.goexit
	/usr/local/go/src/runtime/asm_amd64.s:2086

I would like to print only the stack of three.

Thanks (and sorry for my english)

A Sprint function for services that use json logging

Hello,

In the services that we run we utilise json logging for all of our error logs, I had a peruse and it seems that the current print functions either require a writer or go straight to os.Stderr.

I think it would be very helpful for us, as well as for other users of this package, if there was a function that performs the same as fprint but returns a string for use.

I've created a proof of concept here: https://github.com/jimah/errors/blob/master/errors.go#L253

For example output please see the tests.

Thank you for your time!

Add errors.Errorf method

Hi, would it make sense to have a Newf method to avoid having to write this?

 errors.New(fmt.Sprintf("something %v", x))

If you think it does I can make a PR.

Thank you

Consider StackTrace() []uintptr

Hi,

I am trying to write a library to interoperate with wrapped errors.

There are many error wrapping libraries, but I can only get the StackTrace() of this one by importing this one. This creates problems trying to write library agnostic code.

With Cause() error, I can simulate an error exposing this interface inside my library, meaning I can interoperate with pkg/errors without needing to import it, as well as define an API that other libraries can implement without forcing them into pkg/errors API.

With StackTrace() StackTrace I am forced to tell people wanting to work with my library to import errors, just for the StackTrace object, just to wrap their own stack trace with it.

This is a general problem with APIs that don't use Go's core types as parameters. Since a stack trace is just an array of pointers, if the implementation was StackTrace() []uintptr, then other error wrapping libraries could standardize to this and we could all interoperate.

I would also say that pretty printing a stack trace is behavior reasonably independent of wrapping errors itself. By returning the stack trace raw, you allow libraries to choose how it should be pretty printed: with maybe errors package providing a default implementation like it does now.

Ignore Callstack above printing location

Is it possible when printing an error to only print the Callstack that is generated from this position onward, but ignore the Callstack that happened prior to the method that prints the error?

For instance if I am using a webserver, such as negroni, it includes a lot of function calls which happen before reaching my code. These calls are still included in the stack trace, and clutter it, but I am not particularly interested in them.

Is there a way to ignore them?

If there isn't at present, maybe something can be done to match the runtime.Callers at the printing location with the stack trace inside the error, and ignore the matching part?

Proposal: How to use Type system to force use of decorated errors

First off: thanks for a really great library and hope this is the right place for this.

Within the confines of my particular application I'd really like to make sure that ALL errors have been decorated with a stack trace. I feel like the best way to do this would be with the type system, perhaps:

func MyRecordCreationFunction(some args...) (record, errors.ErrorWithStack) {
   record, err := orm.CreateObj("...")
   if err != nil {
     return nil, errors.WithStack(err)
   }

   err = record.hydrate()
   if err != nil {
     return nil, err // <- fails to compile, type error does not implement method StackTrace() ...
   }

   return record, nil
}

Unfortunately this is quite challenging with the types that this package decides to export right now. Is there another way to achieve this? Or is this worthy of a PR?

Thanks for your time.

Export stack?

Exporting stack would let you get the nice stack formatting that you get with actual errors.

Proposal: errors' context

Hello,

What do you think of errors' contexts like net/context or simple map[string]interface{} stored with the original error?
It would be really handy; example:

var ErrNotFound = error.New("Page not found!")
...

return errors.Wrap(ErrNotFound,pageTitle).WithContext(`http`,404)

this way the imaginary http handler won't need to know about ErrNotFound; it would just grab the value from the context. It could be even better:

var ErrNotFound = errors.Wrap(error.New("Page not found!"),"").WithContext(`http`,404)
...
return errors.Wrap(ErrNotFound,pageTitle)

however, this approach would lose stack data; it would require the stored stack's reset.

The context or map can be initiated on demand (when adding first value), so it wouldn't create some significant overhead if context is not used.

Question on appropriate ways to use errors pkg with additional data in the error

Hi Dave and everyone,
Thank you for the talk at GopherCon and the blog post on error handling in go.
I am wondering what the best way would be to use this package, in combination with returning some additional data about the error.
In your talk, you gave the example of implementing a type temporary interface, and I like that idea very much. But what would you do if you wanted to return a bit more data then could fit into a few boolean interface, such as an error code? My example below just has one field, for simplicity, but lets assume there are more fields with additional context.

Here is what I've come up with (simplified version):

type MyError struct {
    error
    Code  int
}

func (e MyError) Cause() error {
    return errors.Cause(e.error)
}

const UnknownCode = -1

func ErrorCode(err error) int {
    if err == nil {
        return 0
    }
    if e, ok := err.(MyError); ok {
        return e.Code
    }
    return UnknownCode
}

func (e MyError) Format(s fmt.State, verb rune) {
    switch verb {
    case 'v':
        if s.Flag('+') {
            _, _ = fmt.Fprintf(s, "%+v\n(Code:%d)", e.error, e.Code)
            return
        }
        fallthrough
    case 's':
        _, _ = io.WriteString(s, e.Error())
    case 'q':
        _, _ = fmt.Fprintf(s, "%q", e.Error())
    }
}

func (e MyError) Error() string {
    return fmt.Sprintf("%s; (Code:%d)", e.error, e.Code)
}

This approach is sort of based off of how error handling is done in the golang gRPC library (https://github.com/grpc/grpc-go/blob/master/rpc_util.go#L343), but it also makes use of your package, allowing for wrapping and unwrapping errors.

An example usage would be:

var err error = MyError{errors.Wrap(innerErr, "outer error"), 255}

if err != nil {
    if ErrorCode(err) == 255 {
        // handle 255
    }
    // handle other stuff
}

I'm curious if there might be a better way. Some ideas are:

ErrorCode could return an error if the error argument is not of type MyError, but this could make error handling much more verbose:

if err != nil {
    code, codeErr := ErrorCode(err)
    if codeErr != nil {
        panic("passed in wrong error type or shadowed an err...")
    }
    if code == 255 {
        // handle 255
    }
    // handle other stuff
}

Code could be an interface, but it doesn't look quite as nice and is also verbose, and I wonder why the gRPC library didn't go with that way...

type Code interface {
    Code()  int
}

if err != nil {
    if coder, ok := err.(Code); ok {
        if coder.Code() == 255 {
            // handle 255
        }
    }
    // handle other stuff
}

Capitalise trace in Stacktrace

Have you considered changing Stacktrace to StackTrace? There are several examples on the internet where stacktrace is used as one word. However most examples seem to be two separate words suggesting that the Trace should be capitalised. The Go runtime and debug packages mention stack trace as two separate words in documentation.

I know this is pedantic and opinionated so feel free to ignore.

Expose Frame's pc, file, line and function name

Hello,

I'm currently writing a "bridge" for the Sentry Go library: https://github.com/getsentry/raven-go
I create a StackTraceFrame manually with https://godoc.org/github.com/getsentry/raven-go#NewStacktraceFrame
I need to be able to extract file/line information from a Frame https://godoc.org/github.com/pkg/errors#Frame

Could you add methods on a Frame to get:

• pc (by the way, I don't understand the "-1" logic)
• file
• line
• function name

Thank you

Why call callers() on every wrap?

Every time an error is wrapped, callers() is called. This seems wasteful to me. I think the root error's stack trace is the most important and almost always what I want.

I think if the error that is being wrapped implements stackTrace interface, that error's stack trace should be used instead, and the call to callers() can be skipped. For sure, any stack traces in errors between the top and bottom of the error stack are ignored.

I think my optimal format message would include the wrapped msg's all down the various error wraps with a stack trace at the bottom.

Migrate `runtime.Callers/2`

Given Slide 25 from talk: Mid-stack inlining in the Go compiler (external) it seems like errors.callers should migrate.

Basically: "Avoid FuncForPC, use CallersFrames".

What is the reason behind stacktrace being private?

According to errors pkg documentation to retrieve the stack trace of an error or wrapper I should try to use interface:

type stackTracer interface { StackTrace() errors.StackTrace }

The stackTracer interface is private. Does it mean, that you don't want it to be an API for errrors library and it can change its behaviour?

Proposal/Question: Type assertions on the entire error stack

Originally, checking if a error matches a given interface, you would simply m, ok := err.(myInterface). With pkg/errors, it's now m, ok := errors.Cause(err).(myInterface) (Basing this off of the writing here: http://dave.cheney.net/2016/04/27/dont-just-check-errors-handle-them-gracefully)

However, this falls apart if your error stack has myInterface in the middle, since errors.Cause(err).(myInterface) only checks the bottom of the stack:

err := errors.New("Sample error")
// because myCodedError is in the middle of a stack, we have no way
// of getting at Code without writing a custom function that is a near 
// clone of errors.Cause()
err = &myCodedError{code: 404, err: err}
err = Wrap(err, "Wrapping 1")
err = Wrap(err, "Wrapping 2")
err = Wrap(err, "Wrapping 3")

Question: What is the best way to handle this? Custom functions for each of your interfaces which may
get paired with a causer. A generalized function added to pkg/errors?

I've thrown up some code here which shows the varying cases and solutions https://play.golang.org/p/E8eJeWbOuV

Proposal: add a package-level StackTrace() method

I think it would be convenient to have a package-level StackTrace(err error) StackTrace method, as a bit of an analog to the Cause(err error) error method, which returns the earliest stack trace found in the composite error value.

This is something I have implemented in at least three packages myself now, but it seems likely to be of value to others, so I'm beginning to think perhaps it belongs here.

The reason it's not sufficient to simply call err.StackTrace(), is that an error may have been Wrap()ed multiple times, in which case one generally (if not always) wants the earliest stack trace. And:

cause := err.(causer)
stackTrace := cause.(stackTracer)

doesn't work, because generally the cause does not include a stack trace.

So my proposal is to add a package-level StackTrace() method such that one can do, for instance:

if stackTrace := errors.StackTrace(err); stackTrace != nil {
    fmt.Printf("Stacktrace: %+v\n", stackTrace)
}

If this proposal meets general approval, I can quickly submit a PR for further consideration.

Add Wrapf(cause error, format string, args ...interface{}) error

Instead of this

func Wrap(cause error, message string) error

Have these two

func Annot(cause error, a ...interface{}) error
func Annotf(cause error, format string, a ...interface{}) error

In my opinion, Wrap() doesn't convey the purpose of the function as well as Annot() does. Finally, supporting variadics keeps things consistent with everything else.

Why implement errors.Errorf when we have fmt.Errorf?

More of a package design question, but why have errors.Errorf when we already have it in the fmt package?

Is it just so we have a single place for all our erroring needs or is it different in some non-obvious way?

Sprint that wraps Fprint

This topic was partially covered in #20
Given the ping-ponging of that issue I wanted to create a new issue that was solely focused on the smallish proposed Sprint utility that could conceivably take the form:

func Sprint(err error) string {
    var b bytes.Buffer
    errors.Fprint(&b, err)
    return b.String()
}

Dave Cheney cited the reason for denial being

Fprintf is more general and can be used to implement the three line helper you suggested.

While this is true, I would like to have a thought experiment on the usage of the errors library. One of the primary benefits of this library is the ability to wrap errors as they pass up a call stack in order to provide better contextual information when the error bubbles up to a stopping point. This contextual information is then only useful if is is then extracted and reported in some way.

My experience has been two-fold: That this reporting is done by passing the generated information to a logging framework, and that logging frameworks don't expose an io.Writer. This is certainly true of the log package. So while the existing Fprint function is certainly more flexible, it is much less useful. This leaves what I believe is the majority of consumers with the choice of repeating the three-liner ad-nauseam, or creating a utility to wrap errors and import that utility (of course there will also be cases of doing both in the same code base).

I understand and respect that the bar is high for increasing the API footprint of this library. I ask that you reconsider this proposal as it would make the greatest utility that the errors package provides fit better with the part of the standard library it will primarily be used in conjunction with.

Proposal: Optional cause

The current errors.Cause method will gladly return a nil'error if given a nil error (this is expected). However, if the bottom of your error stack is a Causer that also returns nil, Cause returns nil. Example here: https://play.golang.org/p/wQa9urzGX8

The proposed change is simple enough as checking for nil in the errors.Cause loop (If there are additional complexities I haven't considered, please let me know).

func Cause(err error) error {
    type causer interface {
        Cause() error
    }

    for err != nil {
        cause, ok := err.(causer)
        if !ok {
            break
        }

        if err2 := cause.Cause(); err2 != nil {
            err = err2
        } else {
            break
        }
    }
    return err
}

Make stackTracer interface public

I read https://godoc.org/github.com/pkg/errors#hdr-Retrieving_the_stack_trace_of_an_error_or_wrapper
and learn you need to the stackTracer interface yourself.

I understand the intension is exposing APIs as little as possible to enhance the future API compatibility.
However, if everyone must defines the stackTracer interface to retrieve stack traces, it is actually an implicit interface. I think it is not good.

So how about make the stackTracer interface public and adding StackTracer interface?

TestTrimGOPATH fails when vendored

Due to using a simple string inequality test this function will fail if the errors package is located in a vendor subdirectory:

--- FAIL: TestTrimGOPATH (0.00s)
        stack_test.go:165: stack_test.go:9: want "github.com/pkg/errors/stack_test.go", got "example.org/test/vendor/github.com/pkg/errors/stack_test.go"

A simple fix is to replace lines 163-166 with the same testFormatRegex function used elsewhere:

want := tt.want
if want != got {
    t.Errorf("%v: want %q, got %q", tt.Frame, want, got)
}

to

testFormatRegexp(t, got, "%s", tt.want)

This also requires changing the line numbers on the rest of the file, so a pull request is incoming shortly. This will also slightly change the error message returned when this test fails but I am not sure if that of any great consequence.

Create some form of MultiError type

Note: the scope of this issue has tightened, see #15 (comment)

This is an issue for discussion of a potential MutliError (or other name...) type API that allows the accumulation of multiple errors in a collection which still implements the error interface but allows retrieval of the individual errors.

There are a few other options out there: https://godoc.org/?q=multierror but since github.com/pkg/errors already implements, in my opinion, the canonical best practices for error handling in Go it would be be nice if it included this feature.

feature: Location(err)string

really nice project!
I only miss one thing while using logrus

    log.WithFields(logrus.Fields{
        "code": "HandlerCheckBla",
        "user_id": user_id,
        "last_check": last_check,
                 "err_location":  errors.Location(err),   // <-------
    }).Error(err)

could you please add something like ...

func Location( err error) string  {
    str := ""
    for err != nil {
        location, ok := err.(locationer)
        if ok {
            file, line := location.Location()
            str += fmt.Sprintf("%s:%d: \n", file, line)
        }
        cause, ok := err.(causer)
        if !ok {
            return str
            break
        }
        err = cause.Cause()
    }
    return ""
 #}

to errors.go ... ? 👍

API Naming / consistency

For creating errors we have:

func New(message string) error
func Errorf(format string, args ...interface{}) error

If instead we had

func Error(args ...interface{}) error
func Errorf(format string, args ...interface{}) error

or

func New(args ...interface{}) error
func Newf(format string, args ...interface{}) error

we would go more along the lines of eg. the fmt package.

Benchmark

Hi Dave,

I like the API. I'm using this in govendor now.

When working on another project lower level lib, I thought I'd like to look at the performance impact. I'm not too concerned, but I wanted some numbers to put behind me before I used it. I didn't find a benchmark in this repo so I wrote one:

package errors

import (
    "fmt"
    "testing"

    "github.com/pkg/errors"
)

func noErrors(at, depth int) error {
    if at >= depth {
        return fmt.Errorf("no error")
    }
    return noErrors(at+1, depth)
}
func yesErrors(at, depth int) error {
    if at >= depth {
        return errors.Errorf("ye error")
    }
    return yesErrors(at+1, depth)
}

const stacks = 100

func BenchmarkNoErrors(b *testing.B) {
    var err error
    b.ReportAllocs()
    for i := 0; i < b.N; i++ {
        err = noErrors(0, stacks)
    }
    b.StopTimer()
    b.Logf("%v", err)
}

func BenchmarkErrors(b *testing.B) {
    var err error
    b.ReportAllocs()
    for i := 0; i < b.N; i++ {
        err = yesErrors(0, stacks)
    }
    b.StopTimer()
    b.Logf("%v", err)
}

It looks like the cost per op is roughly 1000-3000 ns. Which isn't a concern for me. But I'm glad to know it isn't more expensive.

[feature] errors.Unwrap

We have Wrap which takes an error and returns a new error which supports Cause and will return the original error.

This issue is to discuss adding a complement to Wrap, Unwrap is specified as follows

 // Unwrap reverses the Wrap process above, returning the direct cause
 // of the error. If the error does not support Cause, or the error is nil, the 
 // error value is returned unmodified.
 func Unwrap(err error) error 

For discussion

• Is this useful enough to justify adding to the package, vs doing this with a switch statement ?
• Is the API appropriate, specifically in the case of an error which does not have a cause.

See also #18

frame.Format: %+s prints more than "path of source file relative to the compile time GOPATH"

Change: 7896481?diff=unified#diff-7dc02266d93e96cde6ba279eadbcac8fR62
Doc:

errors/stack.go

Line 49 in 785921b

// %+s path of source file relative to the compile time GOPATH

Changing back to io.WriteString(s, trimGOPATH(fn.Name(), file)) gives the expected output.

This commit was merged 6 months ago; might break people to change it now. Would you accept a PR to put the previous behavior under %#s and an update to the documentation?

MIT or BSD-2-clause?

I just noticed that:

• At the bottom of README.md, it says the license is MIT.
• However, the LICENSE file contains the BSD-2-clause license.

Thanks!

Wrap argument name `cause` could be `err`

This might seem a little strange, but I think cause as an argument name to Wrap and Wrapf is unnecessary. I wonder if err might be good enough?

Here are some reasons:

1. it's obvious that we're wrapping an error that caused the need to add context in the first place
2. there's only one argument of type error
3. it is possible the argument is not the 'root' cause, so in a sense, isn't the 'cause'
4. IDE plugins will autofil and most of the time, the variable name would be err so you could just tab past it

Very minor issue, and mainly I'm trying to save myself two key-presses each time I use this (which I will every day).

Proposal: verbose errors

In many cases when we're generating an error we know that the particular error, if it happens, is indicative of a bug. In those cases, it's very useful if the error message contains more information (like file:line).

Errors might be handled at a different layer (e.g. sent out to a client using %s) where the original context of the error is lost.

The proposal is to add a version of Errorf which results in a "verbose" error which automatically includes the file:line where it was generated as part of the error message.

Proposal: output stack trace if debugging is enabled

In other words, it should be possible for a user at runtime to provide a --debug flag to a binary and then the binary should be able to easily make any error output a stack trace.

The nicest way of doing this (IMO) would be to have something like logrus.LogLevel (except it would be something like errors.EmitStack(bool) that would determine whether "%s" and similar format strings will output a stack trace.

Currently an application writer has to make a decision whether they will make their program debuggable (use %+v everywhere) or helpful to users (use %s everywhere).

variadic Wrap

Example in http://dave.cheney.net/2016/06/12/stack-traces-and-the-errors-package

f, err := os.Open(path)
if err != nil {
        return errors.Wrapf(err, "failed to open %q", path)
}

is actually good example why Wrap() should be variadic - os.PathError already contains Op and Path, so there is no sense to add same thing once again, all we need here is add stack details.

Another example is jsonrpc2.Error type. It is somewhat unusual because it's Error() returns error details serialized into JSON (this is needed to pass thru net/rpc internals while keeping error details). So, if we do errors.Wrap(jsonrpc2_err_here, "") and then net/rpc will internally call Error() then we'll get ": {…json here…}" and that's extra ": " prefix will break restoring error details when it comes out from net/rpc.

Providing variadic errors.Wrap(err) which didn't add any prefix will solve both issues.

Cause only returns the last item in the error chain

When I create a chain of errors (using errors.Wrap) and try to peel off the outer error (using errors.Cause) it will traverse all the way to the last error in the error chain.
For example:

err := errors.Wrap(errors.Wrap(errors.Wrap(errors.New("error1"), "error2"), "error3"), "error4")
fmt.Println(errors.Cause(err))
//output: "error1", expected output: "error3: error2: error1"

To me this looks like unexpected behaviour.

[question] Recommendations for low-level pkgs not using github.com/pkg/errors

RE: https://twitter.com/davecheney/status/758764120468357120 //cc @davecheney

What is your recommendation for low-level pkgs that don't have (and likely won't ever have) any 3rd party dependencies, ie. DB drivers? Should they return errors of certain types satisfying behavior, ie. _, ok := err.(db.ErrTemporary)? Or do you have any other suggestions?

Wrap(nil) should panic

I ran into an ugly bug today where I accidentally did this:

err := stuff()
if err != nil {
  return err
}
result := moreStuff()
if len(result.Failures) > 0 {
  // ... do stuff with failures ...
  return errors.Wrap(err, "Oops")
}

This returns nil because err is nil. The code wanted to make a new err, but it didn't. Due to Go's error conventions, you tend to have a single shared err shared across nested scopes like this, and one has to be careful about what it contains. It took me 10 minutes to realize that err was nil here.

In my opinion, Wrap(nil) should panic, because it makes no sense. The intent of the caller is clearly to wrap an error value (you shouldn't call Wrap if you don't have an error value), and you're telling it to wrap something that doesn't exist. I don't think this is controversial, but I think that, at best, it should return a new error with a nil cause.

I can see an opposite argument that "blanket wrapping" allowed:

return errors.Wrap(doStuff(), "it failed")

However, I think that's a special cause that ought to be relegated to something like MaybeWrap().

Helper to handle errors for the chain functions (that return errors).

Example of helper function: https://github.com/msoap/shell2http/blob/master/shell2http.go#L430
Example of usage: https://github.com/msoap/shell2http/blob/master/shell2http.go#L409

If such a feature would be useful? I do PR

panic: values with embedded slices uncomparable errors

Please see the stack trace here: https://travis-ci.org/opencontainers/image-spec/builds/132403826, described by opencontainers/image-spec#84.

Looks like #27 introduced or exposed a new error type that uses a slice and thus cannot be compared without introducing a panic. We picked this change up through an update to blackfriday, but this could happen anywhere.

While I understand that this library is based on concepts of no longer using sentinel values, this is already a common practice in existing code. There is also limited ways for a caller to avoid the panic.

I suggest that errors with a slice value use a pointer to the error value to avoid this trap in the future. For example, instead of building a stack{}, use &stack{}. This will ensure that existing code can make error comparisons while new code can begin to adopt this great package.

Better stacks for caught panics

First of all, this is a great package and I am using it to good effect in my code, helping both my test and production errors become much more traceable.

However, there is one piece of polishing I was hoping for, and I don't know how to implement it cleanly. I have an http server, and at the top level is a middleware, which catches panics and returns a 500 Error and logs them (both to a file, and an error reporting service). My middleware contains this for catching panics...

	defer func() {
		if rec := recover(); rec != nil {
			rest.ErrorHandler(rw, rec)  // this writes a custom 500 error 
			var wrap error
			if err, ok := rec.(error); ok {
				wrap = errors.Wrap(err, "Panic Handler")
			} else {
				wrap = errors.Errorf("%v", rec)
			}
			logger.WithError(wrap).Error(rec)
		}
	}()

And the error looks like this:

File "/home/jenkins/workspace/backend-tileserver-staging/src/panono/code/server/middleware/recovery.go" line 26 in middleware.recoveryMiddleware.ServeHTTP.func1
File "/usr/local/go16/src/runtime/asm_amd64.s" line 472 in runtime.call32
File "/usr/local/go16/src/runtime/panic.go" line 426 in runtime.gopanic
File "/home/jenkins/workspace/backend-tileserver-staging/src/panono/code/server/tileserver/tileserver.go" line 120 in tileserver.(*server).tileV5Handler
...

(Line 26 is the errors.Wrap line btw)

My wish would be to write something like:

if rec := recover(); rec != nil {
			rest.ErrorHandler(rw, rec)  // this writes a custom 500 error 
			wrap := errors.FromPanic(rec)
			logger.WithError(wrap).Error(rec)
		} 

and that the stack trace would start with the line that threw the panic.

File "/home/jenkins/workspace/backend-tileserver-staging/src/panono/code/server/tileserver/tileserver.go" line 120 in tileserver.(*server).tileV5Handler

There is a line in callers() that gets the stack: n := runtime.Callers(3, pcs[:]) Maybe this depth could be controlled by WithPanic to return n := runtime.Callers(6, pcs[:]) in that case. But I am sure there are cleaner solutions.

I look forward to any feedback on this proposal.

Cause() vs RootCause()

I just came across this package and wondering why package's Cause() function returns the root cause and not just the cause of the passed error value?
I can imagine a situation where I want to inspect the cause of the error and check for some additional information in it, etc. So, to get that information I'll have to do a check for Cause() presence myself, call it, do a type check for additional information methods and only then call them. An additional method that returns the cause might be useful here.

ps. I wonder why this is not part of the standard library. Seems to be a very useful pattern.

add Newf

There is no much sense to mimic fmt.Errorf, and to avoid stutter it may make sense to rename Errorf to Newf (or just add Newf to keep compatibility with current API).

Proposal: errors.Pop()

Hello, we are wrapping our errors in our grpc service, and we came across a situation where we wanted to return the top of the wrapped errors in the return of our endpoints.

For reference, a typical grpc endpoint signature looks like this:

func (s *Server) Hello(ctx context.Context, req *pb.HelloReq) (*pb.HelloResp, error) {
  // ...
}

The method signature has an error type as the second return value. We've written grpc middleware that receives the return values of this function, and does the proper logging, metrics, etc, before ultimately returning to the requesting client.

In the middleware, we log the entire error chain, but what we'd like to return to the grpc client in the error return param is the top error in the chain. So I'm proposing the ability to do this:

first := errors.New("first")
second := errors.Wrap(first, "second")
third := errors.Wrap(second, "third")

// prints the entire chain:
third.Error() // [ third: second: first ]

// proposed option:
errors.Pop(third) // returns an error type with ONLY "third" and no chain attached

Currently there's no easy (that I know of 😄 ) way to get the top most of an error chain. This would allow us to do so.

Thoughts?

Add to README a recommendation/recipe for multi errors reporting/handling

Incorrect file returned by Location when vendored or in a different part of a multipart GOPATH.

To reproduce:

$ go get github.com/ChrisHines/pkg-errors-test
$ pkg-errors-test
ors-test.go:8: where am I?

To reproduce the multipart GOPATH scenario copy the pkg-errors-test repo to a different part of a mulitpart GOPATH from github.com/pkg/errors and delete the vendor folder. In that case you might see something like:

$ go run pkg-errors-test.go
hub.com/ChrisHines/pkg-errors-test/pkg-errors-test.go:8: where am I?

The exact output will depend on the path length delta between the two parts of the GOPATH.

I believe this can be fixed using a technique like what I have done here: https://github.com/go-stack/stack/blob/master/stack.go#L96.

I will work on a PR for this issue.