-
Notifications
You must be signed in to change notification settings - Fork 57
Handle code-less JSON RPC errors #130
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
Conversation
🦋 Changeset detectedLatest commit: 6ee04a2 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
BundleMonFiles updated (4)
Unchanged files (123)
Total files change +944B +0.27% Final result: ✅ View report in BundleMon website ➡️ |
271c7a7
to
eba7948
Compare
appreciate u! |
@@ -40,6 +41,13 @@ describe('getSolanaErrorFromJsonRpcError', () => { | |||
const error = getSolanaErrorFromJsonRpcError({ code, message: 'o no' }); | |||
expect(error).toHaveProperty('context.__code', 123); | |||
}); | |||
it('produces a `SOLANA_ERROR__UNRECOGNIZED_JSON_RPC_ERROR` when no code is given', () => { |
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 think the actual conditional that invokes getSolanaErrorFromJsonRpcError looks like "error" in data
and if it is it calls this.
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 think that according to the specs if the JSON RPC returns a response whose payload does not include an "error"
attribute, it should not be treated as an error.
Therefore, the only thing we need to handle is a fallback for an "error"
object we do not recognise — i.e. no error code provided.
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.
Just to be completely clear for future readers, this is a patch to gracefully handle JSON-RPC that do not comply with the spec. https://www.jsonrpc.org/specification#error_object
eba7948
to
6ee04a2
Compare
Your organization requires reapproval when changes are made, so Graphite has dismissed approvals. See the output of git range-diff
at https://github.com/anza-xyz/solana-web3.js/actions/runs/13544848140
Because there has been no activity on this PR for 14 days since it was merged, it has been automatically locked. Please open a new issue if it requires a follow up. |
Problem
I've had reports that some RPC errors do not include a
code
attribute in their response. For instance:Because our types strongly expect a
bigint | number
code to be provided in the response, thegetSolanaErrorFromJsonRpcError
function of the@solana/errors
package doesn't gracefully handle this use-case and throws the following unhelpful error stack.Summary of Changes
This PR fixes this by introducing a new
SOLANA_ERROR__UNRECOGNIZED_JSON_RPC_ERROR
error which gracefully tackles the case where acode
isn't part of the RPC response.