rpc: get error code/data from wrapped error

[pmikolajczyk41]

errorMessage previously cast err directly to rpc.Error/DataError, which missed errors wrapped with fmt.Errorf("%w", ...). Switch to errors.As so the code and data are extracted correctly from wrapped errors. This turns out to be very useful in client-server communication based on the geth's rpc package (see e.g. OffchainLabs/nitro#4629).

[fjl]

It's a little strange to extract the code and data separately. There could be a situation where one error in the nesting chain supplies the code, and another error supplies the data.

For this reason, I think it is more correct to require that these methods be implemented by the top-level error. It is usually no problem to ensure this.

[pmikolajczyk41]

@fjl I agree that extracting them is a little strange (in general, the situation where two different errors in chain provide this data independently is just strange)

but what about mixing the two approaches and using errors.As to find the first error in the chain that carries the code, and then trying the old approach of casting it to RPC error to get the data?

[fjl]

Maybe that could be OK. I'm just curious what this provides to you.

[fjl]

From reading the issue, you want to do

err := client.Call(&r, "method", ...)
if errors.Is(err, api.ErrSomething) {
    ...
}

and it's supposed to match on the code. This can be done by implementing the Is(error) bool method on the error. My guess is, you want to somehow wrap your ErrSomething on the server side and use the same definition in client side?

[pmikolajczyk41]

yes, exactly; originally my changes to geth (OffchainLabs/go-ethereum#649) contained also the Is implementation, but for some reason I thought I could proceed without it

[fjl]

No the Is on jsonError should not be necessary. You should implement Is on the sentinel error.

type codedError struct{
      code int
      message string
}

func (ce codedError) Error() string { return ce.message }
func (ce codedError) ErrorCode() int { return ce.code }

func (ce codedError) Is(err error) bool {
    // As per documentation in package "errors",
    // An Is method should only shallowly compare err and the target and not call Unwrap.
    rpcerr, ok := err.(rpc.Error)
    return ok && rpcerr.ErrorCode() == ce.code
}

var ErrSomething = codedError{100, "something"}

[pmikolajczyk41]

but doesn't implementing Is for jsonError cover the general case? I mean, does is spare us implementing Is for every sentinel separately?

[fjl]

I think it's not appropriate because jsonError is the more general thing. We don't want to define a notion of equality on that. It's better on the sentinel because that's a specific thing where you know what it should be equivalent to.

[pmikolajczyk41]

fair enough; thank you therefore for this discussion, I will go with the suggested approach