[Bug]: Jsonrpc errors get burried inside error.message string

[BelfordZ]

Describe the bug

Problem

Errors returned from ethereum JSON-RPC api providers, when not expected by ethjs-query, are in a hard to use format.

Using localhost, or using addEthereumNetwork means we have no control over the responses, and thus should hold minimal expectation over their results / errors. While this issue is most prevelant among users who are connecting to ganache or hardhat, you can also get this error by setting too low of gas while using an infura provider.

Examples

1. smart contract call that executes and reverts with a reason: invalid opcode. Any developer will see this when they are connected to ganache and trying their dapp:

REQUEST:
{
  "jsonrpc": "2.0",
  "method": "eth_sendTransaction",
  "params": [
    {
      "from": "0xaa49f066a49677c0165ea0788ad0425426e2d4b5",
      "to": "0x56f50fa1a91a48d8a55ded4c3cb3d179ca99d05a",
      "value": "0x0"
    }
  ],
  "id": 0
}
RESPONSE:
{
  "id": 0,
  "error": {
    "code": -32603,
    "message": "[ethjs-query] while formatting outputs from RPC '{\"value\":{\"code\":-32603,\"data\":{\"message\":\"VM Exception while processing transaction: invalid opcode\",\"code\":-32000,\"data\":{\"0x877c456d0b406f0706f405b0d2eb4e0ebc69629a539503e7ad94a7f9b6ba36af\":{\"error\":\"invalid opcode\",\"program_counter\":13,\"return\":\"0x\"},\"stack\":\"c: VM Exception while processing transaction: invalid opcode\\n    at Function.c.fromResults (/home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:4:192416)\\n    at w.processBlock (/home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:50915)\\n    at processTicksAndRejections (node:internal/process/task_queues:96:5)\",\"name\":\"c\"}}}}'"
  }
}

---

2. Manually setting a bad nonce as an example of non-contract call error

REQUEST:
<same as above, but set the nonce to something really high in MM>

RESPONSE:
{
  "jsonrpc": "2.0",
  "id": 0,
  "error": {
    "code": -32603,
    "message": "[ethjs-query] while formatting outputs from RPC '{\"value\":{\"code\":-32603,\"data\":{\"message\":\"the tx doesn't have the correct nonce. account has nonce of: 0 tx has nonce of: 67676\",\"code\":-32000,\"data\":{\"stack\":\"n: the tx doesn't have the correct nonce. account has nonce of: 0 tx has nonce of: 67676\\n    at i (/home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:55:219953)\\n    at /home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:55:220084\\n    at /home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:48915\\n    at /home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:224017\\n    at /home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:227782\\n    at Object.return (/home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:227355)\\n    at /home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:225688\\n    at e (/home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:227723)\\n    at /home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:227704\\n    at /home/zb/code/masa/smart-contracts/node_modules/ganache-cli/build/ganache-core.node.cli.js:42:224856\\n    at processTicksAndRejections (node:internal/process/task_queues:78:11)\",\"name\":\"n\"}}}}'"
  }
}

---

The useful parts of the error messages are inside of a string, and doubly nested.

Possible Solutions

1. Fix the issue in the most common case by directly handling the error

• Pros:
  • already done
  • smallest possible scope of change
• Cons:
  • leaves other places possibly also giving these bad errors
  • does little to address the actual problem, more of a bandaid.

1.b Same as 1, except leave error.message as is (to not break things), and parse the string out same way as in the PR, but put the result insdie of error.data.

• Pros
  • non-breaking
  • gives developers something slightly better than string parsing error.message
• Cons
  • still a garbage error that we are passing to the end user, they have to dig into it to find anything useful still.

---

2. Update ethjs-query to no longer wrap errors inside result.value, and handle unrecognized errors better by not stringifying the error.

• Pros:
  • simple and fixes the issue in all places at once
• Cons:
  • the project has not made a release in 4 years, and from comments on issues, is frozen.

---

4. 'Strangle' ethjs-query by making a wrapper for it, handling the errors as we want, and replacing all the calls to new EthjsQuery in extension.

• Pros:
  • Dont need to unfreeze ethjs
  • Doesn't affect any other places that are using ethjs-query
  • keeps the difference between errors returned by different node impls contained
  • simple to implement
  • gives us a single place to change when/if we decide to replace ethjsquery
• Cons:
  • Doesn't get rid of or improve the underlying lib being no longer supported
  • another layer of abstraction between api call -> result

---

5. Replace ethjs-query with ethers (which is what ethjs-query recommends using)

• Pros:
  • externally maintained and widely used
• Cons:
  • includes a lot more than we need
  • doenst give us the same thing - would require a lot of work to get it working
  • scope of change fairly significant

---

6. Replace ethjs-query with @open-rpc/client-js as a generic json-rpc client

• Pros:
  • externally maintained and widely used
  • batching support
  • Custom transport sopport allows us to faily easily integrate with networkController's this.provder (json-rpc-engine)
  • doesnt modify the request / result / error, just passes it along
• Cons:
  • doesnt provide the same interface (request({method: "eth_sendRawTransaction"}) vs. client.eth_sendRawTransaction)

---

7. Replace ethjs-query with an openrpc generated client.

• Pros:
  • Generated from an openrpc document which describes the 'base ethereum json rpc'.
  • uses client-js, so all the same features (batching and what not)
  • json schema validation on parameters makes us able to give standardized errors about bad params before hitting rpc endpoint
  • fully typed (also generates all the types involved in eth json rpc)
  • matches interface being used in ethjs-query
  • clearly communicates to client / test client developers what json-rpc api metamask is expecting of them.
• Cons:
  • Using generated code isnt everyones cup of tea
  • scope of change fairly significant

Proposed way forward

1. implement option 2 first.
2. use the strangled interface to impl client-js or generated client as a replacement

Steps to reproduce

(this is just one way to make it happen, but it happens on infura endpoints when making contract calls with a gas setting thats too low)

1. start ganache or hardhat
2. switch metamask to localhost
3. go to test dapp, deploy failing contract, get the deployed contract address from devtools console
4. go to https://metamask.github.io/api-playground/api-documentation/
5. click try it now button on request permissions
6. click play
7. open metamask, copy your metamask addr
8. click try it now on the sendTransaction method
9. use the testdapp failing contract address as the to address
10. use your address for "from"
11. use value "0x0"
12. click play
13. send transaction
14. observe error that is received

[bschorchit]

@worldlyjohn was thinking about something similar so we also get more digested error messages on Mixpanel and can more easily work on addressing the underlying issues.

[kumavis]

im open to 1, 1b, 2, 5

[BelfordZ]

2 seems ideal. Will move forward with that

[Gudahtt]

The problem statement here seems to be that these errors are too difficult to read, but that might be the wrong problem. Most of these errors should never reach the dapp API in the first place. I'm less concerned that our internal errors are readable than I am in reducing the number of internal errors we have. We should not have internal errors. Contract errors, network errors, input validation, etc. should be explicitly handled by us and exposed as the appropriate JSON-RPC/Ethereum provider error, with a nice readable error message. These are situations we can anticipate and handle.