Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
earthboundkid opened this issue Feb 1, 2024 · 7 comments
Closed

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

earthboundkid opened this issue Feb 1, 2024 · 7 comments
Labels
Milestone

Comments

@earthboundkid
Copy link
Contributor

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
Copy link

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
Copy link
Contributor Author

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
}

@seankhliao seankhliao changed the title errors: Add All(error) iter.Seq[error] proposal: errors: Add All(error) iter.Seq[error] Feb 1, 2024
@gopherbot gopherbot added this to the Proposal milestone Feb 1, 2024
@jimmyfrasche
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Contributor

CC @neild

@neild
Copy link
Contributor

neild commented Mar 22, 2024

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
Copy link
Contributor Author

I'm going to close this because I like the proposed AllAs in #66455 more than plain All and to consolidate conversations to one place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Incoming
Development

No branches or pull requests

6 participants