proposal: composite errors

[jonmayergoogle]

Propopsal: composite errors

Summary

I would like to propose a standard interface for creating composite errors.

Why

Composite errors arise when more than one problem needs to be reported. One possible use would be to report a combination of all the errors that appear when joining the results of multiple goroutines. Another possible use would be reporting multiple syntax errors when parsing a file.

This pattern has already arisen in user-contributed packages, but it would be useful to have a consistent solution.

Proposal

I'd like to propose an extension to the errors package that supports composite errors without changing any of the behavior for non-composite errors. Composite and non-composite errors are supported by all public interfaces.

// Combine combines errors into a composite error
func errors.Combine(errs ...error) error

When all arguments to Combine are nil: Combine returns nil.
When only one argument to Combine is non-nil: Combine returns that error.
When more than one argument to Combine is non-nil: Combine returns a composite error (internally, a slice of errors).

I would also suggest a couple of helper functions. Both of these functions should operate whether the underlying error is simple or composite.

// Count returns the number of distinct errors within an error.
// Returns 0 for nil, 1 for an ordinary error, or more than 2 for a composite error.
func Count(e error) int

// ByIndex returns a specific distinct index from an error, or nil if the index is greater than the number
// of distinct errors present.
func ByIndex(e error, i int) error

Sample Implementation

My proposed sample implementation is here:

https://go-review.googlesource.com/48150

Since I'm proposing a change to the global errors package, I thought I'd discuss the change here rather than just sending the code in for review. I'm also amenable to moving this code to a separate package, but "errors" feels like the right place for catching people before they re-invent this wheel.

[mvdan]

This may seem like an obvious question, but what about []error return values? Or are those a problem when satisfying an interface/type that expects the implementation to return a single error?

[mvdan]

It could also be possible to define your own type, like CompositeError, being able to obtain information about the errors it contains like err.(*CompositeError).Children(). Is there a reason why having this in the standard library would be better?

[jonmayergoogle]

mvdan part 1: that is exactly right. []error is not type-compatible with error.

mvdan part 2: my proposal does define it's own type (errorCollection), but it's private. There's no reason to expose a completely new top-level type when I can simply improve the functionality of the generic error type. The dev never needs to know whether they've got an error or a composite error, unless they are expecting composite errors and want to do some special formatting of the error string.

I also think it's cleaner to always have empty sets represented by nil, sets of one always represented by a simple error type, and sets of more than one represented by an error-compatible composite. If I create a new user-visible composite type, then the dev needs to think harder about the empty and singleton sets.

[mvdan]

There's no reason to expose a completely new top-level type when I can simply improve the functionality of the generic error type.

Either way, you're still adding a few new exported names to the errors package. This adds complexity to the standard library. I'm still not clear on why having this in the standard library is better than having it elsewhere.

Do note that there are plenty of "helper" error packages out there, like https://github.com/pkg/errors. Perhaps that would be a better home.

[jonmayergoogle]

why having this in the standard library is better

Because this is a common design pattern. Since errors are used universally throughout every go package, consistently handling composite errors makes sense.

A world in which some modules provide one kind of composite error, and other modules provide another incompatible composite error, is a bad world.

However, this question is exactly the reason why I raised this patch as a proposal rather than just sending the code for review. There is a discussion to be had here.

[jonmayergoogle]

Example of use:

var e error
e = errors.Combine(e, FunctionA())
e = errors.Combine(e, FunctionB())
e = errors.Combine(e, FunctionC())
if e != nil {
  return e
}

[cznic]

The proposal partially duplicates what scanner.ErrorList already provides.

mvdan part 1: that is exactly right. []error is not type-compatible with error.

True but

type errorList []error

func (e errorList) Error() string { ... }

is and can be extended to to provide all what this proposal is about in a few more lines.

[dominikh]

I'm somewhat in favour of the proposal, iff the Go team wants to actively promote this kind of error handling.

Having all these 3rd party package makes it harder than necessary to work with "composite errors" as a consumer of libraries.

However, we might already be too late. Third party packages do exist, are in active use, and often implement more custom functionality (such as automatic collection of stacktraces) that shouldn't be part of the standard library.

[nightlyone]

Currently any user of the proposed API which already returns or dives into multi value errors
(e.g. by type assertions) is not supported by the proposal. This makes roll-out a bit of an adventure for public APIs (e.g. Google APIs) and their consumers.

To ease the transition I propose to handle the optional interface (name debatable)

type combinedError interface {
        Len() int // returns the length of the multi value error.
        At(i int) error // returns any error within at index 0 to Len()-1 and panics, if out of bounds.
}

which is internally handled by the proposed functions as follows:

func Count(e error) int {
        if e == nil {
                return 0
        }
        if ce, ok := e.(combinedError); ok {
                return ce.Len()
        }
        return 1
}

func ByIndex(e error, i int) error {
        if e == nil {
                return nil
        }
        ce, ok := e.(combinedError)
        if ok && 0 <= i && i < ce.Len() {
                return ce.At(i)
        }
        return nil
}

Note: This assumes the internal slice type errorList implements this interface as well to ease implementation.

Exporting this interface might help the transition phase, but documenting that it's behavior might be enough.

Without such a transition path, I would consider the proposed addition too much breakage for the currentl maturity of Go ecosystem.

[bcmills]

@jonmayergoogle

this is a common design pattern

Is it? I think that claim itself needs some support. How often does the need for lists of errors come up in situations where the function must return error instead of []error?

Concrete examples would be helpful.

[dsnet]

I agree with @bcmills. When running in a single goroutine, if you ran into an error, you usually stop work and return. Thus, you only have a single error.

In the case were you have multiple goroutines, it is possible that multiple can fail, but in my experience, when multiple do fail it's because of the same issue (e.g., disk out of storage). In this situation, the first error is good enough and I definitely don't want to see all of the same errors repeated over and over. For that reason, I like what errgroup does, which fails on the first error.

If there are multiple goroutines that may return semantically-different errors, I imagine you would want some way to deduplicate that and format that in a nice way, but that seems really specific to every person's use case.

[jonmayergoogle]

@bradfitz -- this proposal changes a module, but not language syntax. Is LanguageChange appropriate here?

[jonmayergoogle]

@cznic and @nightlyone I think we're actually more in agreement than not. Have you taken a look at my proposed implementation?

https://go-review.googlesource.com/48150

I think we're roughly debating whether or not we have a "Count" function that can operate on both simple and composite errors, versus a Count function that requires an attempt to cast to a composite error, followed by a function call. If I'm understanding you correctly, my preference is still for the former. Is there an argument to be made for the latter?

[jonmayergoogle]

@bcmills -- I don't know how to resolve your comment. The need to report multiple errors arises often in my code. The proliferation of proprietary solutions indicates that I'm not alone.

My goal here is to try to create a unified but minimalist approach, unencumbered by baggage in other solutions (nothing wrong with stack traces, but they belong in individual errors, not as a default part of a composite error system).

What I'm hoping to get out of this thread to make sure that such a change is wanted, and that we make the best first cut at this that we can. Certainly, tossing this into yet another proprietary third-party module is always an alternative.

[jonmayergoogle]

Looking at scanner.ErrorList:

This looks similar in intent, but is definitely not a generalized solution. It only allows strings to be added to the error collection, not errors, and there is no support for combining composite errors.

From a use perspective, I think scanner.ErrorList looks like this:

  var errorList scanner.ErrorList
  err := FooFunction()
  if err != nil {
    errorList.Add(errorList.Len(), err.Error())
  }

If FooFunction might return a composite error, the code expands to

  var errorList scanner.ErrorList
  err := FooFunction()
  if err != nil {
    if otherErrorList, ok := err.(*ErrorList); ok {
      for _, e := range otherErrorList {
        errorList.Add(errorList.Len(), e.Error())
      }
    } else {
     errorList.Add(errorList.Len(), e.Error())
    }
  }

For both simple and composite errors, the Combine function alternative is simple:

  var err error
  err = errors.Combine(err, FooFunction());

[bcmills]

@jonmayergoogle

The proliferation of proprietary solutions indicates that I'm not alone.

Then give some concrete examples, please. If there are lots of existing solutions, then you should be able to analyze the specific use-cases they address and show how your proposal would subsume those use-cases.

[jonmayergoogle]

@bcmills -- I'll do my best.

I think it was you who steered me in the direction of sync/errgroup, which can collect multiple errors but seems to only report the first error. I also referred to hashicorp/go-multierror when thinking about this problem.

• https://godoc.org/?q=multierror -- 19 hits.
• https://godoc.org/?q=merrors -- 1 hit.
• https://godoc.org/?q=multierr -- 1 hit.
• https://github.com/search?l=Go&q=multierr -- 16 hits.
• https://github.com/search?l=Go&q=merror -- 1 hit.

replica and missionMeteora also seem to contain proprietary implementations, too.

I've also seen a reddit discussion thread talking about using channels to return composite errors.

There is probably a lot more out there than just what I've found.

[jonmayergoogle]

In fact, looking again at https://github.com/hashicorp/go-multierror/blob/master/multierror.go -- that implementation does mine one better with the inclusion of an overloadable formatting function. I had originally pushed their code aside because I wasn't sure how I felt about their error wrapping implementation, but that may have been premature.

[jonmayergoogle]

I didn't see it before now, but https://github.com/uber-go/multierr/blob/master/error.go is almost exactly what I proposed.

[gopherbot]

CL https://golang.org/cl/48150 mentions this issue.

[ansel1]

Another use case: validation errors. It's typical to accumulate errors from validating each field of a struct.

Unfortunately, looking at how existing validation libraries have tried to solve this highlights how awkward composite errors are. For example, ozzo-validation implemented a map-like composite error, but then found the need to distinguish between simple validation errors (which are composable) and more serious "internal" errors, which should supersede validation errors.

[bcmills]

The problem with accumulating errors is that there are many different predicates you could/should care about, and it's not clear that we can enumerate them cleanly in a centralized package.

For example, you might care that “all of the errors are nil or proto.RequiredNotSetError”, or “any error is non-nil”, or “all of the errors are nil or have a Temporary() bool method that returns true”.

If you return a structured type, such as []error or map[int]error, then it's clear what the caller needs to do to check those properties. However, if the result is just an error hiding a composite value, it's not obvious how to unpack that value — let alone how to wrap that error without losing its structure.

[ansel1]

Yeah, I agree. The more I poke at this, the more it seems like a bad idea to generalize this pattern too much.

[griesemer]

While scanner.ErrorList seemed like a good idea at first, I would be reluctant to advertise it now. More often than not, what's really wanted is slightly different from what scanner.ErrorList provides, and it's much simpler to just write the few lines specific error handling in those cases.

Closing in favor of a more comprehensive approach to error handling redesign.