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

Throw an error, not an object in json-rpc-engine #1993

Open
MicahZoltu opened this issue Feb 16, 2021 · 0 comments
Open

Throw an error, not an object in json-rpc-engine #1993

MicahZoltu opened this issue Feb 16, 2021 · 0 comments

Comments

@MicahZoltu
Copy link

https://github.com/MetaMask/json-rpc-engine/blob/bece020a9d515e1714ffa3e6ac36c534e33ddfe5/src/JsonRpcEngine.ts#L344

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

@MajorLift MajorLift transferred this issue from MetaMask/json-rpc-engine Nov 7, 2023
@MajorLift MajorLift changed the title Throw an error, not an object. Throw an error, not an object in json-rpc-engine Nov 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants