-
Notifications
You must be signed in to change notification settings - Fork 154
rpc: support wrapped errors and code-based Is for jsonError #649
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -126,12 +126,12 @@ func errorMessage(err error) *jsonrpcMessage { | |
| Code: ErrcodeDefault, | ||
| Message: err.Error(), | ||
| }} | ||
| ec, ok := err.(Error) | ||
| if ok { | ||
| var ec Error | ||
| if errors.As(err, &ec) { | ||
| msg.Error.Code = ec.ErrorCode() | ||
| } | ||
| de, ok := err.(DataError) | ||
| if ok { | ||
| var de DataError | ||
| if errors.As(err, &de) { | ||
| msg.Error.Data = de.ErrorData() | ||
| } | ||
| return msg | ||
|
|
@@ -158,6 +158,19 @@ func (err *jsonError) ErrorData() interface{} { | |
| return err.Data | ||
| } | ||
|
|
||
| // Is reports whether target carries the same JSON-RPC error code as err. | ||
| // 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 { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. |
||
| type errorCoder interface{ ErrorCode() int } | ||
| t, ok := target.(errorCoder) | ||
| if !ok { | ||
| return false | ||
| } | ||
| return err.Code == t.ErrorCode() | ||
| } | ||
|
|
||
| // Conn is a subset of the methods of net.Conn which are sufficient for ServerCodec. | ||
| type Conn interface { | ||
| io.ReadWriteCloser | ||
|
|
||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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