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

Create coded errors to cover all TransactionErrors from the RPC #2213

Conversation

steveluscher
Copy link
Collaborator

@steveluscher steveluscher commented Feb 28, 2024

Summary

Wouldn't it be nice if you could catch particular transaction errors in your application, like BlockhashNotFound, and choose the correct mitigation based on the type of transaction error?

In this PR, we introduce coded exceptions for each TransactionError returned from the RPC's sendTransaction method.

Note

Because the RPC doesn't return structured errors or error codes, we had to break our own rules in this PR and hardcode a map between the error names and the code numbers. My first crack at this employed a source code compression scheme that I later deemed too risky for the 250 gzipped bytes it saved. We might consider such a scheme in the future, especially since the next PR will add InstructionError to the mix.

Test Plan

cd packages/errors
pnpm test:unit:browser
pnpm test:unit:node

Addresses #2118.

@steveluscher steveluscher force-pushed the 02-28-Create_coded_errors_to_cover_all_TransactionErrors_from_the_RPC branch from 3585002 to 2f394a0 Compare February 28, 2024 18:09
export const SOLANA_ERROR__TRANSACTION_ERROR_INVALID_ACCOUNT_FOR_FEE = 7050006 as const;
export const SOLANA_ERROR__TRANSACTION_ERROR_ALREADY_PROCESSED = 7050007 as const;
export const SOLANA_ERROR__TRANSACTION_ERROR_BLOCKHASH_NOT_FOUND = 7050008 as const;
// `InstructionError` intentionally omitted
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll break these off into a separate special case in the next PR.

Comment on lines +89 to +101
} else if (codeOffset === 29 /* DuplicateInstruction */) {
errorContext = {
index: transactionErrorContext as number,
};
} else if (
codeOffset === 30 /* InsufficientFundsForRent */ ||
codeOffset === 34 /* ProgramExecutionTemporarilyRestricted */
) {
errorContext = {
accountIndex: (transactionErrorContext as { account_index: number }).account_index,
};
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These are all special cases, because the RPC spits them out as { ErrorName: { some: "struct" } } instead of "ErrorName".

@steveluscher steveluscher force-pushed the 02-28-Create_coded_errors_to_cover_all_TransactionErrors_from_the_RPC branch from 2f394a0 to a737c14 Compare February 28, 2024 18:14
Copy link
Collaborator

@buffalojoec buffalojoec left a comment

Choose a reason for hiding this comment

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

This lgtm. Shame about the map, but it makes sense. This utility is very useful and I believe was asked for at some point.

I think we should iron out the concept of maybe grouping error codes by discriminator, as you do here, before this goes in.

See #2205 (review)

Copy link
Collaborator

@lorisleiva lorisleiva left a comment

Choose a reason for hiding this comment

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

Love it!

@steveluscher steveluscher changed the base branch from 02-24-refactor_experimental_errors_codecs-core_package to 02-29-Prevent_a_runtime_fatal_in_the_error_message_formatter_when_the_context_contains_null_or_undefined February 29, 2024 17:47
@steveluscher steveluscher force-pushed the 02-28-Create_coded_errors_to_cover_all_TransactionErrors_from_the_RPC branch from 8fdf687 to 42eea0b Compare February 29, 2024 17:47
@steveluscher
Copy link
Collaborator Author

steveluscher commented Feb 29, 2024

Merge activity

  • Feb 29, 9:52 AM PST: @steveluscher started a stack merge that includes this pull request via Graphite.
  • Feb 29, 9:53 AM PST: Graphite rebased this pull request as part of a merge.
  • Feb 29, 9:53 AM PST: @steveluscher merged this pull request with Graphite.

Base automatically changed from 02-29-Prevent_a_runtime_fatal_in_the_error_message_formatter_when_the_context_contains_null_or_undefined to master February 29, 2024 17:52
@steveluscher steveluscher force-pushed the 02-28-Create_coded_errors_to_cover_all_TransactionErrors_from_the_RPC branch from 42eea0b to a8f8adf Compare February 29, 2024 17:52
@steveluscher steveluscher merged commit 8541c2e into master Feb 29, 2024
4 of 6 checks passed
@steveluscher steveluscher deleted the 02-28-Create_coded_errors_to_cover_all_TransactionErrors_from_the_RPC branch February 29, 2024 17:53
steveluscher added a commit that referenced this pull request Feb 29, 2024
# Summary

In this PR, we build atop the work in #2213 and introduce coded exceptions for each `InstructionError` returned from the RPC's `sendTransaction` method.

> [!NOTE]
> Because the RPC doesn't return structured errors or error codes, we had to break our own rules in this PR and hardcode a map between the error names and the code numbers. My [first crack](https://gist.github.com/steveluscher/aaa7cbbb5433b1197983908a40860c47#file-fml-ts-L12) at this employed a source code compression scheme that I later deemed too risky for the 250 gzipped bytes it saved. We might consider such a scheme in the future, especially since the next PR will add `InstructionError` to the mix.

# Test Plan

```shell
cd packages/errors
pnpm test:unit:browser
pnpm test:unit:node
```

Addresses #2118.
Copy link
Contributor

github-actions bot commented Mar 2, 2024

🎉 This PR is included in version 1.90.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

steveluscher added a commit that referenced this pull request Mar 12, 2024
# Summary

So wow. I completely missed these as part of #2118, and it turns out they're a really big deal and required a ton of changes.

There are:

* Errors that have enough data to format a message
* Errors that have no data, but also no context in the message
* Errors that have no data, but really should, because you can't format a message without it
* Preflight errors in which is nested a `TransactionError` (see #2213)

In this PR we create a helper that takes in the `RpcSimulateTransactionResult` from the RPC and reformats it as a coded `SolanaError`.

As always, everything you need to know is in the `packages/errors/src/__tests__/json-rpc-error-test.ts`.

# Test Plan

```
pnpm turbo test:unit:browser
pnpm turbo test:unit:node
```
Copy link
Contributor

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants