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 exceptions for invariant violations #2242

Merged

Conversation

steveluscher
Copy link
Collaborator

@steveluscher steveluscher commented Mar 4, 2024

Summary

Invariant violations are a kind of error that should never get hit ever, and if they do your program is broken.

In one sense we don't want error codes for them because they're not intended to be caught in downstream programs. On the other hand I've tried to come up with a reason not to give them codes and I can't.

Making them a @solana/error gives us compression and error decoding for free.

Addresses #2118.

@steveluscher
Copy link
Collaborator Author

Add link to issue tracker

@steveluscher steveluscher changed the base branch from master to 03-04-Let_the_new_coded_exceptions_from_codecs-core_bubble_up_as-is_through_solana/options_ March 4, 2024 20:34
@steveluscher steveluscher force-pushed the 03-04-Create_coded_exceptions_for_invariant_violations branch from 80f64a6 to 097c92d Compare March 4, 2024 20:34
@steveluscher steveluscher force-pushed the 03-04-Let_the_new_coded_exceptions_from_codecs-core_bubble_up_as-is_through_solana/options_ branch from ea29eaa to 9a1834c Compare March 4, 2024 20:41
Base automatically changed from 03-04-Let_the_new_coded_exceptions_from_codecs-core_bubble_up_as-is_through_solana/options_ to master March 4, 2024 20:43
Comment on lines +14 to +16
throw new SolanaError(SOLANA_ERROR__INVARIANT_VIOLATION_SWITCH_MUST_BE_EXHAUSTIVE, {
unexpectedValue: commitment satisfies never,
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one smells a little bit. I guess the only way this gets thrown is if someone isn't using TypeScript (or ignores it) and sends a string that isn't a valid Commitment. Shouldn't the error say that, then? ie. SOLANA_ERROR__INVALID_COMMITMENT_VALUE?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I was trying to make something that was generally usable for exhaustive switches. The error location will tell the developer where the unhandled value was thrown, which I figured was enough paired with the actual unexpected value.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

I do actually like that we have utility errors for these sort of invariants.

@steveluscher steveluscher force-pushed the 03-04-Create_coded_exceptions_for_invariant_violations branch from 097c92d to 1041fb6 Compare March 4, 2024 22:57
@steveluscher
Copy link
Collaborator Author

steveluscher commented Mar 4, 2024

Merge activity

@steveluscher steveluscher merged commit 9084fdd into master Mar 4, 2024
10 checks passed
@steveluscher steveluscher deleted the 03-04-Create_coded_exceptions_for_invariant_violations branch March 4, 2024 23:30
Copy link
Contributor

github-actions bot commented Mar 7, 2024

🎉 This PR is included in version 1.91.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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 22, 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