RPC error middleware for json-rpc-engine

[bitpshr]

This PR implements new json-rpc-engine-compatible middleware intended to log consistent, human-readable RPC error messages.

Even though the JSON-RPC 2.0 specification documents message as a required property on the RPC error object, it's undefined for most standard RPC error codes when hitting the latest Infura provider. This middleware logs predefined messages based on RPC error codes by default; it can also be configured to use provider-supplied error messages if they exist and to only fall back on predefined messages. The original RPC error object is also always logged regardless of middleware configuration so any additional error data can still be inspected, such as solidity compilation information.

Resolves #3487
Resolves #3054
Resolves #1477
Resolves #794

[metamaskbot]

Builds ready [73faf3a]: mascara, chrome, firefox, edge, opera

[danfinlay]

When I try the reproduction step from #3847 I still get an anonymous error:

web3.currentProvider.sendAsync({ method: 'debug_traceTransaction', params: ['0x33ab8f2c2f73aaef9e0d1b620d8aeabd37dd8f975bb3743d11278ec13214ba38'], from: web3.eth.accounts[0] }, console.log)

That just means it doesn't fix that issue yet.

I couldn't figure out how to reproduce #3054, I'm fine closing it.

#1477 reproduction steps didn't throw an error for me, so I couldn't verify the fix.

#974 still gave returned an unreadable error message to the dapp:

Error: Invalid JSON RPC response: {"id":1,"jsonrpc":"2.0","error":{"code":-32603}}
    at Object.InvalidResponse (util.js:481)

Is it possible I built wrong or something? Do you get a message when calling that top method in browser console?

[bitpshr]

Re: #3487:

We were properly logging the RPC error object to the console, but that error object wasn't populated with a required message property that explains the error code. The middleware now properly logs the correct error message despite no message existing on the error object (I just reverified the call:

MetaMask - RPC Error: Internal JSON-RPC error. {code: -32603}

[danfinlay]

To close #3487 at least, we should pass the error message all the way back to the Dapp that made the call.

[danfinlay]

Two options:

• We remove the resolves for that issue, since message isn't reaching dapp
• We get that issue also resolved in the PR.

Let me know which you prefer, I'm happy to merge this if we keep that issue open.

[bitpshr]

@danfinlay thanks for the review. You were right, I was logging the errors instead of augmenting the actual object that's passed up the chain to a dapp.

This should be ready for another review when you have a chance. I verified that the returned error object now contains a valid message for #3487 and #794.

[anthony33lewis]

Thank You

[danfinlay]

Looks good, thank you, this is going to make a lot of devs happy. Maybe add a changelog entry to let them know?

[bitpshr]

Thanks @danfinlay, CHANGELOG.md entry added.

[danfinlay]

Oh wow, test-deps failing for node security. How is this not an issue on master? Is this a new addition, @kumavis?

[metamaskbot]

Builds ready [2c0e80c]: mascara, chrome, firefox, edge, opera

[metamaskbot]

Builds ready [ce28344]: mascara, chrome, firefox, edge, opera

[bitpshr]

@danfinlay It looks like develop was fixed up, so CI passes now after a rebase. Ready for another review when you have a chance, thanks again!

[kumavis]

this seems like it should go into json-rpc-engine as core always-on functionality

[bitpshr]

@kumavis Would you prefer I close this and submit a PR against json-rpc-engine instead? Technically the message property we're patching is required per the JSON-RPC 2.0 specification, so this middleware is paving over an Infura shortcoming and is specific to our reliance on this specific version of the Infura API that lacks proper error messages.

[danfinlay]

This looks like a good solution to me, I don't think this should be in core json-rpc-engine, since it's compensating for an infura quirk. Maybe we could call it an infura-error-middleware.