Skip to content

lib/errors: new MultiError error type and utilities

Administratorrequested to merge
multi-error-library into main

Created by: bobheadxi

After many tangents (also see https://github.com/sourcegraph/sourcegraph/pull/31431) I decided to rip off the bandaid - this PR is a wholesale migration away from go-multierror into a custom multierror implementation that is fully compatible with cockroachdb/errors, prints all errors, can be introspected with Is, As, and friends, and more. The new multi error type is only available as an interface.

  • resolves our core issue of multi errors being swallowed when wrapped, potentially omitting critical debugging information from logs
  • nicer way to get multi errors: errors.As into the MultiError interface, then use the read-only Error(). No more initializing and passing around the multi error struct
  • removes the need for ErrorOrNil! All multi-errors can be treated transparently as an error with no special handling - you can always rely on err != nil to be trustworthy
  • turns CombineErrors into multi errors as well, because the behaviour of it appears quite deceptive.

It also gets rid of random ways errors are joined into multi errors:

  • no more appending directly to err.Errors
  • no more magic _ = Append(multi, err) - we don't modify the provided errors anymore

This PR also removes a few error list formatters into custom error types, which is probably what they should have been anyway.

There is more discussion and background in https://github.com/sourcegraph/sourcegraph/pull/31431 . That PR is also more or less an issue description at this point, so closes https://github.com/sourcegraph/sourcegraph/pull/31431

All tests pass with only minimal changes due to removal of newlines, because we don't have go-multierror's random extra \n\n anymore

tldr: more error, less *MultiError and ErrorOrNil(), better printing and consistency.

Reviewing

Start here:

image

Everything else should be minor changes to fit the updated semantics

Test plan

Merge request reports