feat: improve errors in the CLI #682
Conversation
SantiagoPittella
force-pushed
the
santiagopittella-cli-error-type
branch
3 times, most recently
from
be14a21 to
5c8c5b7
Compare
There was a problem hiding this comment.
Leaving some comments, the only big one being related to not losing source error information on places where we do err.to_string(). This might have some more information.
Other than that, I think that this is a great improvement over the current CLI errors and so we could merge this after having revised that aspect.
SantiagoPittella
force-pushed
the
santiagopittella-cli-error-type
branch
3 times, most recently
from
a1e39d8 to
8bfbd65
Compare
bin/miden-cli/src/errors.rs
Outdated
|
|
||
| #[derive(Debug, Diagnostic, Error)] | ||
| pub enum CliError { | ||
| #[error("account error: {1} with error {0}")] |
There was a problem hiding this comment.
I think here (and in the other similar variants), showing the source error (ie, displaying {1}) is redundant because error reporters will automatically report the source errors as part of the output. We could test this by inducing an error (randomly picked one):
Error: cli::account_error
× account error: error executing command with error storage slot at index 5 is not of type
│ map
╰─▶ storage slot at index 5 is not of type map
So I think we could just display the String as the top-level reason why the command failed, and then the source error.
SantiagoPittella
force-pushed
the
santiagopittella-cli-error-type
branch
from
528edbc to
f271502
Compare
closes #369
Current state of the errors: