Skip to content
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

simulators/rpc-compat: allow for any message in error checks #975

Closed

Conversation

Rjected
Copy link
Contributor

@Rjected Rjected commented Jan 30, 2024

When reth runs the rpc-compat tests which expect an error, sometimes the error test vector contains a string like this:

+    "message": "invalid argument 0: json: cannot unmarshal hex string without 0x prefix into Go value of type common.Hash

This is something that reth will never be able to output.

Sometimes we also see the following situation:

>>  {"jsonrpc":"2.0","id":1,"method":"debug_getRawReceipts","params":["2"]}
<<  {"jsonrpc":"2.0","error":{"code":-32602,"message":"Invalid params","data":"hex string without 0x prefix at line 1 column 3"},"id":1}
response differs from expected (-- client, ++ test):
 {
   "error": {
     "code": -32602,
-    "data": "hex string without 0x prefix at line 1 column 3",
-    "message": "Invalid params"
+    "message": "invalid argument 0: hex string without 0x prefix"
   },
   "id": 1,
   "jsonrpc": "2.0"
 }

While this is a different way of displaying the error, it includes the correct error code and "invalid params" plus the data field give the right information.

This PR tries to make sure that the deep comparison is still performed for the error object, with the exception of the message and data fields are not compared.

cc @fjl, I pulled this from our old reth PR

@fjl
Copy link
Collaborator

fjl commented Jan 30, 2024

Thanks for the PR. I am working on improvements to the simulator right now, and was also planning to address this error thing.

@fjl
Copy link
Collaborator

fjl commented Feb 1, 2024

Applied a slightly different fix for this in #984

@fjl fjl closed this Feb 1, 2024
@fjl
Copy link
Collaborator

fjl commented Feb 1, 2024

BTW @Rjected it is not recommended to supply an additional error message in the error.data field. This field is supposed to be used for additional user-parseable data related to the error, and as such we should ensure clients behave the same w.r.t. to this field.

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