search

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

fix subtle interface bug on Group.Wait() #34

Closed mightyguava closed 4 years ago

mightyguava commented 4 years ago

Returning the concrete type *Error causes any assignments to an error type to result in a non-nil value, since the error interface would actually have (*Error, nil) rather than (nil, nil). This matches the sync/errgroup interface.

cc: @nickethier

hashicorp-cla commented 4 years ago

CLA assistant check
All committers have signed the CLA.

mentalisttraceur commented 4 years ago

I think the expected way to use Group is to just call .ErrorOrNil() on the *Error result of .Wait() before assigning it into an error type.

So this is actually a backwards-incompatible change for code that already handles this situation by doing .Wait().ErrorOrNil(), which is probably a lot of code using this feature.

I agree that this isn't the most ergonomic interface - in my code the overwhelmingly common case is to do those two calls in a chain like that (I've never had occasion to want the *Error directly).

So I sympathize with the desire to streamline this into one call, but I'm not sure they can do this without breaking backwards-compatibility.

mightyguava commented 4 years ago

Ah, I see how this pattern wouldn't fit in this library. Thanks for explaining.