-
Notifications
You must be signed in to change notification settings - Fork 173
Description
serrors.New returns a pointer. all the wrapper constructors return values.
Is() expects a value. It will fail to recognize a pointer as a basicError. However, if given a value, it will panic for any error that happens to be a wrapper around basicErrors, as it will try to compare these objects which are not comparable.
WithCtx() expects only a value and will fail to downcast a basicError ptr, resulting in always wrapping instead of merging ctx.
A number of test cases have been crafted to expect that un-planned behaviour. For example private/mgmtapi/cppki/api/api_test.go expect a wrapped error while serror() was designed to produce the same error in merged form. (i.e. fixing serrors) will break those tests.
Either we fix serrors and the tests or, may be simpler, we remove that accidentally unused merging mechanism from serrors.
In either case we should decide if we want serrrors to handle values, or references, or both and implement whatever we chose consistently. I suspect we really want references since that's what we always use (otherwise Is() would be panicking).