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

Have error formatting comply with the JSON-RPC error format #252

Closed
kclowes opened this issue Jan 27, 2023 · 3 comments · Fixed by ethereum/web3.py#3061
Closed

Have error formatting comply with the JSON-RPC error format #252

kclowes opened this issue Jan 27, 2023 · 3 comments · Fixed by ethereum/web3.py#3061

Comments

@kclowes
Copy link
Contributor

kclowes commented Jan 27, 2023

What was wrong?

eth-tester (or possibly py-evm) returns errors as strings instead of dicts, which makes downstream libraries have to account for a case in testing that isn't quite what they'd see if they were connected to a node.

How can it be fixed?

A JSON-RPC error format can be found here. Convert errors coming from eth-tester to be compliant.

@reedsa
Copy link
Contributor

reedsa commented Jul 14, 2023

I'm not sure of how the evm behaves in the real world. Is there any need to format errors from py-evm (or the default backend)? Would the evm have its own spec for errors it throws?

I'm considering diving into this one.

@reedsa
Copy link
Contributor

reedsa commented Jul 24, 2023

Looking at eth_getBalance as an example. The response contains the result encoded data in some formatted json.

In eth-tester, t.get_balance simply returns the actual balance.

Is the intent to mimic the response/error that the Ethereum JSON-RPC spec uses? Should the test methods like t.get_balance return a valid JSON response object?

@kclowes
Copy link
Contributor Author

kclowes commented Jul 24, 2023

Looking at this a little bit deeper, I think this actually may be a change that is better in web3 somewhere in here: https://github.com/ethereum/web3.py/blob/main/web3/providers/eth_tester/main.py#L154-L176. It may as straightforward as catching the KeyError on line 158 in web3 and changing the response format from {'error': f"Unknown RPC Endpoint"} to something like:

{
        "jsonrpc": "2.0",
        "error": {
            "code": -32016, # or something, there is a specific code used for this IIRC
            "message": "Unknown RPC Endpoint",
            "data": "...",
        },
        "id": 2987,
    }

and then have something similar in line 160 where we're catching the NotImplementedError. The change that may be beneficial to make in eth-tester is returning the tx data if it's straightforward to do so. It may be on the computation that comes back, but I'm not sure. I believe this is the method that gets called when eth-tester is used from web3: https://github.com/ethereum/eth-tester/blob/master/eth_tester/backends/pyevm/main.py#L779C11-L779C11. Let me know if that brings up more questions!

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 a pull request may close this issue.

2 participants