Someone should formulate a plan to deprecate use of lib/errors.Append and use lib/errors.Combine instead. The issue here is that they’re not semantically equivalent: the former uses a bag of errors and the later has a primary and secondary. I think we need to tag each of the former with a banner error and use the bag as the secondary error. This might take some amount of manual vetting, there’s a number of references that exist.
If anyone expects cockroach's CombineErrors to allow them to introspect on the underlying error via Is, Unwrap, As and friends, well you can't! https://github.com/cockroachdb/errors/issues/62 The underlying type, SecondaryError, completely obscures the second error, except in certain printing situations.
This means that my (and maybe others') assumption that CombineErrors is like Append but for 2 errors instead of vararg errors is not true, the two are fundamentally different
You can use Wrap, but it doesn't allow you to wrap error A within error B, so deceptively CombineErrors seems to be the solution when it actually probably isn't
A test reveals that CombineErrors doesn't show the second error in .Error(), %s, and possibly more so the issue raised about go-multierror actually applies to all our methods of error combination
I attempted to rewrite our own multi error type, but it turns out Unwrap doesn't really work the way I think it does with Cockroach errors. For example:
reference:=New("foo")unwrapped:=Unwrap(err)assert.NotNil(t,err)// okay, so there's an underlying error to a raw "New" error...?assert.True(t,Is(unwrapped,reference))// fails! unwrapped error no longer asserts against a sentinel reference errorassert.True(t,Is(reference,unwrapped))// well this works but semantically makes no sense because the second argument is supposed to be the reference
New comes with a stack layer that gets unwrapped on Unwrap, which makes using New errors as sentinel errors dangerous - if you unwrap an error too far the sentinel error just doesn't work anymore! So we can't depend on the behaviour of Unwrap to build a lightweight multierror layer on top of cockroach errors, and now I'm also unsure if I can depend on Unwrap to do anything
