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

Custom Debug impl for DispatchError #1153

merged 6 commits into from
Sep 11, 2023

Conversation

ascjones
Copy link
Contributor

@ascjones ascjones commented Sep 7, 2023

Currently the Debug implementation prints the whole metadata which fills the screen and doesn't help much:

image

This PR adds a custom Debug impl which instead resolves to virtual pallet and error fields similar to the Display impl. To know what the actual error is extremely useful 😄

See also #1140 (comment)

Copy link
Collaborator

@jsdw jsdw left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

),
};

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

@jsdw jsdw merged commit 022d709 into master Sep 11, 2023
9 checks passed
@jsdw jsdw deleted the aj/module-err-debug branch September 11, 2023 10:37
@jsdw jsdw mentioned this pull request Sep 27, 2023
nanometerzhu pushed a commit to Phala-Network/substrate-subxt that referenced this pull request Jan 25, 2024
* Custom `Debug` impl for `DispatchError`

* Implement suggestion

* Display bytes if details cannot be resolved

* Extract fn and use from Display impl

* Display just shows {details}
nanometerzhu pushed a commit to Phala-Network/substrate-subxt that referenced this pull request Jan 25, 2024
* Custom `Debug` impl for `DispatchError`

* Implement suggestion

* Display bytes if details cannot be resolved

* Extract fn and use from Display impl

* Display just shows {details}
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