Appending nil values results in a non-nil error

[matthewmueller]

Loving this library. One thing surprised me though:

var err error
var err2 error
err = multierror.Append(err, err2)
// err.Error() "0 errors occurred:"

Practically speaking, instead of doing this:

var err error
for _, chrome := range m.chromes {
    err = multierror.Append(err, chrome.Stop())
}
return err

You have to do this:

var err error
for _, chrome := range m.chromes {
    e := chrome.Stop()
    if e != nil {
        err = multierror.Append(err, e)
    }
}
return err

Not terrible, just a bit unexpected. Would you accept a PR fixing this? Thanks!

[mitchellh]

Yes I'd prefer that behavior. Would accept a PR. Thanks!

[matthewmueller]

Hmm, doesn't seem possible actually, but I'm not sure why. Maybe some more experienced Go folks can chime in.

Even if I change the Append function to:

func Append(err, errs ...error) *Error {
  return nil
}

And run it with this:

var err error
var err1 error
err = Append(err, err1)
if err != nil {
  fmt.Println("oh noz")
  // this will get called, even if I return nil.
  // the reflected type is *multierror.Error
}

However this works:

var err error
var err1 error
result := Append(err, err1)
// result will be nil

[mitchellh]

Ah yes, that's right. Because a nil *Error is still a non-nil error implementation. That is a common confusion in Go. This might explain why it behaves the way it does.

Looking into it further, you can actually work around it by calling err.ErrorOrNil and typing your result. For example, do this instead:

var err error
var err1 error
result := Append(err, err1).ErrorOrNil()

You can chain a number of Appends and only call that once.

I'm pretty sure that's why it behaves this way. Sorry for the weirdness! Will close.

[matthewmueller]

ahh okay, makes sense – thanks!

[ryanmcnamara]

Sorry for reviving a dead issue, what would you think of a new Append func (changing the existing seems very not ok) that had this behavior by returning error instead of multierror.Error? Not sure what you would call it, maybe AppendError? Returning nil from there would have the behavior mentioned above and I don't think any drawbacks other than a bigger API.

It would make it possible to use this library without needing to remember to use ErrorOrNil which would be great (it's easy to forget), you could do this: