Bring back db result utility

Author: simolus3

crates/core/src/crud_vtab.rs

@@ -329,7 +329,7 @@ extern "C" fn disconnect(vtab: *mut sqlite::vtab) -> c_int {
 extern "C" fn begin(vtab: *mut sqlite::vtab) -> c_int {
     let tab = unsafe { &mut *(vtab.cast::<VirtualTable>()) };
     let result = tab.begin();
-    vtab_result(vtab, tab.db, result)
+    vtab_result(vtab, result)
 }
 
 extern "C" fn commit(vtab: *mut sqlite::vtab) -> c_int {
@@ -362,7 +362,7 @@ extern "C" fn update(
         // INSERT
         let tab = unsafe { &mut *(vtab.cast::<VirtualTable>()) };
         let result = tab.handle_insert(&args[2..]);
-        vtab_result(vtab, tab.db, result)
+        vtab_result(vtab, result)
     } else {
         // UPDATE - not supported
         ResultCode::MISUSE as c_int

crates/core/src/error.rs

@@ -1,9 +1,8 @@
-use core::fmt::Display;
+use core::{error::Error, fmt::Display};
 
 use alloc::{
     borrow::Cow,
     boxed::Box,
-    format,
     string::{String, ToString},
 };
 use sqlite_nostd::{context, sqlite3, Connection, Context, ResultCode};
@@ -15,16 +14,31 @@ use crate::bson::BsonError;
 ///
 /// We allocate errors in boxes to avoid large [Result] types (given the large size of the
 /// [RawPowerSyncError] enum type).
+#[derive(Debug)]
 pub struct PowerSyncError {
     inner: Box<RawPowerSyncError>,
 }
 
 impl PowerSyncError {
-    pub fn from_sqlite(code: ResultCode, context: impl Into<Cow<'static, str>>) -> Self {
-        RawPowerSyncError::Sqlite {
+    fn errstr(db: *mut sqlite3) -> Option<String> {
+        let message = db.errmsg().unwrap_or(String::from("Conversion error"));
+        if message != "not an error" {
+            Some(message)
+        } else {
+            None
+        }
+    }
+
+    pub fn from_sqlite(
+        db: *mut sqlite3,
+        code: ResultCode,
+        context: impl Into<Cow<'static, str>>,
+    ) -> Self {
+        RawPowerSyncError::Sqlite(SqliteError {
             code,
+            errstr: Self::errstr(db),
             context: Some(context.into()),
-        }
+        })
         .into()
     }
 
@@ -93,32 +107,21 @@ impl PowerSyncError {
     /// Applies this error to a function result context, setting the error code and a descriptive
     /// text.
     pub fn apply_to_ctx(self, description: &str, ctx: *mut context) {
-        let mut desc = self.description(ctx.db_handle());
+        let mut desc = self.to_string();
         desc.insert_str(0, description);
         desc.insert_str(description.len(), ": ");
 
         ctx.result_error(&desc);
         ctx.result_error_code(self.sqlite_error_code());
     }
 
-    /// Obtains a description of this error, fetching it from SQLite if necessary.
-    pub fn description(&self, db: *mut sqlite3) -> String {
-        if let RawPowerSyncError::Sqlite { .. } = &*self.inner {
-            let message = db.errmsg().unwrap_or(String::from("Conversion error"));
-            if message != "not an error" {
-                return format!("{}, caused by: {message}", self.inner);
-            }
-        }
-
-        self.inner.to_string()
-    }
-
     pub fn sqlite_error_code(&self) -> ResultCode {
         use RawPowerSyncError::*;
 
         match self.inner.as_ref() {
-            Sqlite { code, .. } => *code,
-            ArgumentError { .. } | StateError { .. } => ResultCode::MISUSE,
+            Sqlite(desc) => desc.code,
+            ArgumentError { .. } => ResultCode::CONSTRAINT_DATATYPE,
+            StateError { .. } => ResultCode::MISUSE,
             MissingClientId
             | SyncProtocolError { .. }
             | DownMigrationDidNotUpdateVersion { .. } => ResultCode::ABORT,
@@ -134,6 +137,8 @@ impl Display for PowerSyncError {
     }
 }
 
+impl Error for PowerSyncError {}
+
 impl From<RawPowerSyncError> for PowerSyncError {
     fn from(value: RawPowerSyncError) -> Self {
         return PowerSyncError {
@@ -144,10 +149,11 @@ impl From<RawPowerSyncError> for PowerSyncError {
 
 impl From<ResultCode> for PowerSyncError {
     fn from(value: ResultCode) -> Self {
-        return RawPowerSyncError::Sqlite {
+        return RawPowerSyncError::Sqlite(SqliteError {
             code: value,
+            errstr: None,
             context: None,
-        }
+        })
         .into();
     }
 }
@@ -164,11 +170,8 @@ enum RawPowerSyncError {
     /// [PowerSyncError::description] to create a detailed error message.
     ///
     /// This error should _never_ be created for anything but rethrowing underlying SQLite errors.
-    #[error("internal SQLite call returned {code}")]
-    Sqlite {
-        code: ResultCode,
-        context: Option<Cow<'static, str>>,
-    },
+    #[error("{0}")]
+    Sqlite(SqliteError),
     /// A user (e.g. the one calling a PowerSync function, likely an SDK) has provided invalid
     /// arguments.
     ///
@@ -205,6 +208,45 @@ enum RawPowerSyncError {
     Internal { cause: PowerSyncErrorCause },
 }
 
+#[derive(Debug)]
+struct SqliteError {
+    code: ResultCode,
+    errstr: Option<String>,
+    context: Option<Cow<'static, str>>,
+}
+
+impl Display for SqliteError {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        if let Some(context) = &self.context {
+            write!(f, "{}: ", context)?;
+        }
+
+        write!(f, "internal SQLite call returned {}", self.code)?;
+        if let Some(desc) = &self.errstr {
+            write!(f, ": {}", desc)?
+        }
+
+        Ok(())
+    }
+}
+
+pub trait PSResult<T> {
+    fn into_db_result(self, db: *mut sqlite3) -> Result<T, PowerSyncError>;
+}
+
+impl<T> PSResult<T> for Result<T, ResultCode> {
+    fn into_db_result(self, db: *mut sqlite3) -> Result<T, PowerSyncError> {
+        self.map_err(|code| {
+            RawPowerSyncError::Sqlite(SqliteError {
+                code,
+                errstr: PowerSyncError::errstr(db),
+                context: None,
+            })
+            .into()
+        })
+    }
+}
+
 #[derive(Debug)]
 pub enum PowerSyncErrorCause {
     Json(serde_json::Error),

crates/core/src/migrations.rs

@@ -56,6 +56,7 @@ CREATE TABLE IF NOT EXISTS ps_migration(id INTEGER PRIMARY KEY, down_migrations
             let rs = local_db.exec_safe(&sql);
             if let Err(code) = rs {
                 return Err(PowerSyncError::from_sqlite(
+                    local_db,
                     code,
                     format!(
                         "Down migration failed for {:} {:} {:}",
@@ -74,6 +75,7 @@ CREATE TABLE IF NOT EXISTS ps_migration(id INTEGER PRIMARY KEY, down_migrations
         let rc = current_version_stmt.step()?;
         if rc != ResultCode::ROW {
             return Err(PowerSyncError::from_sqlite(
+                local_db,
                 rc,
                 "Down migration failed - could not get version",
             ));

crates/core/src/operations_vtab.rs

@@ -92,24 +92,24 @@ extern "C" fn update(
 
         if op == "save" {
             let result = insert_operation(db, args[3].text());
-            vtab_result(vtab, db, result)
+            vtab_result(vtab, result)
         } else if op == "sync_local" {
             let result = sync_local(&tab.state, db, &args[3]);
             if let Ok(result_row) = result {
                 unsafe {
                     *p_row_id = result_row;
                 }
             }
-            vtab_result(vtab, db, result)
+            vtab_result(vtab, result)
         } else if op == "clear_remove_ops" {
             let result = clear_remove_ops(db, args[3].text());
-            vtab_result(vtab, db, result)
+            vtab_result(vtab, result)
         } else if op == "delete_pending_buckets" {
             let result = delete_pending_buckets(db, args[3].text());
-            vtab_result(vtab, db, result)
+            vtab_result(vtab, result)
         } else if op == "delete_bucket" {
             let result: Result<(), ResultCode> = delete_bucket(db, args[3].text());
-            vtab_result(vtab, db, result)
+            vtab_result(vtab, result)
         } else {
             ResultCode::MISUSE as c_int
         }

crates/core/src/vtab_util.rs

@@ -1,5 +1,6 @@
 extern crate alloc;
 
+use alloc::string::ToString;
 use core::ffi::{c_char, c_int};
 
 use sqlite::ResultCode;
@@ -65,13 +66,12 @@ pub extern "C" fn vtab_no_close(_cursor: *mut sqlite::vtab_cursor) -> c_int {
 
 pub fn vtab_result<T, E: Into<PowerSyncError>>(
     vtab: *mut sqlite::vtab,
-    db: *mut sqlite::sqlite3,
     result: Result<T, E>,
 ) -> c_int {
     if let Err(error) = result {
         let error = error.into();
 
-        vtab.set_err(&error.description(db));
+        vtab.set_err(&error.to_string());
         error.sqlite_error_code() as c_int
     } else {
         ResultCode::OK as c_int

dart/test/error_test.dart

@@ -0,0 +1,42 @@
+import 'dart:convert';
+
+import 'package:sqlite3/common.dart';
+import 'package:test/test.dart';
+
+import 'utils/native_test_utils.dart';
+
+void main() {
+  group('error reporting', () {
+    late CommonDatabase db;
+
+    setUp(() async {
+      db = openTestDatabase();
+    });
+
+    tearDown(() {
+      db.dispose();
+    });
+
+    test('contain inner SQLite descriptions', () {
+      expect(
+        () => db.execute('SELECT powersync_replace_schema(?)', [
+          json.encode({
+            // This fails because we're trying to json_extract from the string
+            // in e.g. update_tables.
+            'tables': ['invalid entry'],
+          })
+        ]),
+        throwsA(isSqliteException(
+          1,
+          'powersync_replace_schema: internal SQLite call returned ERROR',
+        )),
+      );
+    });
+  });
+}
+
+Matcher isSqliteException(int code, String message) {
+  return isA<SqliteException>()
+      .having((e) => e.extendedResultCode, 'extendedResultCode', code)
+      .having((e) => e.message, 'message', message);
+}