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

Custom Debug impl for DispatchError #1153

Merged
merged 6 commits into from
Sep 11, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 27 additions & 9 deletions subxt/src/error/dispatch_error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ pub enum TransactionalError {
}

/// Details about a module error that has occurred.
#[derive(Clone, Debug, thiserror::Error)]
#[derive(Clone, thiserror::Error)]
#[non_exhaustive]
pub struct ModuleError {
metadata: Metadata,
Expand All @@ -142,16 +142,19 @@ impl PartialEq for ModuleError {

impl Eq for ModuleError {}

impl std::fmt::Display for ModuleError {
/// Custom `Debug` implementation, ignores the very large `metadata` field, using it instead (as
/// intended) to resolve the actual pallet and error names. This is much more useful for debugging.
impl Debug for ModuleError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let Ok(details) = self.details() else {
return f
.write_str("Unknown pallet error (pallet and error details cannot be retrieved)");
};
let details = self.details_string();
write!(f, "ModuleError(<{details}>)")
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Would it be beneficial to have this syntax ModuleError<..> also for the Display impl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fine by me, wdyt @jsdw?

Copy link
Collaborator

@jsdw jsdw Sep 11, 2023

Choose a reason for hiding this comment

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

I think the display impl can be "prettier" (it doesn't need to fit in with the rust like syntax that everything else spits out for debug impls), but perhaps we should indeed standardise these; maybe Debug should print ModuleError(<{details}>) as it does here and Display should just print {details}, so we can have a shared func to write details and then Debug just wraps to make fit better with rusty syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

}
}

let pallet = details.pallet.name();
let error = &details.variant.name;
write!(f, "Pallet error {pallet}::{error}")
impl std::fmt::Display for ModuleError {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let details = self.details_string();
write!(f, "{details}")
}
}

Expand All @@ -166,6 +169,21 @@ impl ModuleError {
Ok(ModuleErrorDetails { pallet, variant })
}

/// Return a formatted string of the resolved error details for debugging/display purposes.
pub fn details_string(&self) -> String {
match self.details() {
Ok(details) => format!(
"{pallet_name}::{variant_name}",
pallet_name = details.pallet.name(),
variant_name = details.variant.name,
),
Err(_) => format!(
"Unknown pallet error '{bytes:?}' (pallet and error details cannot be retrieved)",
bytes = self.bytes
),
}
}

/// Return the underlying module error data that was decoded.
pub fn bytes(&self) -> [u8; 5] {
self.bytes
Expand Down