-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add multichain error codes #2063
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
Pull Request Overview
This PR expands the documentation by adding a table of Multichain API error codes and improves code formatting in markdown pages through SCSS styling updates.
- Added a new "Error codes" section with a table in the multichain API reference markdown.
- Updated the SCSS theme to style inline code, ensuring a consistent look in markdown content.
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
wallet/reference/multichain-api.md | Added a table listing Multichain API error codes along with descriptions and related methods. |
src/scss/theme/_markdown.scss | Introduced styles for code blocks to use a monospace font with a slightly smaller font size. |
| Code | Description | Related methods | | ||
| ---- | ----------- | --------------- | | ||
| 5000 | Unknown error with request | `wallet_revokeSession`, `wallet_createSession` | |
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.
@vandan @adonesky1 we're firing -32603
on wallet_revokeSession, same for wallet_createSession
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.
Hmm. I don't see error codes in that range anywhere in the CAIP standards for this so it seems like we should instead map any errors fired to codes in the 5000 range (ex. 5500, 5501, or 5502 for wallet_revokeSession
).
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.
We probably should throw CAIP-25's unknown error code (5000) instead of -32602 here
| 5101 | Requested methods are not supported | `wallet_createSession` | | ||
| 5102 | Requested notifications are not supported | `wallet_createSession` | | ||
| 5300 | Invalid `scopedProperties` requested | `wallet_createSession`| |
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.
we aren't technically firing this yet. thoughts, @adonesky1 ?
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.
Lol do we fire any of these errors? 5100, 5101, 5102, 5300? I'm pretty sure we've decided all of these are way too restrictive
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.
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 also don't see 5300 or 5301 thrown anywhere in our code
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.
We should remove these from the OpenRPC doc:
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.
The only one of these errors I can see us actually throwing all the way through to the dApp is 5302
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.
Sorry actually we also throw 5100
so its just 5100 and 5302
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.
Created a ticket to do cleanup of this in the YAML file and wallet_createSession
handler here: https://github.com/MetaMask/MetaMask-planning/issues/5251
| 5102 | Requested notifications are not supported | `wallet_createSession` | | ||
| 5300 | Invalid `scopedProperties` requested | `wallet_createSession`| | ||
| 5301 | `scopedProperties` can only be outside of `sessionScopes` | `wallet_createSession`| |
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 believe we treat this as invalid scope object and simply ignore the scopeobject. thoughts, @adonesky1 ?
@jiexi can we supply a fresh list based on the error codes we support? |
Description
Add table with Multichain API error codes.
Preview
https://metamask-docs-q54mtd23a-consensys-ddffed67.vercel.app/wallet/reference/multichain-api/#error-codes
Checklist
Complete this checklist before merging your PR: