fix(chain,compact-filters): replace Debug delegation in Display impls with per-variant messages#937
Open
none34829 wants to merge 2 commits intogetfloresta:masterfrom
Open
Conversation
… with proper per-variant messages
BlockchainError and IterableFilterStoreError both implemented Display
by delegating to Debug (write!(f, "{self:?}") and Debug::fmt(self, f)
respectively). This produces machine-oriented output unsuitable for
user-facing error messages or for composition with thiserror's
#[error(transparent)].
Replace both with explicit match arms that produce human-readable
messages for each variant. Inner types that already implement Display
are formatted with {}, while StumpError and Box<dyn DatabaseError>
(which only implement Debug) continue to use {:?}.
Contributes to getfloresta#667.
| BlockchainError::InvalidProof => write!(f, "Invalid Utreexo proof"), | ||
| BlockchainError::UtreexoError(e) => write!(f, "Utreexo accumulator error: {e:?}"), | ||
| BlockchainError::UtreexoLeaf(e) => write!(f, "Utreexo leaf error: {e}"), | ||
| BlockchainError::Database(e) => write!(f, "Database error: {e:?}"), |
Member
There was a problem hiding this comment.
Database error doesn't implement Display? It would be nice if it did
Author
There was a problem hiding this comment.
good catch, addressed in 5208a5c. added Display as a supertrait on DatabaseError and implemented it for FlatChainstoreError (the only concrete impl), so BlockchainError::Database now formats via Display ({e}) instead of Debug.
Assisted-by: Claude Opus 4.6
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
BlockchainErrorandIterableFilterStoreErrorboth implementDisplayby delegating toDebug:This produces machine-oriented output that is unsuitable for user-facing error messages and cannot be composed with
thiserror's#[error(transparent)]in a meaningful way.Changes
Replaced both with explicit
matcharms producing human-readable messages for each variant. Inner types that already implementDisplayare formatted with{}, whileStumpErrorandBox<dyn DatabaseError>(which only implementDebug) continue to use{:?}.This is complementary to PR #879, which adds
Displayimpls to error types that have none. This PR improves types that already have aDisplayimpl but delegate it toDebug.Files changed
crates/floresta-chain/src/pruned_utreexo/error.rs—BlockchainError(15 variants)crates/floresta-compact-filters/src/lib.rs—IterableFilterStoreError(4 variants)Contributes to #667.