golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.08k stars 17.55k forks source link

proposal: errors: Add All(error) iter.Seq[error] #65428

Closed earthboundkid closed 6 months ago

earthboundkid commented 7 months ago

Proposal Details

This is a follow up to #53435. It is now possible to build a tree of wrapped errors, but there is no convenient way to iterate through all members of the tree. It should just do a simple depth-first traversal of all the errors it finds with Unwrap() error/Unwrap() []error. The main use case for this is some error reporting service that wants to log the concrete types of all the sub-errors it finds in an error.

DeedleFake commented 7 months ago

A corollary: The behavior should probably not be dependent on the wrap status of the passed error, so calling All(err) where err doesn't wrap any other errors should still yield an iter.Seq that just yields err.

earthboundkid commented 7 months ago

Yes, it should handle all of that automatically. I think this is a working implementation:

func All(err error) iter.Seq[error] {
    return func(yield func(error) bool) {
        all(err, yield)
    }
}

func all(err error, yield func(error) bool) bool {
    if err == nil {
        return true
    }
    if !yield(err) {
        return false
    }
    if wrapped := errors.Unwrap(err); wrapped != nil {
        return all(wrapped, yield)
    }
    multi, ok := err.(interface{ Unwrap() []error })
    if !ok {
        return true
    }
    for _, suberr := range multi.Unwrap() {
        if !all(suberr, yield) {
            return false
        }
    }
    return true
}
jimmyfrasche commented 7 months ago

What are the use cases? I can see wanting to walk the error tree but I'd imagine you'd need something more like a file system walker where you can skip branches and get the context (parent/index/kind of wrapping)

earthboundkid commented 7 months ago

I'm imagining something like Sentry.io recording the tree of an error for debugging purposes. In those cases, you would want all of the children, but yes you'd also want to know what was the parent and what was the child so you can represent that in the UI. So maybe this is something third parties should do for themselves…

ianlancetaylor commented 7 months ago

CC @neild

neild commented 6 months ago

Sorry, I missed this when it was opened.

I'll copy my comment from #66455:

Iterating the error tree became substantially more complicated when we introduced multiple-wrapping. Prior to then, you could just call errors.Unwrap in a loop, which isn't so bad. I seem to recall that we considered adding a tree-iterator, but decided to wait until we figured out what the iterator API was going to look like.

We've got an iterator API now, so it seems reasonable to add an error-tree iteration function.

The name All as proposed here seems more in line with the nomenclature we're using for iterators than errors.Iter in #66455. I think the idea in #66455 of providing a way to iterate over errors that match a specific type (combining All and As) seems useful enough to be something we should do as well.

earthboundkid commented 6 months ago

I'm going to close this because I like the proposed AllAs in https://github.com/golang/go/issues/66455 more than plain All and to consolidate conversations to one place.