Skip to content

serrors isn't treating by value or by reference errors consistently #4486

@jiceathome

Description

@jiceathome

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).

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions