bufbuild / buf

The best way of working with Protocol Buffers.
https://buf.build
Apache License 2.0
9.18k stars 278 forks source link

Drop go.uber.org/multierr in favor of errors.Join #3441

Closed stefanvanburen closed 3 weeks ago

stefanvanburen commented 3 weeks ago

Fairly mechanical update to use errors.Join in all of the situations in which we were previously using multierr.

github-actions[bot] commented 3 weeks ago

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedNov 1, 2024, 5:48 PM
dylan-bourque commented 3 weeks ago

One gotcha I've run into with errors.Join() is that calling .Error() on the returned error will always inject newlines between the joined errors. It's usually not an issue, but there are things out there that would very much prefer single-line errors.

Consider calling out the potential behavior change.

bufdev commented 3 weeks ago

I know I requested this, but I'm actually starting to wonder if this is strictly worse and whether we should just not do this. I want to standardize on stdlib as much as anyone, but multierr isn't hurting anyone and (to my estimation) is done significantly better than errors.Join (no random wrapping of single errors, no usage of newlines). Thoughts @mfridman @stefanvanburen @doriable

mfridman commented 3 weeks ago

I've made peace with errors.Join and for the times when you want precise control over the formatted error, you have options:

err1 := sql.ErrNoRows
err2 := errors.New("call 2 failed")
err3 := errors.New("call 3 failed")
var err4 error

all := errors.Join(err1, err2, err3, err4)
if joined, ok := all.(interface{ Unwrap() []error }); ok {
    fmt.Println(formatErrors(joined.Unwrap()))
    // sql: no rows in result set: call 2 failed: call 3 failed
}

It's not very convenient, but most of the time, it's probably good enough. I wish in the original proposal they kept the option for the caller to supply the separator, or at the very least allow for customization via errors.SetSeparator.

stefanvanburen commented 3 weeks ago

no strong feeling, mostly in favor for stdlib & consistency with our other repos.

dylan-bourque commented 3 weeks ago

I've also just accepted that you have to use Unwrap() []error for precise formatting. It's unfortunate, but we have to use what we have. 🤷‍♂️

fwiw I'm in favor of standardizing on stdlib packages anywhere possible.

bufdev commented 3 weeks ago

Alright, let's go for it then. I'll play along :-)