Skip to content

Put error text in message and omit data for automatic errors - #311

Open
MysticRyuujin wants to merge 1 commit into
status-im:masterfrom
MysticRyuujin:error-text-in-message
Open

MysticRyuujin wants to merge 1 commit into
status-im:masterfrom
MysticRyuujin:error-text-in-message

Conversation

@MysticRyuujin

Copy link
Copy Markdown

The router builds three errors by itself: invalid params (ParamsMismatchError), uncaught exceptions (CatchableError), and remote server errors. For these, message is "<method> raised an exception" and the useful text is in data:

{"code":-32602,"message":"`eth_getBalance` raised an exception","data":"Parameter [data] of type 'Address' could not be decoded: hex string too short"}

data is meant to be machine-readable, and the Ethereum execution API spec now allows it only where a method declares it (ethereum/execution-apis#967). Other execution clients put this text in message.

This PR puts the error text in message and sends no data on these three paths:

{"code":-32602,"message":"Parameter [data] of type 'Address' could not be decoded: hex string too short"}
  • If the error text is empty, the message stays "<method> raised an exception".
  • ApplicationError and local RpcResponseError keep the code, message and data that the caller sets.
  • The tests expect the new shape.

This changes the wire format for callers that read data on these errors. When nimbus-eth1 updates this dependency, tests/test_rpc.nim (debug_getRawBlockAccessList) must expect exc.msg == "Resource not found" and an empty exc.data.

Invalid params, uncaught exceptions and remote server errors now return the error text in `message` and no `data`. Before, `message` was "`<method>` raised an exception" and the text was in `data`. This changes the wire format for callers that read `data`.

This branch has not been deployed

No deployments
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.

1 participant