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

WIP on #798: nicer error logs #830

Closed
wants to merge 3 commits into from

Conversation

cqfd
Copy link
Contributor

@cqfd cqfd commented Oct 4, 2021

#798

This is pretty rough but I think I need to hurry up and start a PR, oof. I think just about all tests are passing except for lockup, which doesn't even build, but I can't seem to find why its registry #[program] is unhappy.

There are two ideas:

First idea: Box up behind a dyn trait the various different error enums that can show up in an anchor program (low-level Solana ProgramErrors, Anchor's framework ErrorCode enum, Anchor's wrapper Error enum, your own custom errors, etc.). In principle it ought to be possible to avoid this dynamic dispatch approach but I'm not sure how to actually do it :/

Second idea/huge hack: Anchor's built-in traits (Accounts, AccountsExit, etc.) all return ProgramResults. This means that internal Anchor ErrorCodes get smooshed into ProgramError::Custom(u32)s, throwing away their messages. I think probably the right fix for this is to change these return types, either to Result<(), crate::error::Error>, or I don't know, maybe by adding an associated type Error: Into<ProgramError>. I tried the first option. It ends up being a bit of a slog that I never quite managed to finish, and it's a breaking change that requires changing a lot of tests (there are some test ix handlers that rely on auto-into-ing ProgramErrors in functions returning Result<()>, but that breaks if Anchor traits start using crate::error::Error).

So, I thought it was worth starting a PR with something way hackier, based on the original idea I tried: macro generate a mapping from Custom(u32) --> Anchor's ErrorCode enum (we don't need to do this for your own custom #[error] enums because you can just use a good return type for your ix handlers, and then the dyn trait stuff handles the rest). This feels so gross but it does seem to work?

}
}

pub type DynAnchorResult = std::result::Result<(), Box<dyn AnchorError>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not super into these names, happy to change them

@@ -16,7 +16,7 @@ pub fn generate(program: &Program) -> proc_macro2::TokenStream {
// on chain.
#[inline(never)]
#[cfg(not(feature = "no-idl"))]
pub fn __idl_dispatch(program_id: &Pubkey, accounts: &[AccountInfo], idl_ix_data: &[u8]) -> ProgramResult {
pub fn __idl_dispatch(program_id: &Pubkey, accounts: &[AccountInfo], idl_ix_data: &[u8]) -> ::anchor_lang::DynAnchorResult {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These return type changes are where I'm not sure how to do the enum/no-dynamic dispatch approach—what should these return types be? It would depend on whether or not you've generated your own #[error] enum, right? I definitely agree that it's possible to have a nice static return type here (only a static number of error enums in your program), but I wasn't sure how to actually do it.

Copy link

Choose a reason for hiding this comment

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

This is really nice to use to get rid of dynamic dispatch
https://docs.rs/enum_dispatch/0.3.7/enum_dispatch/

e
let program_error = e.to_program_error();
anchor_lang::solana_program::msg!(&program_error.to_string());
if let ::anchor_lang::solana_program::program_error::ProgramError::Custom(code) = program_error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This printing is late-night hackyness, it seems that the anchor ts code isn't super lenient with error parsing, so I'm just printing both the old Custom error code: 0x thing as well as the new one.

@armaniferrante
Copy link
Member

One problem with this approach is that it doesn't allow us to add a filename + line number to the error message. So we'll probably need to change this when we want to add that feature. Tag @paul-schaaf.

@armaniferrante
Copy link
Member

Closing in favor of #1462.

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.

3 participants