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

refactor(experimental): errors: assertions package #2188

Merged

Conversation

buffalojoec
Copy link
Collaborator

Adds custom SolanaError throws to the @solana/assertions package.

@mergify mergify bot added the community label Feb 24, 2024
@buffalojoec buffalojoec changed the title refactor(experimental): errors: \'assertions\' package refactor(experimental): errors: assertions package Feb 24, 2024
@buffalojoec buffalojoec force-pushed the 02-24-refactor_experimental_errors_assertions_package branch from e7fb883 to 325f801 Compare February 24, 2024 19:54
@buffalojoec buffalojoec force-pushed the 02-24-refactor_experimental_errors_addresses_package branch from 1b48e25 to 91b4a68 Compare February 24, 2024 19:56
@buffalojoec buffalojoec force-pushed the 02-24-refactor_experimental_errors_assertions_package branch from 325f801 to ec6802f Compare February 24, 2024 19:56
packages/errors/src/messages.ts Outdated Show resolved Hide resolved
Copy link
Collaborator

@steveluscher steveluscher left a comment

Choose a reason for hiding this comment

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

There's something about the ‘missing’ terminology that bothers me, but I can't figure it out. Maybe it's right! Maybe ‘unimplemented?’

Added via Giphy

'Cryptographic operations are only allowed in secure browser contexts. Read more ' +
'here: https://developer.mozilla.org/en-US/docs/Web/Security/Secure_Contexts',
);
throw new SolanaError(SOLANA_ERROR__SUBTLE_CRYPTO_MISSING);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean, as a corollary yes, but the actual error is SOLANA_ERROR__CRYPTO_DISALLOWED_IN_INSECURE_CONTEXT or something.

Copy link
Collaborator

Choose a reason for hiding this comment

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

…like, SubtleCrypto can be missing for other reasons, like the environment is otherwise ‘secure’ but just doesn't support SubtleCrypto.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I went for SOLANA_ERROR__SUBTLE_CRYPTO_DISALLOWED_IN_INSECURE_CONTEXT in the feedback PR.

I also changes the _MISSING suffixes with _UNIMPLEMENTED as you suggested and added a description for it in the comments.

@buffalojoec buffalojoec force-pushed the 02-24-refactor_experimental_errors_addresses_package branch from 2bf93b9 to b10a9e8 Compare February 28, 2024 16:30
@buffalojoec buffalojoec force-pushed the 02-24-refactor_experimental_errors_assertions_package branch from 51680e6 to 37bdedc Compare February 28, 2024 16:30
@steveluscher
Copy link
Collaborator

steveluscher commented Feb 29, 2024

Merge activity

  • Feb 29, 11:33 AM PST: @steveluscher started a stack merge that includes this pull request via Graphite.
  • Feb 29, 11:39 AM PST: Graphite rebased this pull request as part of a merge.
  • Feb 29, 11:40 AM PST: @steveluscher merged this pull request with Graphite.

@steveluscher steveluscher force-pushed the 02-24-refactor_experimental_errors_addresses_package branch from bb22886 to 16ce665 Compare February 29, 2024 19:36
Base automatically changed from 02-24-refactor_experimental_errors_addresses_package to master February 29, 2024 19:38
@steveluscher steveluscher force-pushed the 02-24-refactor_experimental_errors_assertions_package branch from 2ae6db6 to 54664fb Compare February 29, 2024 19:38
@steveluscher steveluscher merged commit 2e0ae95 into master Feb 29, 2024
6 checks passed
@steveluscher steveluscher deleted the 02-24-refactor_experimental_errors_assertions_package branch February 29, 2024 19:40
steveluscher pushed a commit that referenced this pull request Feb 29, 2024
…tions (#2200)

As mentioned in
[this PR comment](#2188 (comment)),
if you need to provide some kind of "reason" or "message" to an error, it's
usually a smell.

Now that we have custom error codes and can perform proper error handling, we
can replace this mechanism for piping error messages (effectively "reasons") to
fixed codec assertions!

This change refactors out `message` from `assertIsFixed`. In a later commit,
I'll replace the thrown error with a coded `SolanaError` and add the necessary
`catch` statements downstream.
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 📦🚀

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[experimental] Convert all thrown errors to SolanaError
3 participants