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

2/n improve sui-json-rpc error codes and handling #11833

Merged
merged 9 commits into from
May 12, 2023
Merged

Conversation

wlmyng
Copy link
Contributor

@wlmyng wlmyng commented May 9, 2023

Description

Update authority.rs to use SuiResult instead of anyhow::Error. Some places that depended on anyhow::Error needed to be updated. Most noticeably, previously, because of anyhow::Error, there's no need to convert the underlying SuiError into Error (SuiJsonRpcError) where Rust can then do the conversion into Error (RpcError), as RpcError supports from anyhow::Error. Now though, we need to explicitly convert into the intermediary, causing instances where we have .map_err(Error::from)?.map_err(Error::from)?. One thought is to have an _internal() fn for all methods that rely on calls to state and other services.

Test Plan

Existing unit + integration tests. Manually call each api endpoint and verify that success and error responses are as expected


If your changes are not user-facing and not a breaking change, you can skip the following section. Otherwise, please indicate what changed, and then add to the Release Notes section as highlighted during the release process.

Type of Change (Check all that apply)

  • user-visible impact
  • breaking change for a client SDKs
  • breaking change for FNs (FN binary must upgrade)
  • breaking change for validators or node operators (must upgrade binaries)
  • breaking change for on-chain data layout
  • necessitate either a data wipe or data migration

Release notes

The following endpoints have a change in error response codes:

  1. query_transaction_blocks
  2. query_events
  3. get_checkpoints
  4. get_total_transaction_blocks
  5. dev_inspect_transaction_block

Previously, these endpoints returned an error code of -32000 for any error response. With this update, if the underlying error maps to one of the following error types:

  • UserInputError
  • SuiError::TransactionNotFound
  • SuiError::TransactionsNotFound

The error code will now be -32602 instead.

Action Required:

Clients and consumers of the API relying on the previous error code -32000 for handling specific errors should update their implementations to handle the new error code -32602. This will ensure the correct handling of errors related to UserInputError, TransactionNotFound, and TransactionsNotFound.

Also note that the error string for error variants not mentioned may be slightly different.

@vercel
Copy link

vercel bot commented May 9, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

4 Ignored Deployments
Name Status Preview Comments Updated (UTC)
explorer ⬜️ Ignored (Inspect) May 12, 2023 2:26am
explorer-storybook ⬜️ Ignored (Inspect) May 12, 2023 2:26am
sui-wallet-kit ⬜️ Ignored (Inspect) May 12, 2023 2:26am
wallet-adapter ⬜️ Ignored (Inspect) May 12, 2023 2:26am

@wlmyng wlmyng force-pushed the better-error-handling-2 branch 5 times, most recently from bfcd908 to 7b4dd74 Compare May 9, 2023 23:03
@wlmyng wlmyng force-pushed the better-error-handling-2 branch from 7b4dd74 to ce1f397 Compare May 11, 2023 19:53
@petvaizAkhtar petvaizAkhtar mentioned this pull request May 11, 2023
@MystenLabs MystenLabs deleted a comment from petvaizAkhtar May 11, 2023
@wlmyng wlmyng force-pushed the better-error-handling-2 branch from a08d409 to 8d6bb51 Compare May 12, 2023 01:57
@wlmyng wlmyng merged commit b7625b6 into main May 12, 2023
@wlmyng wlmyng deleted the better-error-handling-2 branch May 12, 2023 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants