Skip to content

Instantly share code, notes, and snippets.

Embed
What would you like to do?
Go 2 error inspection for error groups.

The current error proposal for inspection is great. I have two minor suggestions:

  • rename Wrapper to Unwrapper
  • change the function name from "As" to "Find"

The words "As" someties indicates casting, but we are just doing a retrieval.

Handling error groups

The current error inspection proposal laments not having a way to deal with error groups. I think we should try to fix this now at the same time.

Errors have two forms of composition.

  • vertical: wrapping of errors
  • horizontal: multiple errors

The current proposal deals with vertical composition, but recognizes it has not tackled horizontal composition. For improving horizontal composition, the first step is to try to agree on a standard interface. Here is the interface provided by uber-go/multierr:

type ErrorGroup interface {
       Errors() []error
}

Note that Kubernetes has a similar interface.

When you unwrap a chain (vertically) and come to an ErrorGroup, you get just that ErrorGroup, not one of its members. What happens when you unwrap an ErrorGroup? That depends on the ErrorGroup. However, most would not unwrap to something else.

That means our errors.Find function needs to check for ErrorGroup as it traverses. We can do this with an abstracted out traversal function WalkDeep.

func Find(type E)(origErr error) (E, bool) {
	var foundErr E
	found := WalkDeep(origErr, func(err error) bool {
		if e, ok := err.(E); ok {
			foundErr = e
			return true
		}
		return false
	})
	return foundErr, found
}

Appendix: traversal implementation

package errors

// Unwrapper is a standard interface to give the next error in the chain.
// wrapping context around another error.
type Unwrapper interface {
	// Unwrap returns the next error in the error chain.
	// If there is no next error, Unwrap returns nil.
	Unwrap() error
}

// Unwrap uses Unwrapper to return the next error in the chain or nil.
func Unwrap(err error) error {
	if unErr, ok := err.(Unwrapper); ok {
		return unErr.Unwrap()
	}
	return nil
}

// ErrorGroup is an interface for multiple errors that are not a chain.
// This happens for example when executing multiple operations in parallel.
type ErrorGroup interface {
	Errors() []error
}

// WalkDeep does a depth-first traversal of all errors.
// Any ErrorGroup is traversed (after going deep).
// The visitor function can return false to end the traversal early
// In that case, WalkDeep will return false, otherwise true
func WalkDeep(err error, visitor func(err error) bool) bool {
	// Go deep
	unErr := err
	for unErr != nil {
		if more := visitor(unErr); more == true {
			return true
		}
		unErr = Unwrap(unErr)
	}

	// Go wide
	if hasGroup, ok := err.(ErrorGroup); ok {
		for _, err := range hasGroup.Errors() {
			if more := WalkDeep(err, visitor); more == true {
				return true
			}
		}
	}

	return true
}
@posener

This comment has been minimized.

Copy link

@posener posener commented Sep 19, 2018

I also thought that the horizontal composition is something that is missing in the proposal.

I have several things to say about it.

Background

There are two packages which I know about that enable aggregating several errors into a single error,
by chance, they are both named multierror:

Both of the packages organizations are heavy Go uses. I think that the value of such functionality is important
and I myself had encountered several times the need for one of those packages.

Example

var me error

if err := step1(); err != nil {
	me = multierror.Append(me, err)
}
if err := step2(); err != nil {
	me = multierror.Append(me, err)
}

return me

Value

The packages are enabling appending multiple error values into a single error values - not vertically
as proposed in the Go 2 Error Values proposal and in the github.com/pkg/errors package,
but horizontally.

I think that supporting both of those errors aggregation dimensions is important.

This could be done by adding an Append function to the new Go 2 error package.

I have no opinion about the naming of As and Wrapper, I kind of like them.

I disagree with adding an interface ErrorGroup, I think that this could be integrated seamlessly to the existing interface.

Cheers,
Eyal

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment