uber-go / multierr Goto Github PK
View Code? Open in Web Editor NEWCombine one or more Go errors together
Home Page: https://go.uber.org/multierr
License: MIT License
Combine one or more Go errors together
Home Page: https://go.uber.org/multierr
License: MIT License
../go.uber.org/multierr/error.go:197:6: undefined: errors.As
../go.uber.org/multierr/error.go:210:6: undefined: errors.Is
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
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:
Is
and As
don't necessarily make sense for collections of errors anyways.Unwrap
. Simple, but not fully correct.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!
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 errorserror
if there was only one error[]error
if there's multiple errorsI think the experience we want is a single function that takes the error
, and returns either:
zap.Skip()
if there was no errorzap.Error(..)
if there's a single errorzap.Errors(...)
if there are multiple errorscc @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.
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.
zap.MarshalLogArray
in the internal multierr
typeSince 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).
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.
zap.MultiError(error)
field typeThis 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.
multierr
that provides the above field constructorWe 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.
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.
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?
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.
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.
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.
Which probably shouldn't be the case given this lib has hit v1.0.0
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.
I'm Gabriela and I work on behalf of Google and the OpenSSF suggesting supply-chain security changes :)
I believe showing the equality of errors is broken. Below is my example program showing
$ go run main.go
Nope, they are not the same
TotalError1: 1; 2; 3
TotalError2: 1; 2; 3
main.go
package main
import (
"errors"
"fmt"
"go.uber.org/multierr"
)
var (
Err1 = errors.New("1")
Err2 = errors.New("2")
Err3 = errors.New("3")
)
func main() {
var TotalError1 error
TotalError1 = multierr.Append(TotalError1, Err1)
TotalError1 = multierr.Append(TotalError1, Err2)
TotalError1 = multierr.Append(TotalError1, Err3)
var TotalError2 error
TotalError2 = multierr.Append(TotalError2, Err1)
TotalError2 = multierr.Append(TotalError2, Err2)
TotalError2 = multierr.Append(TotalError2, Err3)
if errors.Is(TotalError1, TotalError2) {
fmt.Println("Yay! They are the same!")
} else {
fmt.Println("Nope, they are not the same")
fmt.Printf("TotalError1: %s\n", TotalError1)
fmt.Printf("TotalError2: %s\n", TotalError2)
}
}
go.mod
module example
go 1.22.4
require go.uber.org/multierr v1.11.0 // indirect
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.
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..
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:
err = multierr.Append(err, somethingFaliable())
somethingFaliable
failed or not in addition to collecting its error, then I have to break that apart, and do my own nil checksomeErr
;-)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
}
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.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
๐ Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. ๐๐๐
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google โค๏ธ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.