refactor!: migrate from thiserror to snafu (#181)

# Rationale for this change

In order to make this rep `no_std` compatible, we need `no_std` compatible errors. `thiserror` is not `no_std` compatible, while `snafu` is. We are currently using `thiserror`.

Note: While dtolnay/thiserror#304 is an open PR, it is not clear how soon this will get merged. Additionally, Substrate uses `snafu`, which would help unify dependencies for Substrate-based integrations.

# What changes are included in this PR?
NOTE: the individual commits may be good to review individually.
- All error enum variants consisting of tuple structs are transformed
into named structs. This is necessary because `snafu` does not support
tuple structs.
- Every `#[derive(Error)]` is substituted with the analogous
`#[derive(Snafu)]`. In particular:
* `#[error(...)]` attributes are substituted with equivalent
`#[snafu(display(...))]` attributes
* `#[error(transparent)]` attributes are substituted with equivalent
`#[snafu(transparent)]` attributes (which also derive the corresponding
`From` implementation)
* For `ConversionError::TimestampConversionError`, the
`#[snafu(context(false), display(...))]` attribute is used for deriving
a `From` implementation, and at the same time maintain the custom error
message
- A `std` feature is introduced for the `proof-of-sql` crate, which in
turns activates the `snafu/std` feature. The `std` feature is required
for the `posql_db` example, because `Clap` relies on the
`std::error::Error` trait.
- `thiserror` still appears in the dependency tree (`cargo tree -i
thiserror`), but only as a transitive dependency (via `blitzar`, and
dev-dependencies)

# Are these changes tested?
Yes. This PR is a refactoring, and all existing tests pass.

Cargo.toml

@@ -55,7 +55,7 @@ rand_core = { version = "0.6", default-features = false }
 rayon = { version = "1.5" }
 serde = { version = "1", default-features = false }
 serde_json = { version = "1" }
-thiserror = { version = "1", default-features = false }
+snafu = { version = "0.8.4", default-features = false, features = ["std"] }
 tiny-keccak = { version = "2.0.2", features = [ "keccak" ] }
 # tokio = { version = "1.39.3" }
 tracing = { version = "0.1.36" }

crates/proof-of-sql-parser/Cargo.toml

@@ -20,7 +20,7 @@ bigdecimal = { workspace = true }
 chrono = { workspace = true, features = ["serde"] }
 lalrpop-util = { workspace = true, features = ["lexer", "unicode"] }
 serde = { workspace = true, features = ["serde_derive", "alloc"] }
-thiserror = { workspace = true }
+snafu = { workspace = true }
 
 [build-dependencies]
 lalrpop = { workspace = true }

crates/proof-of-sql-parser/src/error.rs

@@ -1,18 +1,27 @@
 use alloc::string::String;
-use thiserror::Error;
+use snafu::Snafu;
 
 /// Errors encountered during the parsing process
-#[derive(Debug, Error, Eq, PartialEq)]
+#[derive(Debug, Snafu, Eq, PartialEq)]
 pub enum ParseError {
-    #[error("Unable to parse query")]
+    #[snafu(display("Unable to parse query"))]
     /// Cannot parse the query
-    QueryParseError(String),
-    #[error("Unable to parse identifier")]
+    QueryParseError {
+        /// The underlying error
+        error: String,
+    },
+    #[snafu(display("Unable to parse identifier"))]
     /// Cannot parse the identifier
-    IdentifierParseError(String),
-    #[error("Unable to parse resource_id")]
+    IdentifierParseError {
+        /// The underlying error
+        error: String,
+    },
+    #[snafu(display("Unable to parse resource_id"))]
     /// Can not parse the resource_id
-    ResourceIdParseError(String),
+    ResourceIdParseError {
+        /// The underlying error
+        error: String,
+    },
 }
 
 /// General parsing error that may occur, for example if the provided schema/object_name strings

crates/proof-of-sql-parser/src/identifier.rs

@@ -45,8 +45,8 @@ impl FromStr for Identifier {
     fn from_str(string: &str) -> ParseResult<Self> {
         let name = IdentifierParser::new()
             .parse(string)
-            .map_err(|e| ParseError::IdentifierParseError(
-                format!("failed to parse identifier, (you may have used a reserved keyword as an ID, i.e. 'timestamp') {:?}", e)))?;
+            .map_err(|e| ParseError::IdentifierParseError{ error:
+                format!("failed to parse identifier, (you may have used a reserved keyword as an ID, i.e. 'timestamp') {:?}", e)})?;
 
         Ok(Identifier::new(name))
     }

crates/proof-of-sql-parser/src/intermediate_ast_tests.rs

@@ -1323,18 +1323,18 @@ fn we_cannot_parse_literals_outside_of_i128_range_in_the_result_expr() {
         .is_ok());
     assert_eq!(
         "select 170141183460469231731687303715884105728 from tab".parse::<SelectStatement>(),
-        Err(super::error::ParseError::QueryParseError(
-            "i128 out of range".to_string()
-        ))
+        Err(super::error::ParseError::QueryParseError {
+            error: "i128 out of range".to_string()
+        })
     );
     assert!("select -170141183460469231731687303715884105728 from tab"
         .parse::<SelectStatement>()
         .is_ok());
     assert_eq!(
         "select -170141183460469231731687303715884105729 from tab".parse::<SelectStatement>(),
-        Err(super::error::ParseError::QueryParseError(
-            "i128 out of range".to_string()
-        ))
+        Err(super::error::ParseError::QueryParseError {
+            error: "i128 out of range".to_string()
+        })
     );
 }
 

crates/proof-of-sql-parser/src/intermediate_decimal.rs

@@ -10,27 +10,30 @@ use alloc::string::String;
 use bigdecimal::{num_bigint::BigInt, BigDecimal, ParseBigDecimalError, ToPrimitive};
 use core::{fmt, hash::Hash, str::FromStr};
 use serde::{Deserialize, Serialize};
-use thiserror::Error;
+use snafu::Snafu;
 
 /// Errors related to the processing of decimal values in proof-of-sql
-#[derive(Error, Debug, PartialEq)]
+#[derive(Snafu, Debug, PartialEq)]
 pub enum IntermediateDecimalError {
     /// Represents an error encountered during the parsing of a decimal string.
-    #[error("{0}")]
-    ParseError(ParseBigDecimalError),
+    #[snafu(display("{error}"))]
+    ParseError {
+        /// The underlying error
+        error: ParseBigDecimalError,
+    },
     /// Error occurs when this decimal cannot fit in a primitive.
-    #[error("Value out of range for target type")]
+    #[snafu(display("Value out of range for target type"))]
     OutOfRange,
     /// Error occurs when this decimal cannot be losslessly cast into a primitive.
-    #[error("Fractional part of decimal is non-zero")]
+    #[snafu(display("Fractional part of decimal is non-zero"))]
     LossyCast,
     /// Cannot cast this decimal to a big integer
-    #[error("Conversion to integer failed")]
+    #[snafu(display("Conversion to integer failed"))]
     ConversionFailure,
 }
 impl From<ParseBigDecimalError> for IntermediateDecimalError {
     fn from(value: ParseBigDecimalError) -> Self {
-        IntermediateDecimalError::ParseError(value)
+        IntermediateDecimalError::ParseError { error: value }
     }
 }
 
@@ -88,7 +91,7 @@ impl FromStr for IntermediateDecimal {
             .map(|value| IntermediateDecimal {
                 value: value.normalized(),
             })
-            .map_err(ParseError)
+            .map_err(|err| ParseError { error: err })
     }
 }
 

crates/proof-of-sql-parser/src/posql_time/error.rs

@@ -1,41 +1,56 @@
 use alloc::string::{String, ToString};
 use serde::{Deserialize, Serialize};
-use thiserror::Error;
+use snafu::Snafu;
 
 /// Errors related to time operations, including timezone and timestamp conversions.s
-#[derive(Error, Debug, Eq, PartialEq, Serialize, Deserialize)]
+#[derive(Snafu, Debug, Eq, PartialEq, Serialize, Deserialize)]
 pub enum PoSQLTimestampError {
     /// Error when the timezone string provided cannot be parsed into a valid timezone.
-    #[error("invalid timezone string: {0}")]
-    InvalidTimezone(String),
+    #[snafu(display("invalid timezone string: {timezone}"))]
+    InvalidTimezone {
+        /// The invalid timezone
+        timezone: String,
+    },
 
     /// Error indicating an invalid timezone offset was provided.
-    #[error("invalid timezone offset")]
+    #[snafu(display("invalid timezone offset"))]
     InvalidTimezoneOffset,
 
     /// Indicates a failure to convert between different representations of time units.
-    #[error("Invalid time unit")]
-    InvalidTimeUnit(String),
+    #[snafu(display("Invalid time unit"))]
+    InvalidTimeUnit {
+        /// The underlying error
+        error: String,
+    },
 
     /// The local time does not exist because there is a gap in the local time.
     /// This variant may also be returned if there was an error while resolving the local time,
     /// caused by for example missing time zone data files, an error in an OS API, or overflow.
-    #[error("Local time does not exist because there is a gap in the local time")]
+    #[snafu(display("Local time does not exist because there is a gap in the local time"))]
     LocalTimeDoesNotExist,
 
     /// The local time is ambiguous because there is a fold in the local time.
     /// This variant contains the two possible results, in the order (earliest, latest).
-    #[error("Unix timestamp is ambiguous because there is a fold in the local time.")]
-    Ambiguous(String),
+    #[snafu(display("Unix timestamp is ambiguous because there is a fold in the local time."))]
+    Ambiguous {
+        /// The underlying error
+        error: String,
+    },
 
     /// Represents a catch-all for parsing errors not specifically covered by other variants.
-    #[error("Timestamp parsing error: {0}")]
-    ParsingError(String),
+    #[snafu(display("Timestamp parsing error: {error}"))]
+    ParsingError {
+        /// The underlying error
+        error: String,
+    },
 
     /// Represents a failure to parse a provided time unit precision value, PoSQL supports
     /// Seconds, Milliseconds, Microseconds, and Nanoseconds
-    #[error("Timestamp parsing error: {0}")]
-    UnsupportedPrecision(String),
+    #[snafu(display("Timestamp parsing error: {error}"))]
+    UnsupportedPrecision {
+        /// The underlying error
+        error: String,
+    },
 }
 
 // This exists because TryFrom<DataType> for ColumnType error is String

crates/proof-of-sql-parser/src/posql_time/timestamp.rs

@@ -61,8 +61,11 @@ impl PoSQLTimestamp {
     /// assert_eq!(intermediate_timestamp.timezone(), PoSQLTimeZone::FixedOffset(10800)); // 3 hours in seconds
     /// ```
     pub fn try_from(timestamp_str: &str) -> Result<Self, PoSQLTimestampError> {
-        let dt = DateTime::parse_from_rfc3339(timestamp_str)
-            .map_err(|e| PoSQLTimestampError::ParsingError(e.to_string()))?;
+        let dt = DateTime::parse_from_rfc3339(timestamp_str).map_err(|e| {
+            PoSQLTimestampError::ParsingError {
+                error: e.to_string(),
+            }
+        })?;
 
         let offset_seconds = dt.offset().local_minus_utc();
         let timezone = PoSQLTimeZone::from_offset(offset_seconds);
@@ -108,9 +111,9 @@ impl PoSQLTimestamp {
                 timeunit: PoSQLTimeUnit::Second,
                 timezone: PoSQLTimeZone::Utc,
             }),
-            LocalResult::Ambiguous(earliest, latest) => Err(PoSQLTimestampError::Ambiguous(
+            LocalResult::Ambiguous(earliest, latest) => Err(PoSQLTimestampError::Ambiguous{ error:
                 format!("The local time is ambiguous because there is a fold in the local time: earliest: {} latest: {} ", earliest, latest),
-            )),
+        }),
             LocalResult::None => Err(PoSQLTimestampError::LocalTimeDoesNotExist),
         }
     }
@@ -177,9 +180,9 @@ mod tests {
         let input = "not-a-timestamp";
         assert_eq!(
             PoSQLTimestamp::try_from(input),
-            Err(PoSQLTimestampError::ParsingError(
-                "input contains invalid characters".into()
-            ))
+            Err(PoSQLTimestampError::ParsingError {
+                error: "input contains invalid characters".into()
+            })
         );
     }
 
@@ -198,7 +201,10 @@ mod tests {
         // This test assumes that there's a catch-all parsing error case that isn't covered by the more specific errors.
         let malformed_input = "2009-01-03T::00Z"; // Intentionally malformed timestamp
         let result = PoSQLTimestamp::try_from(malformed_input);
-        assert!(matches!(result, Err(PoSQLTimestampError::ParsingError(_))));
+        assert!(matches!(
+            result,
+            Err(PoSQLTimestampError::ParsingError { .. })
+        ));
     }
 
     #[test]

crates/proof-of-sql-parser/src/posql_time/timezone.rs

@@ -45,7 +45,9 @@ impl TryFrom<&Option<Arc<str>>> for PoSQLTimeZone {
                         let total_seconds = sign * ((hours * 3600) + (minutes * 60));
                         Ok(PoSQLTimeZone::FixedOffset(total_seconds))
                     }
-                    _ => Err(PoSQLTimestampError::InvalidTimezone(tz.to_string())),
+                    _ => Err(PoSQLTimestampError::InvalidTimezone {
+                        timezone: tz.to_string(),
+                    }),
                 }
             }
             None => Ok(PoSQLTimeZone::Utc),

crates/proof-of-sql-parser/src/posql_time/unit.rs

@@ -23,7 +23,9 @@ impl TryFrom<&str> for PoSQLTimeUnit {
             "3" => Ok(PoSQLTimeUnit::Millisecond),
             "6" => Ok(PoSQLTimeUnit::Microsecond),
             "9" => Ok(PoSQLTimeUnit::Nanosecond),
-            _ => Err(PoSQLTimestampError::UnsupportedPrecision(value.into())),
+            _ => Err(PoSQLTimestampError::UnsupportedPrecision {
+                error: value.into(),
+            }),
         }
     }
 }
@@ -65,7 +67,7 @@ mod time_unit_tests {
             let result = PoSQLTimeUnit::try_from(value);
             assert!(matches!(
                 result,
-                Err(PoSQLTimestampError::UnsupportedPrecision(_))
+                Err(PoSQLTimestampError::UnsupportedPrecision { .. })
             ));
         }
     }

crates/proof-of-sql-parser/src/resource_id.rs

@@ -91,9 +91,11 @@ impl FromStr for ResourceId {
     type Err = ParseError;
 
     fn from_str(string: &str) -> ParseResult<Self> {
-        let (schema, object_name) = ResourceIdParser::new()
-            .parse(string)
-            .map_err(|e| ParseError::ResourceIdParseError(format!("{:?}", e)))?;
+        let (schema, object_name) = ResourceIdParser::new().parse(string).map_err(|e| {
+            ParseError::ResourceIdParseError {
+                error: format!("{:?}", e),
+            }
+        })?;
 
         // use unsafe `Identifier::new` to prevent double parsing the ids
         Ok(ResourceId {

crates/proof-of-sql-parser/src/select_statement.rs

@@ -62,7 +62,9 @@ impl FromStr for SelectStatement {
     fn from_str(query: &str) -> ParseResult<Self> {
         SelectStatementParser::new()
             .parse(query)
-            .map_err(|e| ParseError::QueryParseError(e.to_string()))
+            .map_err(|e| ParseError::QueryParseError {
+                error: e.to_string(),
+            })
     }
 }
 

crates/proof-of-sql/Cargo.toml

@@ -45,7 +45,7 @@ rand = { workspace = true, default-features = false, optional = true }
 rayon = { workspace = true, optional = true }
 serde = { workspace = true, features = ["serde_derive"] }
 serde_json = { workspace = true }
-thiserror = { workspace = true }
+snafu = { workspace = true }
 tiny-keccak = { workspace = true }
 tracing = { workspace = true, features = ["attributes"] }
 zerocopy = { workspace = true }

crates/proof-of-sql/src/base/commitment/column_bounds.rs

@@ -1,11 +1,11 @@
 use super::committable_column::CommittableColumn;
 use alloc::boxed::Box;
 use serde::{Deserialize, Serialize};
-use thiserror::Error;
+use snafu::Snafu;
 
 /// Cannot construct bounds where min is greater than max.
-#[derive(Error, Debug)]
-#[error("cannot construct bounds where min is greater than max")]
+#[derive(Snafu, Debug)]
+#[snafu(display("cannot construct bounds where min is greater than max"))]
 pub struct NegativeBounds;
 
 /// Inner value for [`Bounds::Sharp`] and [`Bounds::Bounded`].
@@ -187,9 +187,14 @@ where
 }
 
 /// Columns with different [`ColumnBounds`] variants cannot operate with each other.
-#[derive(Debug, Error)]
-#[error("column with bounds {0:?} cannot operate with column with bounds {1:?}")]
-pub struct ColumnBoundsMismatch(Box<ColumnBounds>, Box<ColumnBounds>);
+#[derive(Debug, Snafu)]
+#[snafu(display(
+    "column with bounds {bounds_a:?} cannot operate with column with bounds {bounds_b:?}"
+))]
+pub struct ColumnBoundsMismatch {
+    bounds_a: Box<ColumnBounds>,
+    bounds_b: Box<ColumnBounds>,
+}
 
 /// Column metadata storing the bounds for column types that have order.
 ///
@@ -254,9 +259,10 @@ impl ColumnBounds {
             (ColumnBounds::Int128(bounds_a), ColumnBounds::Int128(bounds_b)) => {
                 Ok(ColumnBounds::Int128(bounds_a.union(bounds_b)))
             }
-            (bounds_a, bounds_b) => {
-                Err(ColumnBoundsMismatch(Box::new(bounds_a), Box::new(bounds_b)))
-            }
+            (bounds_a, bounds_b) => Err(ColumnBoundsMismatch {
+                bounds_a: Box::new(bounds_a),
+                bounds_b: Box::new(bounds_b),
+            }),
         }
     }
 
@@ -282,7 +288,10 @@ impl ColumnBounds {
             (ColumnBounds::TimestampTZ(bounds_a), ColumnBounds::TimestampTZ(bounds_b)) => {
                 Ok(ColumnBounds::TimestampTZ(bounds_a.difference(bounds_b)))
             }
-            (_, _) => Err(ColumnBoundsMismatch(Box::new(self), Box::new(other))),
+            (_, _) => Err(ColumnBoundsMismatch {
+                bounds_a: Box::new(self),
+                bounds_b: Box::new(other),
+            }),
         }
     }
 }