-
-
Notifications
You must be signed in to change notification settings - Fork 267
Description
This code will (at least sometimes) throw a { code: ..., message: ... } object rather than an instance of Error. This means there is no stack trace and no contextual information which makes debugging quite difficult.
We can see in these two locations that the type of this "error" is unknown, which should never be thrown directly:
https://github.com/MetaMask/json-rpc-engine/blob/bece020a9d515e1714ffa3e6ac36c534e33ddfe5/src/JsonRpcEngine.ts#L361
https://github.com/MetaMask/json-rpc-engine/blob/bece020a9d515e1714ffa3e6ac36c534e33ddfe5/src/JsonRpcEngine.ts#L396
A potentially bigger issue, and maybe the source of the problem, is that this code isn't properly rejecting on failure, it is instead returning a successfully resolved async function with an error result rather than rejecting, so I don't think we won't benefit from proper stack traces even if we had an error:
https://github.com/MetaMask/json-rpc-engine/blob/bece020a9d515e1714ffa3e6ac36c534e33ddfe5/src/JsonRpcEngine.ts#L404
I think this is the source of the issue, note that it is just calling end with JsonRpcResponse.error. It isn't including any additional context like the original request (which would be immensely valuable for troubleshooting) or a stack trace which (if async/await is used properly) would provide a potentially useful call stack.
https://github.com/MetaMask/json-rpc-engine/blob/bece020a9d515e1714ffa3e6ac36c534e33ddfe5/src/JsonRpcEngine.ts#L401
This is along the lines of what should be returned:
https://github.com/MetaMask/json-rpc-engine/blob/bece020a9d515e1714ffa3e6ac36c534e33ddfe5/src/JsonRpcEngine.ts#L415-L423