[Merged by Bors] - feat: print more error causes #3475
Conversation
crates/fluvio-cluster/src/error.rs
Outdated
| FailedPrecheck(CheckStatuses), | ||
| /// Encountered an error while performing one or more pre-checks | ||
| #[error("Failed to perform one or more pre-checks")] | ||
| #[error("Failed to perform one or more pre-checks: {0:#?}")] |
There was a problem hiding this comment.
These kind of formats {#?} could span many lines causing upper logs to be missed by the Terminal buffer.
For these cases I think we could have a one liner, perhaps implementing a Display trait on them and have a summary of the details in a single line.
There was a problem hiding this comment.
makes sense! let me try to fix this
There was a problem hiding this comment.
Removed debug prints for now. In next iteration we can implement custom Display, I didn't want to do it now because it is typedef as Vec<_> so can't implement trait on it directly, need to make it a struct which will be a breaking change.
| @@ -4,14 +4,14 @@ use thiserror::Error; | |||
| /// Possible errors from Auth | |||
| #[derive(Error, Debug)] | |||
| pub enum AuthError { | |||
| #[error("IoError")] | |||
| #[error("IoError: {0}")] | |||
There was a problem hiding this comment.
let's get rid of this and move to anyhow
There was a problem hiding this comment.
I want to do anyhow, but then it requires changes in other places in the code. Want to make this change like this and do other PRs for anyhow in future.
This should still print relevant info for now
| @@ -16,33 +16,33 @@ use crate::common::target::TargetError; | |||
| pub enum CliError { | |||
There was a problem hiding this comment.
Let's remove this and move to anyhow
|
merging this, and working on |
|
bors r+ |
Print more error causes. This is short term minimal change solution that just adds printing causes to `thiserror` derivations. I made changes for a couple of crates, will do other ones in next PRs. I realise this might be printing too much but I think it is still better than not printing enough in some cases. Later on we can refine it more using `anyhow`.
|
Pull request successfully merged into master. Build succeeded! The publicly hosted instance of bors-ng is deprecated and will go away soon. If you want to self-host your own instance, instructions are here. If you want to switch to GitHub's built-in merge queue, visit their help page. |
bors
bot
changed the title
Print more error causes. This is short term minimal change solution that just adds printing causes to
thiserrorderivations.I made changes for a couple of crates, will do other ones in next PRs.
I realise this might be printing too much but I think it is still better than not printing enough in some cases. Later on we can refine it more using
anyhow.