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.

Sign up for GitHub

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

Jump to bottom

Migrate custom error trait impls to thiserror #1856

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Migrate custom error trait impls to thiserror #1856

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
65fcb31
Migrate to thiserror
pkhry Nov 6, 2024
75e6371
missing bits
pkhry Nov 6, 2024
14a5062
review comment
pkhry Nov 6, 2024
28a2a78
Apply suggestions from code review
pkhry Nov 6, 2024
8ff3479
From<scale_decode::visitor::Error> to remove Into::intos
jsdw Nov 6, 2024
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
223 changes: 123 additions & 100 deletions Cargo.lock
View file
Open in desktop

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ scale-typegen-description = "0.9.0"
serde = { version = "1.0.210", default-features = false, features = ["derive"] }
serde_json = { version = "1.0.128", default-features = false }
syn = { version = "2.0.77", features = ["full", "extra-traits"] }
thiserror = "1.0.64"
thiserror = { version = "2.0.0", default-features = false }
tokio = { version = "1.40", default-features = false }
tracing = { version = "0.1.40", default-features = false }
tracing-wasm = "0.2.1"
Expand Down
3 changes: 3 additions & 0 deletions core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,9 @@ tracing = { workspace = true, default-features = false }
# AccountId20
keccak-hash = { workspace = true}

# Error handling
thiserror = { workspace = true, default-features = false }
pkhry marked this conversation as resolved.
Show resolved Hide resolved

[dev-dependencies]
assert_matches = { workspace = true }
bitvec = { workspace = true }
Expand Down
3 changes: 2 additions & 1 deletion core/src/blocks/extrinsic_signed_extensions.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ impl<'a, T: Config> ExtrinsicSignedExtension<'a, T> {
&mut &self.bytes[..],
self.ty_id,
self.metadata.types(),
)?;
)
.map_err(Into::<scale_decode::Error>::into)?;
Ok(value)
}

Expand Down
3 changes: 2 additions & 1 deletion core/src/blocks/extrinsics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,8 @@ where
.iter()
.map(|f| scale_decode::Field::new(f.ty.id, f.name.as_deref()));
let decoded =
scale_value::scale::decode_as_fields(bytes, &mut fields, self.metadata.types())?;
scale_value::scale::decode_as_fields(bytes, &mut fields, self.metadata.types())
.map_err(Into::<scale_decode::Error>::into)?;

Ok(decoded)
}
Expand Down
205 changes: 46 additions & 159 deletions core/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,203 +4,142 @@

//! The errors that can be emitted in this crate.

use core::fmt::Display;

use alloc::boxed::Box;
use alloc::string::String;
use subxt_metadata::StorageHasher;
use thiserror::Error as DeriveError;

/// The error emitted when something goes wrong.
#[derive(Debug)]
#[derive(Debug, DeriveError)]
pub enum Error {
/// Codec error.
Codec(codec::Error),
#[error("Scale codec error: {0}")]
pkhry marked this conversation as resolved.
Show resolved Hide resolved
Codec(#[from] codec::Error),
/// Metadata error.
Metadata(MetadataError),
#[error("Metadata Error: {0}")]
pkhry marked this conversation as resolved.
Show resolved Hide resolved
Metadata(#[from] MetadataError),
/// Storage address error.
StorageAddress(StorageAddressError),
#[error("Storage Error: {0}")]
pkhry marked this conversation as resolved.
Show resolved Hide resolved
StorageAddress(#[from] StorageAddressError),
/// Error decoding to a [`crate::dynamic::Value`].
Decode(scale_decode::Error),
#[error("Error decoding into dynamic value: {0}")]
Decode(#[from] scale_decode::Error),
/// Error encoding from a [`crate::dynamic::Value`].
Encode(scale_encode::Error),
#[error("Error encoding from dynamic value: {0}")]
Encode(#[from] scale_encode::Error),
/// Error constructing the appropriate extrinsic params.
ExtrinsicParams(ExtrinsicParamsError),
#[error("Extrinsic params error: {0}")]
pkhry marked this conversation as resolved.
Show resolved Hide resolved
ExtrinsicParams(#[from] ExtrinsicParamsError),
/// Block body error.
Block(BlockError),
}

impl core::fmt::Display for Error {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
Error::Codec(e) => write!(f, "Scale codec error: {e}"),
Error::Metadata(e) => write!(f, "Metadata Error: {e}"),
Error::StorageAddress(e) => write!(f, "Storage Error: {e}"),
Error::Decode(e) => write!(f, "Error decoding into dynamic value: {e}"),
Error::Encode(e) => write!(f, "Error encoding from dynamic value: {e}"),
Error::ExtrinsicParams(e) => write!(f, "Extrinsic params error: {e}"),
Error::Block(e) => write!(f, "Error working with block_body: {}", e),
}
}
#[error("Error working with block_body: {0}")]
Block(#[from] BlockError),
}

#[cfg(feature = "std")]
impl std::error::Error for Error {}

impl_from!(ExtrinsicParamsError => Error::ExtrinsicParams);
impl_from!(BlockError => Error::Block);
impl_from!(MetadataError => Error::Metadata);
impl_from!(scale_decode::Error => Error::Decode);
impl_from!(scale_decode::visitor::DecodeError => Error::Decode);
impl_from!(scale_encode::Error => Error::Encode);
impl_from!(StorageAddressError => Error::StorageAddress);
impl_from!(codec::Error => Error::Codec);

/// Block error
#[derive(Debug)]
#[derive(Debug, DeriveError)]
pub enum BlockError {
/// Leftover bytes found after decoding the extrinsic.
#[error("After decoding the extrinsic at index {extrinsic_index}, {num_leftover_bytes} bytes were left, suggesting that decoding may have failed")]
LeftoverBytes {
/// Index of the extrinsic that failed to decode.
extrinsic_index: usize,
/// Number of bytes leftover after decoding the extrinsic.
num_leftover_bytes: usize,
},
/// Something went wrong decoding the extrinsic.
#[error("Failed to decode extrinsic at index {extrinsic_index}: {error}")]
ExtrinsicDecodeError {
/// Index of the extrinsic that failed to decode.
extrinsic_index: usize,
/// The decode error.
error: ExtrinsicDecodeError,
},
}
impl Display for BlockError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
BlockError::LeftoverBytes {
extrinsic_index,
num_leftover_bytes,
} => {
write!(
f,
"After decoding the extrinsic at index {extrinsic_index}, {num_leftover_bytes} bytes were left, suggesting that decoding may have failed"
)
}
BlockError::ExtrinsicDecodeError {
extrinsic_index,
error,
} => {
write!(
f,
"Failed to decode extrinsic at index {extrinsic_index}: {error}"
)
}
}
}
}

/// An alias for [`frame_decode::extrinsics::ExtrinsicDecodeError`].
///
pub type ExtrinsicDecodeError = frame_decode::extrinsics::ExtrinsicDecodeError;

/// Something went wrong trying to access details in the metadata.
#[derive(Clone, Debug, PartialEq)]
#[derive(Clone, Debug, PartialEq, DeriveError)]
#[non_exhaustive]
pub enum MetadataError {
/// The DispatchError type isn't available in the metadata
#[error("The DispatchError type isn't available")]
DispatchErrorNotFound,
/// Type not found in metadata.
#[error("Type with ID {0} not found")]
TypeNotFound(u32),
/// Pallet not found (index).
#[error("Pallet with index {0} not found")]
PalletIndexNotFound(u8),
/// Pallet not found (name).
#[error("Pallet with name {0} not found")]
PalletNameNotFound(String),
/// Variant not found.
#[error("Variant with index {0} not found")]
VariantIndexNotFound(u8),
/// Constant not found.
#[error("Constant with name {0} not found")]
ConstantNameNotFound(String),
/// Call not found.
#[error("Call with name {0} not found")]
CallNameNotFound(String),
/// Runtime trait not found.
#[error("Runtime trait with name {0} not found")]
RuntimeTraitNotFound(String),
/// Runtime method not found.
#[error("Runtime method with name {0} not found")]
RuntimeMethodNotFound(String),
/// Call type not found in metadata.
#[error("Call type not found in pallet with index {0}")]
CallTypeNotFoundInPallet(u8),
/// Event type not found in metadata.
#[error("Event type not found in pallet with index {0}")]
EventTypeNotFoundInPallet(u8),
/// Storage details not found in metadata.
#[error("Storage details not found in pallet with name {0}")]
StorageNotFoundInPallet(String),
/// Storage entry not found.
#[error("Storage entry {0} not found")]
StorageEntryNotFound(String),
/// The generated interface used is not compatible with the node.
#[error("The generated code is not compatible with the node")]
IncompatibleCodegen,
/// Custom value not found.
#[error("Custom value with name {0} not found")]
CustomValueNameNotFound(String),
}
impl Display for MetadataError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
MetadataError::DispatchErrorNotFound => {
write!(f, "The DispatchError type isn't available")
}
MetadataError::TypeNotFound(e) => write!(f, "Type with ID {e} not found"),
MetadataError::PalletIndexNotFound(e) => write!(f, "Pallet with index {e} not found"),
MetadataError::PalletNameNotFound(e) => write!(f, "Pallet with name {e} not found"),
MetadataError::VariantIndexNotFound(e) => write!(f, "Variant with index {e} not found"),
MetadataError::ConstantNameNotFound(e) => write!(f, "Constant with name {e} not found"),
MetadataError::CallNameNotFound(e) => write!(f, "Call with name {e} not found"),
MetadataError::RuntimeTraitNotFound(e) => {
write!(f, "Runtime trait with name {e} not found")
}
MetadataError::RuntimeMethodNotFound(e) => {
write!(f, "Runtime method with name {e} not found")
}
MetadataError::CallTypeNotFoundInPallet(e) => {
write!(f, "Call type not found in pallet with index {e}")
}
MetadataError::EventTypeNotFoundInPallet(e) => {
write!(f, "Event type not found in pallet with index {e}")
}
MetadataError::StorageNotFoundInPallet(e) => {
write!(f, "Storage details not found in pallet with name {e}")
}
MetadataError::StorageEntryNotFound(e) => write!(f, "Storage entry {e} not found"),
MetadataError::IncompatibleCodegen => {
write!(f, "The generated code is not compatible with the node")
}
MetadataError::CustomValueNameNotFound(e) => {
write!(f, "Custom value with name {e} not found")
}
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for MetadataError {}

/// Something went wrong trying to encode or decode a storage address.
#[derive(Clone, Debug)]
#[derive(Clone, Debug, DeriveError)]
#[non_exhaustive]
pub enum StorageAddressError {
/// Storage lookup does not have the expected number of keys.
#[error("Storage lookup requires {expected} keys but more keys have been provided.")]
TooManyKeys {
/// The number of keys provided in the storage address.
expected: usize,
},
/// This storage entry in the metadata does not have the correct number of hashers to fields.
#[error("Storage entry in metadata does not have the correct number of hashers to fields")]
WrongNumberOfHashers {
/// The number of hashers in the metadata for this storage entry.
hashers: usize,
/// The number of fields in the metadata for this storage entry.
fields: usize,
},
/// We weren't given enough bytes to decode the storage address/key.
#[error("Not enough remaining bytes to decode the storage address/key")]
NotEnoughBytes,
/// We have leftover bytes after decoding the storage address.
#[error("We have leftover bytes after decoding the storage address")]
TooManyBytes,
/// The bytes of a storage address are not the expected address for decoding the storage keys of the address.
#[error("Storage address bytes are not the expected format. Addresses need to be at least 16 bytes (pallet ++ entry) and follow a structure given by the hashers defined in the metadata")]
UnexpectedAddressBytes,
/// An invalid hasher was used to reconstruct a value from a chunk of bytes that is part of a storage address. Hashers where the hash does not contain the original value are invalid for this purpose.
#[error("An invalid hasher was used to reconstruct a value with type ID {ty_id} from a hash formed by a {hasher:?} hasher. This is only possible for concat-style hashers or the identity hasher")]
HasherCannotReconstructKey {
/// Type id of the key's type.
ty_id: u32,
Expand All @@ -209,77 +148,28 @@ pub enum StorageAddressError {
},
}

impl Display for StorageAddressError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
StorageAddressError::TooManyKeys { expected } => write!(
f,
"Storage lookup requires {expected} keys but more keys have been provided."
),
StorageAddressError::WrongNumberOfHashers { .. } => write!(
f,
"Storage entry in metadata does not have the correct number of hashers to fields"
),
StorageAddressError::NotEnoughBytes => write!(
f,
"Not enough remaining bytes to decode the storage address/key"
),
StorageAddressError::TooManyBytes => write!(
f,
"We have leftover bytes after decoding the storage address"
),
StorageAddressError::UnexpectedAddressBytes => write!(
f,
"Storage address bytes are not the expected format. Addresses need to be at least 16 bytes (pallet ++ entry) and follow a structure given by the hashers defined in the metadata"
),
StorageAddressError::HasherCannotReconstructKey { ty_id, hasher } => write!(
f,
"An invalid hasher was used to reconstruct a value with type ID {ty_id} from a hash formed by a {hasher:?} hasher. This is only possible for concat-style hashers or the identity hasher"
),
}
}
}

#[cfg(feature = "std")]
impl std::error::Error for StorageAddressError {}

/// An error that can be emitted when trying to construct an instance of [`crate::config::ExtrinsicParams`],
/// encode data from the instance, or match on signed extensions.
#[derive(Debug)]
#[derive(Debug, DeriveError)]
#[non_exhaustive]
pub enum ExtrinsicParamsError {
/// Cannot find a type id in the metadata. The context provides some additional
/// information about the source of the error (eg the signed extension name).
#[error("Cannot find type id '{type_id} in the metadata (context: {context})")]
MissingTypeId {
/// Type ID.
type_id: u32,
/// Some arbitrary context to help narrow the source of the error.
context: &'static str,
},
/// A signed extension in use on some chain was not provided.
#[error("The chain expects a signed extension with the name {0}, but we did not provide one")]
UnknownSignedExtension(String),
/// Some custom error.
#[error("Error constructing extrinsic parameters: {0}")]
Custom(Box<dyn CustomError>),
}

impl Display for ExtrinsicParamsError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
ExtrinsicParamsError::MissingTypeId { type_id, context } => write!(
f,
"Cannot find type id '{type_id} in the metadata (context: {context})"
),
ExtrinsicParamsError::UnknownSignedExtension(e) => write!(
f,
"The chain expects a signed extension with the name {e}, but we did not provide one"
),
ExtrinsicParamsError::Custom(e) => {
write!(f, "Error constructing extrinsic parameters: {e}")
}
}
}
}

/// Anything implementing this trait can be used in [`ExtrinsicParamsError::Custom`].
#[cfg(feature = "std")]
pub trait CustomError: std::error::Error + Send + Sync + 'static {}
Expand All @@ -292,9 +182,6 @@ pub trait CustomError: core::fmt::Debug + core::fmt::Display + Send + Sync + 'st
#[cfg(not(feature = "std"))]
impl<T: core::fmt::Debug + core::fmt::Display + Send + Sync + 'static> CustomError for T {}

#[cfg(feature = "std")]
impl std::error::Error for ExtrinsicParamsError {}

impl From<core::convert::Infallible> for ExtrinsicParamsError {
fn from(value: core::convert::Infallible) -> Self {
match value {}
Expand Down
3 changes: 2 additions & 1 deletion core/src/events.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,8 @@ impl<T: Config> EventDetails<T> {
.map(|f| scale_decode::Field::new(f.ty.id, f.name.as_deref()));

let decoded =
scale_value::scale::decode_as_fields(bytes, &mut fields, self.metadata.types())?;
scale_value::scale::decode_as_fields(bytes, &mut fields, self.metadata.types())
.map_err(Into::<scale_decode::Error>::into)?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm curious what has changed that makes these new .map_errs necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

decode_as_fields returns DecodeError and although scale_decode::Error implements From<DecodeError> compiler needs an explicit cast

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't get why that would be the case though offhand, when it wasn't before? Also, out of interest, can you use Into::into or do you need the explicit type there too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea either.
Fails to infer the type without explicit annotation

Copy link
Collaborator

Choose a reason for hiding this comment

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

I had a look and there was a missing From impl for scale_decode::visitor::DecodeError (confusingly similar name!); adding that let the Into's go away :)


Ok(decoded)
}
Expand Down
Loading
Loading