hashicorp / go-multierror

A Go (golang) package for representing a list of errors as a single error.
Mozilla Public License 2.0
2.3k stars 123 forks source link

Use `error` instead of `*Error` in `Group` #58

Open ccampo133 opened 2 years ago

ccampo133 commented 2 years ago

Changes the type of Group.err to error instead of *Error, and also changes the return value of Group.Wait to error instead of *Error. The *Error concrete type can lead to unintuitive, subtle bugs around nil checks (see: https://golang.org/doc/faq#nil_error). Returning the error interface instead eliminates this possibility. Note that because this changes a public API signature, it should be considered a breaking change.

Fixes #57

hashicorp-cla commented 2 years ago

CLA assistant check
All committers have signed the CLA.

dolmen commented 5 months ago

That would be a breaking change, so unlikely to be accepted.

The only solution at this point would be a linter detecting a call to Group.Wait being assigned to an error variable.

ccampo133 commented 3 months ago

That would be a breaking change, so unlikely to be accepted.

The only solution at this point would be a linter detecting a call to Group.Wait being assigned to an error variable.

In the current version of the API, you are correct. There could certainly be a v2 release though.

Unfortunately this is a poorly designed API for the reasons I point out in https://github.com/hashicorp/go-multierror/issues/57. In fact, I stopped using it years ago, shortly after I ran into this issue. I don't believe there much need for this library now since https://pkg.go.dev/golang.org/x/sync/errgroup exists along with Go 1.20's error joining.