uber-go/multierr

Combine one or more Go errors together

multierr

multierr allows combining one or more Go errors together.

Features

• Idiomatic: multierr follows best practices in Go, and keeps your code idiomatic.
  • It keeps the underlying error type hidden, allowing you to deal in error values exclusively.
  • It provides APIs to safely append into an error from a defer statement.
• Performant: multierr is optimized for performance:
  • It avoids allocations where possible.
  • It utilizes slice resizing semantics to optimize common cases like appending into the same error object from a loop.
• Interoperable: multierr interoperates with the Go standard library's error APIs seamlessly:
  • The errors.Is and errors.As functions just work.
• Lightweight: multierr comes with virtually no dependencies.

Installation

go get -u go.uber.org/multierr@latest

Status

Stable: No breaking changes will be made before 2.0.

---

Released under the MIT License.

multierr's Issues

Set minimum permissions to workflows

Setting minimum permissions to workflows is important to keep your repository safe against supply-chain attacks. GitHub grants a GITHUB_TOKEN with higher permissions than necessary by default for workflows. With higher permissions, if your workflow suffers an attack, the attacker would be able to push malicious code to your repository for example. To avoid that, we set minimum permissions.

I see both workflows, fossa.yaml and go.yml, don't need much permissions, so setting contents: read should be enough. If you agree with the changes, I can open a PR!

This is considered good-practice and recommended by GitHub and other security tools, such as Scorecards and StepSecurity.

Additional context

I'm Gabriela and I work on behalf of Google and the OpenSSF suggesting supply-chain security changes :)

AppendInvoke silently drops errors subtly

Thanks for putting together this library!

The behavior I found surprising was that if you do not use the less-common pattern where you define variables in the function return signature, that using defer multierr.AppendInvoke(&err, someInvoker) will not have the error returned by someInvoker included in the error that is returned by the function.

Example: https://go.dev/play/p/JoSVtFwapQL

Note how outer20 is missing foo from its list of errors.

I guess Go's runtime is returning (or copying the return value) before the defer is running when the return value is defined in the function body vs in the function signature.

go get uber-go / multierr error

go.uber.org/multierr

../go.uber.org/multierr/error.go:197:6: undefined: errors.As
../go.uber.org/multierr/error.go:210:6: undefined: errors.Is

YAML package update

Hello,

The current release version 1.8.0 is using a vulnerable version of the gopkg.in/yaml.v3 package.

https://www.cve.org/CVERecord?id=CVE-2022-28948

Is it possible to have a minor update to the release to fix this issue with the update to 3.0.1

LateAppendInto as convenient shorthand for io.Closer and similar use cases

In cases like

func writeToTempFile(content string) (err error) {
	var tmpFile io.WriteCloser
	if tmpFile, err = os.CreateTemp(os.TempDir(), "tmp-*"); err != nil {
		return
	}
	defer func() {
		err = multierr.Append(err, tmpFile.Close())
	}()
	_, err = tmpFile.Write([]byte(content))
	return
}

the AppendInto seems to be handy but unfortunately it is not because

defer multierr.AppendInto(&err, tmpFile.Close())

immediately closes the file.

I would like to propose a LateAppendInto:

func LateAppendInto(into *error, errFunc func() error) (errored bool) {
	return multierr.AppendInto(into, errFunc())
}

that captures the function returning an error to actually call it when the defer calls the actual function to improve the initial example to:

func writeToTempFile(content string) (err error) {
	var tmpFile io.WriteCloser
	if tmpFile, err = os.CreateTemp(os.TempDir(), "tmp-*"); err != nil {
		return
	}
	defer multierr.LateAppendInto(&err, tmpFile.Close)
	_, err = tmpFile.Write([]byte(content))
	return
}

I'd be glad to create a PR but I thought I'd ask you first what you think and if you would accept something like this.

Support other multierr implementations

We previously dropped support for integrating with error implementations which
expose the underlying list of errors via some interface like,

type MultiError interface {
    Causes() []error
}
// Type and method name up for discussion

This issue is to discuss the possibility of adding something like that back
and implementing that interface on the library's error type.

Besides integrating with other libraries, this will allow multierr to be
compatible with other versions of the same library. That is, adding an
interface like this will allow us to handle the case where one repo is using
version A from foo/vendor/go.uber.org/multierr and another repo is using
version B from bar/vendor/go.uber.org/multierr. Error types returned by
version A will be compatible with Append calls in version B because they'll
both implement the MultiError interface.

Implementation detail: We'll still want to keep the optimization where the
error type matches *multiError exactly. We'll just add a fallback case for
the interface-version which will be a bit slower.

Support multierr.Every

Hello,

There seems to be a correctness issue the implementation of multiError.Is. I say "seems" because I cannot find any consensus on what errors.Is(err, ErrIgnorable) should actually be doing when err is a multiError containing both ErrIgnorable and an important error (that you do want to handle).

Perhaps Is should only return true if all contained errors match the target, so instead

func (merr *multiError) Is(target error) bool {
        if len(merr.Errors()) == 0 {
                return false
        }
	for _, err := range merr.Errors() {
		if !errors.Is(err, target) {
			return false
		}
	}
	return true
}

an alternative would be that Is is not implemented at all, and then errors.Is should return false.

Again, I don't know if the golang developers ever specified some semantics for errors.Is, but I think that from both an intuitive and practical sense, errors.Is(err, ErrIgnorable) should return false. Intuitive, because a multiError is not equal to a single error, and practical because if the important error is not represented by a value, e.g. val ErrImportant = errors.New(...), then there is no way to easily ignore only ignorable error without calling Errors() and iterating over the individual errors..

Caller to 'demux' list of multiple errors returned

If the caller is optionally aware that a multierr might be returned, she can access the list of errors and switch case among them. As a result, she is now able to act upon them accordingly.

Wish we could tell if the append happened

I've hit a surprising number of cases recently where I wished that multierr.Append would tell me if the append happened or not. It's not so much for dubious micro perf reasons (CPUs being cheap, fast, and plentiful these days), but more on account of simplifying control flow:

• when I don't need to tell if it happened, I can use the inline form like err = multierr.Append(err, somethingFaliable())
• but if I need to know if somethingFaliable failed or not in addition to collecting its error, then I have to break that apart, and do my own nil check
• really the hardest part of this, as always, is choosing what to name someErr ;-)

package main

type T struct{}

type Scanable interface {
	Scan(...interface{}) bool
}

// harvestT loads T from some Scanable source. Since we really like T, we don't
// let mundane details like parsing errors get in our way; instead we continue
// on to harvest as much T as we can, collecting any errors that we encounter.
func harvestT(iter Scanable) ([]T, error) {
	var (
		mess   string // each string from the db
		datum  T      // each parsed item
		data   []T    // all items parsed
		retErr error  // any collected errors
	)

	// with current multierr
	for iter.Scan(&mess) {
		if parseErr := tryToParse(s, &datum); parseErr != nil {
			retErr = multierr.Append(retErr, parseErr)
			continue
		}
		data = append(data, datum)
	}

	// however what if we could:
	for iter.Scan(&mess) {
		retErr, failed = multierr.Appended(retErr, parseErr)
		if failed {
			continue
		}
		data = append(data, datum)
	}

	// or even "better" in this case:
	for iter.Scan(&mess) {
		retErr, failed = multierr.Appended(retErr, parseErr)
		if !failed {
			data = append(data, datum)
		}
	}

	// alternatively, if we used a *error:
	for iter.Scan(&mess) {
		if multierr.AppendInto(&retErr, parseErr) {
			continue
		}
		data = append(data, datum)
	}

	// which of course could be simplified in our singleton case (you wouldn't
	// want to do this if you had more than one faliable step in the loop
	// body):
	for iter.Scan(&mess) {
		if !multierr.AppendInto(&retErr, parseErr) {
			data = append(data, datum)
		}
	}

	return data, retErr
}

Invitation to comment on proposed standard library multierror support

Hi!

I have an in-review proposal to add support for wrapping multiple errors to the standard library. The latest version of the proposal is:
golang/go#53435 (comment)

This proposal is intended to allow interoperation between multierror implementations, as well as to simplify these implementations. It isn't intended to supplant existing implementations, except in the simplest of cases. The quick summary of the proposal is that errors.Is and errors.As would support descending into multiple errors when a type implements an Unwrap() []error method returning a list of wrapped errors; for details see the above link.

I'm opening an issue in this repo to invite your comments on this proposal, either in the proposal issue or here. Would you find this feature useful? If adopted, would you add an Unwrap() error to your error types? Do you see any problems with this proposal, or are there changes that would make it more useful to you?

Thanks!

README says status "beta"

Which probably shouldn't be the case given this lib has hit v1.0.0

Zap integration (again)

Currently, errors produced by this package implement fmt.Formatter with special support for the "%+v" format. We should consider removing this support, since it doesn't actually add any information (which is typically the purpose of the + flag).

err := multierr.Append(errors.New("foo"), errors.New("bar"))
fmt.Println(err.Error())
// foo; bar
fmt.Printf("%v\n", err)
// foo; bar
fmt.Printf("%+v\n", err)
// the following errors occurred:
//  - foo
//  - bar

This came up because it makes logging these errors with zap particularly verbose - see post-merge discussion on uber-go/zap#460.

Figure out integration story for logging errors, especially using zap

Right now, if we do zap.Error(err) and the error is a multierr, then we end up with a single concatenated string that contains all the errors.

Ideally we would have output more similar to zap.Errors when writing out a multiple errors.

The multierr just returns an error, which may be:

• nil if there were no errors
• single error if there was only one error
• type wrapping []error if there's multiple errors

I think the experience we want is a single function that takes the error, and returns either:

• zap.Skip() if there was no error
• zap.Error(..) if there's a single error
• zap.Errors(...) if there are multiple errors

cc @abhinav and @akshayjshah

I've documented some options for this below, I'm leaning towards the latter options. Would be great to document any other options.

Expose Causes(error) []error from multierr

The user would be required to do:

var errs error
errs = multierr.Append(errs, ...)
logger.Error("Here are the errors.", zap.Errors(multierr.Causes(errs)))

The Causes function would always allocate a slice, and return a slice with either 0, 1, or N elements depending on the input. The final output would always contain "errors" even if there's 0 errors or just 1 error.

Implement zap.MarshalLogArray in the internal multierr type

Since the underlying type is not exported, the user would probably do:

var errs error
errs = multierr.Append(errs, ...)
logger.Error("Here are the errors.", zap.Any("errors", errs))

This would require a dependency on zap from multierr -- if there is a dependency, I think the other way makes more sense.

The final output would always contain "errors" (similar to above).

Have zap implicitly recognize multierr

zap could recognize a multierr error, and automatically get all the errors using Causes() []error and use zap.Errors instead.

zap would need to depend on multierr (which seems like a more reasonable dependency) but implicit handling may be a little unexpected.

Add explicit zap.MultiError(error) field type

This would still require a dependency, but would make the checks more explicit. zap.MultiError would return fields depending on the number of errors. This would be the ideal user experience, but extends the zap interface to add another Field constructor which takes error. This may be confusing to users.

Have a subpackage of multierr that provides the above field constructor

We could add a multierr/zerr package for users logging multierr with zap. This avoids cross dependencies between the core packages, but the zerr package could depend on both multierr and zap, and provide a field constructor Error(err error) zap.Field that does the same logic as the previous option.

The user experience is a little worse since users now need to import a second package. However, the cost may be worth avoiding cross dependencies or extending zap's interface.

Go 1.20: Errors() should support any multi-error

Follow up to #69

Now that the interface for errors wrapping multiple errors has been standardized to, Unwrap() []error, the multierr.Errors(error) []error function should support any error that conforms to it, not just its own error type.

This behavior should be gated behind the go1.20 build tag.

Depending on this project ends up pulling in lots of dependencies

I vendor dependencies in my project. Due to the "tools.go" imports in this file, I'm now vendoring all of the staticcheck project. This seems like overkill for a library that is designed to handle multiple errors.

Perhaps you could move the imports in tools.go to a different place, like the private repositories at Uber?

Go 1.13 unwrapping behavior

Go 1.13 introduced errors.Is, errors.As, and errors.Unwrap, all of which inspect errors that implement an optional Unwrap(error) error method. Currently, multierr's wrapper type doesn't implement Unwrap, so using multierr.Append or multierr.Combine prevents the standard library from walking the error chain. Is there a good way to extend multierr to interoperate with the new stdlib functions?

The main problem is that the standard library's Unwrap API expects a linked list of errors, and multierr builds a tree. Some options:

• Don't bother. Is and As don't necessarily make sense for collections of errors anyways.
• Treat the first (or last) error as the "main" error and return it from Unwrap. Simple, but not fully correct.
• Something else? It's hard to build an iterator over the error tree, since that introduces yet another wrapper.