-
Notifications
You must be signed in to change notification settings - Fork 5k
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
Fix RPC error messages #7173
Fix RPC error messages #7173
Conversation
dc83635
to
4c775e0
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! It works for me locally.
I suspect this closes #7160 |
I didn't even see #7160. Seems that it fixed the provider approval error, but not the signing methods they mentioned. metamask-extension/app/scripts/lib/message-manager.js Lines 84 to 87 in 4d88e1c
metamask-extension/app/scripts/lib/personal-message-manager.js Lines 90 to 93 in 7985f4f
metamask-extension/app/scripts/lib/typed-message-manager.js Lines 80 to 85 in cc71b4f
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed one typo which is arguably breaking.
I don't fully understand how eth-json-rpc-errors
improves error messages, or how this fixes a former break, so it might be nice to also document those two things, even just here in the PR before approve/merge.
942de95
to
672ade3
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for adding the extended explanation!
Fix error message issues described in: #7138 (comment)
9/18/2019 update: Ready to review.
PendingFix error serialization json-rpc-engine#19In a previous commit,
json-rpc-engine
started using eth-json-rpc-errors for error serialization. This error serialization swallowed the original error message if the error had a missing or invalid code. The above PR tojson-rpc-engine
preserves the original error message if it exists and adds a valid code (JSON RPC Internal error,-32603
) if the original code was missing/invalid.With that change and the code changes introduced in this PR, all RPC errors returned from the extension to the inpage provider will follow JSON RPC 2.0 and EIP 1193, allowing dapp devs to identify and handle errors by code. Consistently using the
eth-json-rpc-errors
API internally will allow us to do the same.