Cause() vs RootCause() about errors

[elithrar]

Would the below not work to achieve inspecting the underlying error? This library shouldn't change that.

if err != nil {
    // From the README
    switch err := errors.Cause(err).(type) {
    case *MyError:
        // handle specifically
    default:
        // unknown error
}

Replace *MyError with net.Error or similar to test for the types you care about. Or are you saying that you want to check the most recent error? Can you provide an example of what you're trying to achieve?

[ash2k]

Yes, I could imagine a usecase where I want to be able to check the most recent error, not the root cause.

Lets say A takes function B and calls it. If B returns an error, A wraps it and returns. If I call A and want to get error returned from B and inspect it, there is no easy way. Error from B may itself be a wrapper around some other error - we might not know anything about implementation of B, just the signature, but we might know its "contract", know that it returns enhanced errors with additional information (and may also have Cause()).

[davecheney]

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?

Because these are wrappers around the original error, the cause. It may have been a poor choice of names, but I cannot change it now.

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.

Yup, but I'm not sure what you're expecting to discover as all the error values returned from this package are anonymous.

[ash2k]

Yup, but I'm not sure what you're expecting to discover as all the error values returned from this package are anonymous.

Yes, if using this package's Wrap(). What if I want to add additional information in A to the error returned from B, not just wrap it? E.g. in Java I would create an Exception subclass with additional fields and initialize them in constructor via constructor arguments AND also pass the cause to the super() constructor. Then receiver of the exception has access to the cause and to the additional information. I think this "pattern" might be useful in Go aswell. WDYT?

i.e. this package adds cause support but there is no support for additional information attached to the wrapper except a string message. I'm not proposing something concrete, just thinking out loud.

[davecheney]

What if I want to add additional information in A to the error returned from B, not just wrap it? E.g. in Java I would create an Exception subclass with additional fields and initialize them in constructor via constructor arguments AND also pass the cause to the super() constructor. Then receiver of the exception has access to the cause and to the additional information. I think this "pattern" might be useful in Go aswell. WDYT?

You don't have to use errors.Wrap, any type that defines a Cause method will work.

type MyError struct {
     cause err
     message string
     time time.Time
     dayOfMonth int
}

func (e *MyError) Cause() error { return e.cause }
func (e *MyError) Error() string { return fmt.Sprintf("%v(%d): %s", e.time, e.dayOfMonth, e.message") }

[ash2k]

Yes, that is what I'm saying. But then errors.Cause(errorFromA) method will return me the root cause (cause of error from B) and not this enriched error, which B returned.
And also using MyError will not capture the location like Wrap() does.

[ChrisHines]

I've been wondering about this question as well. There is an asymmetry between Wrap and Cause. Wrap builds up anonymous layers of error context, but errors.Cause could strip away layers not created by Wrap—layers that are external to this package—if those error types implement causer. Because Wrap uses anonymous types there is currently no practical way to avoid this asymmetry.

This asymmetry could be resolved by adding this function.

// Unwrap removes annotations added by Wrap from err until it
// finds an error value not created by Wrap which it then returns.
func Unwrap(err error) error

[davecheney]

@ChrisHines I agree it asymmetric. Can you give me an example on what you would do with the value from Unwrap?

[davecheney]

@ash2k thank you for clarifying. At the moment there are two, incomplete, ways you can do this with the errors package.

The first is to write your own error implementation, and if you want it to report location information, add a Location() (string, int) method. Collecting that location data is left as an exercise to the user as well.

The second is to use something like errors.Wrapf(err, "some %s, that includes %v and time %v", "extra", "data", time.Now()). Which is not inspectable, in the way that an error type would be.

To give two pieces of information, possibly justifications, i'm not sure.

1. My philosophy is that errors should not be inspected by type or by value. See my recent presentation at GoCon Japan for the details, so I'm not really looking to add mechanisms to enable these patterns into the errors package, save what errors.Cause already provides.
2. Based on my interactions with the Go team, if something like this package was to be proposed for inclusion in the standard library, the smaller it is the better a chance it has of being included. To make this concrete, apart from the discussion in #15, I expect this package to move towards 1.0 rapidly, and at that point see very little enhancement.

[ash2k]

As far as I understand from slides you are suggesting to inspect error values for the behaviour, not for some type/value but what I'm trying to say is that currently this package does not provide means to get the cause of the error (to let me inspect it for behaviour).
Function B may define behaviour of returned errors just like net.Error or like this where the interface is hidden:

package x

type isMagic interface {
    IsMagic() bool
}

type myError struct {
    cause error
    message string
    time time.Time
    dayOfMonth int
    isMagic bool
}

func (e *myError) IsMagic() bool { return e.isMagic }
func (e *myError) Cause() error { return e.cause }
func (e *myError) Error() string { return fmt.Sprintf("%v(%d): %s", e.time, e.dayOfMonth, e.message) }

func IsMagic(e error) bool {
    m, ok := e.(isMagic)
    return ok && m.IsMagic()
}

// B returns errors that have IsMagic() method. Check using IsMagic() function.
func B() (int, error) {
    return 0, &myError{errors.New("Cause"), "msg", time.Now(), 0, true}
}

Then A will wrap errors from B and lets say my code wants to check for the IsMagic():

func A(b func() (int, error)) int, error {
    n, err := b()
    if err != nil {
        return 0, errors.Wrap(err, "Boom")
    }
    // Some other operation here, returning non-magic error
    err = ....
    if err != nil {
        return 0, errors.Wrap(err, "Big boom")
    }
    ...
    return n+10, nil
}

a, err := A(x.B)
if err != nil {
    e := errors.CauseOfError(err) // Cannot do this
    if x.IsMagic(e) {
         // Magic!
    }
}

I.e. I don't see how what I'm asking about contradicts your philosophy :)

[davecheney]

but what I'm trying to say is that currently this package does not provide means to get the cause of the error (to let me inspect it for behaviour).

I believe it does, but we disagree on the definition of the cause of an error. This package defines the cause as the first error value that does not implement a Cause method. I believe that what you are saying is you want to create an error type which is not the cause, as defined above (ie, it has a Cause method), but is also not the root cause of the error. Did I understand this correctly ?

[ash2k]

package main

import (
    "fmt"
    "time"
    "errors"
    pkgErr "github.com/pkg/errors"
)

type isMagic interface {
    IsMagic() bool
}

type myError struct {
    cause error
    message string
    time time.Time
    dayOfMonth int
    isMagic bool
}

func (e *myError) IsMagic() bool { return e.isMagic }
func (e *myError) Cause() error { return e.cause }
func (e *myError) Error() string { return fmt.Sprintf("%v(%d): %s", e.time, e.dayOfMonth, e.message) }

func IsMagic(e error) bool {
    m, ok := e.(isMagic)
    return ok && m.IsMagic()
}

// B returns errors that have IsMagic() method. Check using IsMagic() function.
func B() (int, error) {
    return 0, &myError{errors.New("Cause"), "myErrorOfMagic", time.Now(), 0, true}
}

func A(b func() (int, error)) (int, error) {
    n, err := b()
    if err != nil {
        return 0, pkgErr.Wrap(err, "Boom")
    }
    // Some other operation here, returning non-magic error
    err = errors.New("Something is wrong")
    if err != nil {
        return 0, pkgErr.Wrap(err, "Big boom")
    }
    return n+10, nil
}

func main() {
    a, err := A(B)
    if err != nil {
        e := DirectCause(err)
        //e := pkgErr.Cause(err)
        if IsMagic(e) {
            fmt.Printf("%v is a magic error", e)
        } else {
            fmt.Printf("%v is not a magic error", e)
        }
        return
    }
    fmt.Println(a)
}

type causer interface {
    Cause() error
}

func DirectCause(err error) error {
    cause, ok := err.(causer)
    if ok {
        return cause.Cause()
    }
    return err
}

Yes, our definitions of "cause" are different. Sorry for confusion.

1. I think it might be useful to have DirectCause() in the library (see usage example above);
2. It would be nice to be able to reuse all functionality of this package to create custom errors with cause, location and additional error-specific information (not just a string, but potentially more information of different types; why only string?). To achieve this package could export an error type that implements Location()/Cause() so that I, as the user of the package, can augment this type into my custom error type with additional information. With current implementation to achieve this I'll have to copy all of this package into my codebase to create a custom error. WDYT?

[davecheney]

Thank you for your reply. wrt to question one, see #21 for a counter suggestion.

wrt 2. This package does implement Location() and Cause() for all errors returned from both Wrap and Errorf and Location() from New. Can you please try to explain what is missing, I'm sorry but I must not be understanding the piece that you want to do which you cannot currently do.

[ash2k]

1. #21 is what I originally wanted when I created the issue. Thanks!
2. what can I, as the user of the package, do to reuse the Cause(), Error() and Location() methods from the errors package's error types in my own error types? If location and cause types were exported, I could have embedded them into my error type and "inherited" their methods.

I'm a Go newbie so I might be overlooking something. Sorry if that is the case.

[davecheney]

@ash2k I'm sorry but I will not be making either of those types public. errors.cause is trivial, there is no reason someone should have to depend on the errors package to add two fields to their type.

location is complicated, but only because this package chooses to store the program counter rather than the file and line information reported by runtime.Callers. You can find this in a publicly supported package here. https://github.com/go-stack/stack

[ash2k]

@davecheney It makes sense if we look at this package as at just a random package. But if the end goal is to build something standard, make it part of standard library then it may make sense to think about the usecase I'm describing. If the code is already in the package anyway and the package is part of std. lib AND I'm using it anyway (.Wrap()/etc) then why would I depend on something else? To me it feels like the current implementation solves part of the problem but does not solve the other part even though the code to enable the usecase is already there. Just my thoughts :)

[davecheney]

I don't really know what to say. The design of this package is based on
behaviour not implementation. The behaviour is Cause, which lets people
stack errors, and for convenience I provide a function to undo this
stacking. Everything this package does, and my philosophy around error
handling is based on behaviour, not asserting a specific type or value of
an error. I understand that sometimes this is overly idealistic, but it is
still the philosophy of this package and drives its design.

If I understand your objections, you want more functionality embedded in
this package and for everyone to import it. This is a level of source level
coupling that I do not wish to see.

Please accept my apologies on advance if I have misunderstood your
positions.

On Wed, 11 May 2016, 14:05 Mikhail Mazurskiy, [email protected]
wrote:

@davecheney https://github.com/davecheney It makes sense if we look at
this package as at just a random package. But if the end goal is to build
something standard, make it part of standard library then it may make sense
to think about the usecase I'm describing. If the code is already in the
package anyway and the package is part of std. lib AND I'm using it anyway (
.Wrap()/etc) then why would I depend on something else? To me it feels
like the current implementation solves part of the problem but does not
solve the other part even though the code to enable the usecase is already
there. Just my thoughts :)

—
You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#18 (comment)

[heyimalex]

@ash2k, check out how hashicorp/errwrap does it. The gist is that error wrapping forms a linked list (just like in this package; cause is the pointer to the next "node" so to speak), and the list is traversable via a public interface (like "Causer") with the terminal node being the first error to not implement the interface or to return no wrapped error.

This allows adding structured/typed information to errors without erasing the underlying error, and to be able to do it recursively. In addition, the package provides many helpers for walking the error list, so you're able to get to an underlying error if necessary.

The package ansel1/merry takes this same pattern to an extreme; each "node" looks like this:

type node struct {
    key, value interface{}
    err error
}

So the "list" basically becomes a map.

This strategy has its pros and its cons, but I'm not sure which I prefer yet.

[jaekwon]

Related proposed solution: #144

[puellanivis]

This is closed for good reason. You can type assert for an interface { Cause() error } to step through causation yourself.