Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

RPC: Improve error messages for decoding errors #28530

Merged
merged 1 commit into from
Oct 24, 2022
Merged

RPC: Improve error messages for decoding errors #28530

merged 1 commit into from
Oct 24, 2022

Conversation

jstarry
Copy link
Contributor

@jstarry jstarry commented Oct 21, 2022

Problem

Error messages for decoding failures in RPC responses aren't very clear. For example, an invalid base64 encoded message passed to getFeeForMessage could return "InvalidLength" as an error message and it's not clear that this is related to invalid base64 encoding or some other invalid length in deserialization or message sanitization.

Summary of Changes

  • Use more verbose error messages for errors from dependency crates like bs58 and base64
  • Expand test coverage for error cases

Fixes #

@jstarry jstarry requested a review from KirillLykov October 21, 2022 02:34
@jstarry jstarry force-pushed the rpc/improve-decoding-errs branch from 8ba0758 to b07c5c9 Compare October 21, 2022 05:38
KirillLykov
KirillLykov previously approved these changes Oct 21, 2022
View reviewed changes
Copy link
Contributor

@KirillLykov KirillLykov left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Yet some tests are failing, need to fix (see below). There is also some preexisting code repetition in match case, but since there are only two options (base64 and base58) it is better not to extract a function.

KirillLykov
KirillLykov reviewed Oct 21, 2022
View reviewed changes
@jstarry jstarry added the automerge Merge this Pull Request automatically once CI passes label Oct 21, 2022
@mergify mergify bot dismissed KirillLykov’s stale review October 21, 2022 09:03

Pull request has been modified.

@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Oct 21, 2022
@mergify
Copy link
Contributor

mergify bot commented Oct 21, 2022

automerge label removed due to a CI failure

@jstarry jstarry force-pushed the rpc/improve-decoding-errs branch from 597cb18 to deca5c1 Compare October 24, 2022 02:12
@jstarry jstarry force-pushed the rpc/improve-decoding-errs branch from deca5c1 to 8c59a3a Compare October 24, 2022 02:13
@jstarry jstarry added the automerge Merge this Pull Request automatically once CI passes label Oct 24, 2022
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Oct 24, 2022
@mergify
Copy link
Contributor

mergify bot commented Oct 24, 2022

automerge label removed due to a CI failure

@jstarry jstarry merged commit 322280c into solana-labs:master Oct 24, 2022
@jstarry jstarry deleted the rpc/improve-decoding-errs branch October 24, 2022 11:19
gnapoli23 pushed a commit to gnapoli23/solana that referenced this pull request Dec 16, 2022
@jstarry @gnapoli23
RPC: Improve error messages for decoding errors (solana-labs#28530)
3a3a99a
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@KirillLykov KirillLykov KirillLykov left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jstarry @KirillLykov