Cleanup db crate, introduce explicit RowError type

crates/bitwarden-db/src/error.rs

@@ -1,6 +1,6 @@
 use thiserror::Error;
 
-use crate::migrator::MigratorError;
+use crate::RowError;
 
 #[derive(Debug, Error)]
 pub enum DatabaseError {
@@ -10,13 +10,6 @@ pub enum DatabaseError {
     #[error("Failed to open connection to database")]
     FailedToOpenConnection,
 
-    #[error(transparent)]
-    Migrator(#[from] MigratorError),
-
-    // #[error(transparent)]
-    // SerdeJson(#[from] serde_json::Error),
-    // #[error(transparent)]
-    // MissingField(#[from] MissingFieldError),
     #[error("Unable to get version")]
     UnableToGetVersion,
     #[error("Unable to set version")]
@@ -25,4 +18,8 @@ pub enum DatabaseError {
     #[cfg(not(target_arch = "wasm32"))]
     #[error(transparent)]
     Rusqlite(#[from] rusqlite::Error),
+
+    // Used on the consuming side, resolves to rusqlite for sqlite and our RowError for wasm
+    #[error(transparent)]
+    RowError(RowError),
 }

crates/bitwarden-db/src/lib.rs

@@ -4,30 +4,29 @@ pub use error::DatabaseError;
 mod migrator;
 pub use migrator::*;
 
-// #[cfg(all(feature = "sqlite", feature = "wasm"))]
-// compile_error!("Sqlite and wasm are mutually exclusive and cannot be enabled together");
-
 #[cfg(not(target_arch = "wasm32"))]
 mod sqlite;
 #[cfg(not(target_arch = "wasm32"))]
 pub type Database = sqlite::SqliteDatabase;
 #[cfg(not(target_arch = "wasm32"))]
+pub type RowError = sqlite::RowError;
+#[cfg(not(target_arch = "wasm32"))]
 pub use sqlite::{named_params, params, Params, Row};
 
 #[cfg(target_arch = "wasm32")]
 mod wasm;
 #[cfg(target_arch = "wasm32")]
 pub type Database = wasm::WasmDatabase;
 #[cfg(target_arch = "wasm32")]
+pub type RowError = wasm::RowError;
+#[cfg(target_arch = "wasm32")]
 pub use wasm::{Params, Row, ToSql};
 
 /// Persistent storage for the Bitwarden SDK
 ///
 /// The database is used to store the user's data, such as ciphers, folders, and settings.
 /// Since we need to support multiple platforms, the database is abstracted to allow for different
 /// implementations.
-///
-/// The default and recommended implementation is SqliteDatabase.
 pub trait DatabaseTrait {
     async fn get_version(&self) -> Result<usize, DatabaseError>;
     async fn set_version(&self, version: usize) -> Result<(), DatabaseError>;
@@ -46,5 +45,5 @@ pub trait DatabaseTrait {
         row_to_type: F,
     ) -> Result<Vec<T>, DatabaseError>
     where
-        F: Fn(&Row) -> Result<T, DatabaseError>;
+        F: Fn(&Row) -> Result<T, RowError>;
 }

crates/bitwarden-db/src/migrator.rs

@@ -3,12 +3,18 @@ use std::{borrow::Cow, cmp::Ordering};
 use log::info;
 use thiserror::Error;
 
-use crate::{Database, DatabaseTrait};
+use crate::{Database, DatabaseError, DatabaseTrait};
 
 #[derive(Debug, Error)]
 pub enum MigratorError {
     #[error("Internal error: {0}")]
     Internal(Cow<'static, str>),
+
+    #[error("Failed to apply migrations")]
+    MigrationFailed,
+
+    #[error(transparent)]
+    Database(#[from] DatabaseError),
 }
 
 /// Database migrator
@@ -26,15 +32,13 @@ impl Migrator {
         }
     }
 
+    /// Migrate the database to the target version, or the last migration if not specified
     pub async fn migrate(
         &self,
         db: &Database,
         target_version: Option<usize>,
     ) -> Result<(), MigratorError> {
-        let current_version = db
-            .get_version()
-            .await
-            .map_err(|_| MigratorError::Internal("Failed to get user_version".into()))?;
+        let current_version = db.get_version().await?;
 
         let target_version = target_version.unwrap_or(self.migrations.len());
 
@@ -52,19 +56,17 @@ impl Migrator {
                 true => {
                     db.execute_batch(migration.up)
                         .await
-                        .map_err(|_| MigratorError::Internal("Failed to apply migration".into()))?;
+                        .map_err(|_| MigratorError::MigrationFailed)?;
                 }
                 false => {
                     db.execute_batch(migration.down)
                         .await
-                        .map_err(|_| MigratorError::Internal("Failed to apply migration".into()))?;
+                        .map_err(|_| MigratorError::MigrationFailed)?;
                 }
             }
         }
 
-        db.set_version(target_version)
-            .await
-            .map_err(|_| MigratorError::Internal("Failed to set user_version".into()))?;
+        db.set_version(target_version).await?;
 
         Ok(())
     }

crates/bitwarden-db/src/sqlite.rs

@@ -4,6 +4,8 @@ use tokio::sync::Mutex;
 
 use super::{DatabaseError, DatabaseTrait};
 
+pub type RowError = rusqlite::Error;
+
 #[derive(Debug)]
 pub struct SqliteDatabase {
     conn: Mutex<Connection>,
@@ -81,16 +83,15 @@ impl DatabaseTrait for SqliteDatabase {
         row_to_type: F,
     ) -> Result<Vec<T>, DatabaseError>
     where
-        F: Fn(&Row) -> Result<T, DatabaseError>,
+        F: Fn(&Row) -> Result<T, RowError>,
     {
         let guard = self.conn.lock().await;
 
         let mut stmt = guard.prepare(query)?;
-        let rows: Result<Vec<T>, rusqlite::Error> = stmt
-            .query_map(params, |row| row_to_type(row).map_err(|err| err.into()))?
-            .collect();
+        let rows: Result<Vec<T>, rusqlite::Error> =
+            stmt.query_map(params, |row| row_to_type(row))?.collect();
 
-        Ok(rows?)
+        rows.map_err(DatabaseError::RowError)
     }
 }
 
@@ -132,14 +133,12 @@ mod tests {
             .await
             .unwrap();
 
-        /*
-        let count: i64 = db
-            .conn
-            .query_row("SELECT COUNT(*) FROM test", [], |row| row.get(0))
+        let count: Vec<i64> = db
+            .query_map("SELECT COUNT(*) FROM test", [], |row| row.get(0))
+            .await
             .unwrap();
 
-        assert_eq!(count, 1);
-        */
+        assert_eq!(*count.first().unwrap(), 1);
     }
 
     #[tokio::test]

crates/bitwarden-db/src/wasm/mod.rs

@@ -1,3 +1,5 @@
+use std::borrow::Cow;
+use thiserror::Error;
 use wasm_bindgen::{prelude::wasm_bindgen, JsValue};
 
 use super::{DatabaseError, DatabaseTrait};
@@ -6,6 +8,12 @@ mod params;
 use params::FromSql;
 pub use params::{Params, ToSql};
 
+#[derive(Debug, Error)]
+pub enum RowError {
+    #[error("Internal error: {0}")]
+    Internal(Cow<'static, str>),
+}
+
 #[wasm_bindgen]
 extern "C" {
     type SqliteDatabase;
@@ -88,7 +96,7 @@ impl DatabaseTrait for WasmDatabase {
     ) -> Result<Vec<T>, DatabaseError>
     where
         P: Params,
-        F: Fn(&Row) -> Result<T, DatabaseError>,
+        F: Fn(&Row) -> Result<T, RowError>,
     {
         let result = self.db.query_map(sql, params.to_sql()).await;
 
@@ -99,7 +107,7 @@ impl DatabaseTrait for WasmDatabase {
                 let row = Row {
                     data: data.to_vec(),
                 };
-                row_to_type(&row)
+                row_to_type(&row).map_err(DatabaseError::RowError)
             })
             .collect::<Result<Vec<T>, DatabaseError>>()?;
 
@@ -112,7 +120,7 @@ pub struct Row {
 }
 
 impl Row {
-    pub fn get<T: FromSql>(&self, idx: u8) -> Result<T, DatabaseError> {
+    pub fn get<T: FromSql>(&self, idx: u8) -> Result<T, RowError> {
         let value = self.data.get(idx as usize).expect("ABLE TO UNWRAP");
 
         let result = T::from_sql(value.clone());

crates/bitwarden-vault/src/cipher/repository.rs

@@ -1,7 +1,7 @@
 use std::sync::Arc;
 
 use bitwarden_core::{require, MissingFieldError};
-use bitwarden_db::{named_params, params, Database, DatabaseError, DatabaseTrait};
+use bitwarden_db::{named_params, params, Database, DatabaseError, DatabaseTrait, RowError};
 use thiserror::Error;
 use tokio::sync::Mutex;
 use uuid::Uuid;
@@ -91,7 +91,7 @@ impl CipherRepository {
             .query_map(
                 "SELECT id, value FROM ciphers",
                 [],
-                |row| -> Result<CipherRow, DatabaseError> {
+                |row| -> Result<CipherRow, RowError> {
                     Ok(CipherRow {
                         id: row.get(0)?,
                         value: row.get(1)?,