From c7a4bc9f938fdd02e97abe0302d2d4a8fb402303 Mon Sep 17 00:00:00 2001 From: Seva Zhidkov Date: Fri, 2 Feb 2024 23:01:01 +0000 Subject: [PATCH] feat(db): propagate text error messages from mdbx (#6353) --- crates/interfaces/src/db.rs | 59 +++++++++++++------ crates/interfaces/src/error.rs | 2 +- .../db/src/implementation/mdbx/cursor.rs | 15 +++-- .../storage/db/src/implementation/mdbx/mod.rs | 9 +-- .../storage/db/src/implementation/mdbx/tx.rs | 6 +- 5 files changed, 56 insertions(+), 35 deletions(-) diff --git a/crates/interfaces/src/db.rs b/crates/interfaces/src/db.rs index b428f85d93..43b471a3c8 100644 --- a/crates/interfaces/src/db.rs +++ b/crates/interfaces/src/db.rs @@ -1,43 +1,64 @@ +use std::fmt::Display; use thiserror::Error; /// Database error type. #[derive(Clone, Debug, PartialEq, Eq, Error)] pub enum DatabaseError { /// Failed to open the database. - #[error("failed to open the database ({0})")] - Open(i32), + #[error("failed to open the database: {0}")] + Open(DatabaseErrorInfo), /// Failed to create a table in the database. - #[error("failed to create a table ({0})")] - CreateTable(i32), + #[error("failed to create a table: {0}")] + CreateTable(DatabaseErrorInfo), /// Failed to write a value into a table. #[error(transparent)] Write(#[from] Box), /// Failed to read a value from a table. - #[error("failed to read a value from a database table ({0})")] - Read(i32), + #[error("failed to read a value from a database table: {0}")] + Read(DatabaseErrorInfo), /// Failed to delete a `(key, value)` pair from a table. - #[error("database delete error code ({0})")] - Delete(i32), + #[error("database delete error code: {0}")] + Delete(DatabaseErrorInfo), /// Failed to commit transaction changes into the database. - #[error("failed to commit transaction changes ({0})")] - Commit(i32), + #[error("failed to commit transaction changes: {0}")] + Commit(DatabaseErrorInfo), /// Failed to initiate a transaction. - #[error("failed to initialize a transaction ({0})")] - InitTx(i32), + #[error("failed to initialize a transaction: {0}")] + InitTx(DatabaseErrorInfo), /// Failed to initialize a cursor. - #[error("failed to initialize a cursor ({0})")] - InitCursor(i32), + #[error("failed to initialize a cursor: {0}")] + InitCursor(DatabaseErrorInfo), /// Failed to decode a key from a table. #[error("failed to decode a key from a table")] Decode, /// Failed to get database stats. - #[error("failed to get stats ({0})")] - Stats(i32), + #[error("failed to get stats: {0}")] + Stats(DatabaseErrorInfo), /// Failed to use the specified log level, as it's not available. #[error("log level {0:?} is not available")] LogLevelUnavailable(LogLevel), } +/// Common error struct to propagate implementation-specific error information. +#[derive(Debug, Error, Clone, PartialEq, Eq)] +#[error("{message} ({code})")] +pub struct DatabaseErrorInfo { + /// Human-readable error message. + pub message: String, + /// Error code. + pub code: i32, +} + +impl From for DatabaseErrorInfo +where + E: Display + Into, +{ + #[inline] + fn from(value: E) -> Self { + Self { message: value.to_string(), code: value.into() } + } +} + impl From for DatabaseError { #[inline] fn from(value: DatabaseWriteError) -> Self { @@ -48,12 +69,12 @@ impl From for DatabaseError { /// Database write error. #[derive(Clone, Debug, PartialEq, Eq, Error)] #[error( - "write operation {operation:?} failed for key \"{key}\" in table {table_name:?} ({code})", + "write operation {operation:?} failed for key \"{key}\" in table {table_name:?}: {info}", key = reth_primitives::hex::encode(key), )] pub struct DatabaseWriteError { - /// The error code. - pub code: i32, + /// The error code and message. + pub info: DatabaseErrorInfo, /// The write operation type. pub operation: DatabaseWriteOperation, /// The table name. diff --git a/crates/interfaces/src/error.rs b/crates/interfaces/src/error.rs index dad569390d..73ce20d24b 100644 --- a/crates/interfaces/src/error.rs +++ b/crates/interfaces/src/error.rs @@ -73,7 +73,7 @@ mod size_asserts { static_assert_size!(RethError, 56); static_assert_size!(BlockExecutionError, 48); static_assert_size!(ConsensusError, 48); - static_assert_size!(DatabaseError, 16); + static_assert_size!(DatabaseError, 40); static_assert_size!(ProviderError, 48); static_assert_size!(NetworkError, 0); static_assert_size!(CanonicalError, 48); diff --git a/crates/storage/db/src/implementation/mdbx/cursor.rs b/crates/storage/db/src/implementation/mdbx/cursor.rs index 1c0cfa0e2b..4eec839996 100644 --- a/crates/storage/db/src/implementation/mdbx/cursor.rs +++ b/crates/storage/db/src/implementation/mdbx/cursor.rs @@ -11,7 +11,7 @@ use crate::{ tables::utils::*, DatabaseError, }; -use reth_interfaces::db::{DatabaseWriteError, DatabaseWriteOperation}; +use reth_interfaces::db::{DatabaseErrorInfo, DatabaseWriteError, DatabaseWriteOperation}; use reth_libmdbx::{self, Error as MDBXError, TransactionKind, WriteFlags, RO, RW}; use std::{borrow::Cow, collections::Bound, marker::PhantomData, ops::RangeBounds}; @@ -58,7 +58,7 @@ impl Cursor { /// Decodes a `(key, value)` pair from the database. #[allow(clippy::type_complexity)] pub fn decode( - res: Result, Cow<'_, [u8]>)>, impl Into>, + res: Result, Cow<'_, [u8]>)>, impl Into>, ) -> PairResult where T: Table, @@ -218,8 +218,7 @@ impl DbDupCursorRO for Cursor { .map_err(|e| DatabaseError::Read(e.into()))? .map(|val| decoder::((Cow::Owned(key), val))) } else { - let err_code = MDBXError::to_err_code(&MDBXError::NotFound); - Some(Err(DatabaseError::Read(err_code))) + Some(Err(DatabaseError::Read(MDBXError::NotFound.into()))) } } (None, None) => self.first().transpose(), @@ -248,7 +247,7 @@ impl DbCursorRW for Cursor { .put(key.as_ref(), value.unwrap_or(&this.buf), WriteFlags::UPSERT) .map_err(|e| { DatabaseWriteError { - code: e.into(), + info: e.into(), operation: DatabaseWriteOperation::CursorUpsert, table_name: T::NAME, key: key.into(), @@ -270,7 +269,7 @@ impl DbCursorRW for Cursor { .put(key.as_ref(), value.unwrap_or(&this.buf), WriteFlags::NO_OVERWRITE) .map_err(|e| { DatabaseWriteError { - code: e.into(), + info: e.into(), operation: DatabaseWriteOperation::CursorInsert, table_name: T::NAME, key: key.into(), @@ -294,7 +293,7 @@ impl DbCursorRW for Cursor { .put(key.as_ref(), value.unwrap_or(&this.buf), WriteFlags::APPEND) .map_err(|e| { DatabaseWriteError { - code: e.into(), + info: e.into(), operation: DatabaseWriteOperation::CursorAppend, table_name: T::NAME, key: key.into(), @@ -330,7 +329,7 @@ impl DbDupCursorRW for Cursor { .put(key.as_ref(), value.unwrap_or(&this.buf), WriteFlags::APPEND_DUP) .map_err(|e| { DatabaseWriteError { - code: e.into(), + info: e.into(), operation: DatabaseWriteOperation::CursorAppendDup, table_name: T::NAME, key: key.into(), diff --git a/crates/storage/db/src/implementation/mdbx/mod.rs b/crates/storage/db/src/implementation/mdbx/mod.rs index 2d1a654761..821287548d 100644 --- a/crates/storage/db/src/implementation/mdbx/mod.rs +++ b/crates/storage/db/src/implementation/mdbx/mod.rs @@ -363,6 +363,7 @@ mod tests { AccountChangeSet, }; use reth_interfaces::db::{DatabaseWriteError, DatabaseWriteOperation}; + use reth_libmdbx::Error; use reth_primitives::{Account, Address, Header, IntegerList, StorageEntry, B256, U256}; use std::{path::Path, str::FromStr, sync::Arc}; use tempfile::TempDir; @@ -725,7 +726,7 @@ mod tests { assert_eq!( cursor.insert(key_to_insert, B256::ZERO), Err(DatabaseWriteError { - code: -30799, + info: Error::KeyExist.into(), operation: DatabaseWriteOperation::CursorInsert, table_name: CanonicalHeaders::NAME, key: key_to_insert.encode().into(), @@ -869,7 +870,7 @@ mod tests { assert_eq!( cursor.append(key_to_append, B256::ZERO), Err(DatabaseWriteError { - code: -30418, + info: Error::KeyMismatch.into(), operation: DatabaseWriteOperation::CursorAppend, table_name: CanonicalHeaders::NAME, key: key_to_append.encode().into(), @@ -951,7 +952,7 @@ mod tests { AccountBeforeTx { address: Address::with_last_byte(subkey_to_append), info: None } ), Err(DatabaseWriteError { - code: -30418, + info: Error::KeyMismatch.into(), operation: DatabaseWriteOperation::CursorAppendDup, table_name: AccountChangeSet::NAME, key: transition_id.encode().into(), @@ -964,7 +965,7 @@ mod tests { AccountBeforeTx { address: Address::with_last_byte(subkey_to_append), info: None } ), Err(DatabaseWriteError { - code: -30418, + info: Error::KeyMismatch.into(), operation: DatabaseWriteOperation::CursorAppend, table_name: AccountChangeSet::NAME, key: (transition_id - 1).encode().into(), diff --git a/crates/storage/db/src/implementation/mdbx/tx.rs b/crates/storage/db/src/implementation/mdbx/tx.rs index cdf93b84e4..1e7ee3aabc 100644 --- a/crates/storage/db/src/implementation/mdbx/tx.rs +++ b/crates/storage/db/src/implementation/mdbx/tx.rs @@ -310,7 +310,7 @@ impl DbTxMut for Tx { |tx| { tx.put(self.get_dbi::()?, key.as_ref(), value, WriteFlags::UPSERT).map_err(|e| { DatabaseWriteError { - code: e.into(), + info: e.into(), operation: DatabaseWriteOperation::Put, table_name: T::NAME, key: key.into(), @@ -383,7 +383,7 @@ mod tests { assert_eq!( tx.get::(0).err(), - Some(DatabaseError::Open(reth_libmdbx::Error::NotFound.to_err_code())) + Some(DatabaseError::Open(reth_libmdbx::Error::NotFound.into())) ); // Transaction is not timeout-ed assert!(!tx.metrics_handler.unwrap().backtrace_recorded.load(Ordering::Relaxed)); // Backtrace is not recorded } @@ -402,7 +402,7 @@ mod tests { assert_eq!( tx.get::(0).err(), - Some(DatabaseError::Open(reth_libmdbx::Error::ReadTransactionAborted.to_err_code())) + Some(DatabaseError::Open(reth_libmdbx::Error::ReadTransactionAborted.into())) ); // Transaction is timeout-ed assert!(tx.metrics_handler.unwrap().backtrace_recorded.load(Ordering::Relaxed)); // Backtrace is recorded }