Skip to content

rpc: support wrapped errors and code-based Is for jsonError#649

Open
pmikolajczyk41 wants to merge 1 commit into
masterfrom
pmikolajczyk/nit-4738-json-rpc-errors
Open

rpc: support wrapped errors and code-based Is for jsonError#649
pmikolajczyk41 wants to merge 1 commit into
masterfrom
pmikolajczyk/nit-4738-json-rpc-errors

Conversation

@pmikolajczyk41
Copy link
Copy Markdown
Member

  1. 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.
  2. Add (*jsonError).Is so that errors.Is(receivedErr, sentinel) works on the client side when receivedErr is a jsonError reconstructed from the wire. Matching is by error code: if the sentinel implements ErrorCode() int, the codes are compared. This avoids the need for string-based error reconstruction in callers.

pulled in by OffchainLabs/nitro#4629

Comment thread rpc/json.go
ec, ok := err.(Error)
if ok {
var ec Error
if errors.As(err, &ec) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is annoying that the upstream geth doesn't use error.As already here.
But changing our fork like this can hurt us down the line, when updating the fork with upstream geth.
Also, not sure if this will negatively affect native geth APIs.
So avoiding changes like this in geth, if the alternative is not too complicated, is advisable.

An alternative is, the server side, e.g. execution/rpcserver.Server. DigestMessage, checks if the error to be returned has a wrapped RPCError through error.As. If so then it returns a plain RPCError, with the same error code of the wrapped RPCError, and error message equal to the original error.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

proposed this change to upstream: ethereum/go-ethereum#34886

Comment thread rpc/json.go
// This allows errors.Is(receivedErr, sentinel) to work on the client side
// when receivedErr is a jsonError reconstructed from the wire and sentinel
// is any error that exposes an ErrorCode() int method.
func (err *jsonError) Is(target error) bool {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one can be avoided if the client side, e.g. execution/rpcclient.Client.DigestMessage, always tries to build a RPCError, from the error returned by the server, using the same error code and message provided by the error returned by the server.
RPCError already has a Is func.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants