Better enums for errors

Author: simolus3

crates/core/src/checkpoint.rs

@@ -26,7 +26,8 @@ fn powersync_validate_checkpoint_impl(
     args: &[*mut sqlite::value],
 ) -> Result<String, PowerSyncError> {
     let data = args[0].text();
-    let checkpoint: Checkpoint = serde_json::from_str(data)?;
+    let checkpoint: Checkpoint =
+        serde_json::from_str(data).map_err(PowerSyncError::json_argument_error)?;
     let db = ctx.db_handle();
     let buckets: Vec<OwnedBucketChecksum> = checkpoint
         .buckets
@@ -45,7 +46,7 @@ fn powersync_validate_checkpoint_impl(
         failed_buckets: failed_buckets,
     };
 
-    Ok(json::to_string(&result)?)
+    Ok(json::to_string(&result).map_err(PowerSyncError::internal)?)
 }
 
 create_sqlite_text_fn!(

crates/core/src/crud_vtab.rs

@@ -12,7 +12,7 @@ use sqlite::{Connection, ResultCode, Value};
 use sqlite_nostd::ManagedStmt;
 use sqlite_nostd::{self as sqlite, ColumnType};
 
-use crate::error::{PowerSyncError, RawPowerSyncError};
+use crate::error::PowerSyncError;
 use crate::ext::SafeManagedStmt;
 use crate::schema::TableInfoFlags;
 use crate::state::DatabaseState;
@@ -85,7 +85,7 @@ impl VirtualTable {
         let current_tx = self
             .current_tx
             .as_mut()
-            .ok_or_else(|| PowerSyncError::from(RawPowerSyncError::CrudVtabOutsideOfTransaction))?;
+            .ok_or_else(|| PowerSyncError::state_error("Not in tx"))?;
         let db = self.db;
 
         if self.state.is_in_sync_local.load(Ordering::Relaxed) {
@@ -162,7 +162,8 @@ impl VirtualTable {
                     } else {
                         None
                     },
-                })?;
+                })
+                .map_err(PowerSyncError::internal)?;
                 stmt.bind_text(2, &serialized, sqlite::Destructor::STATIC)?;
                 stmt.exec()?;
 
@@ -187,7 +188,7 @@ impl VirtualTable {
         let tx_id = if statement.step()? == ResultCode::ROW {
             statement.column_int64(0) - 1
         } else {
-            return Err(PowerSyncError::from(RawPowerSyncError::Internal));
+            return Err(PowerSyncError::unknown_internal());
         };
 
         self.current_tx = Some(ActiveCrudTransaction {

crates/core/src/diff.rs

@@ -10,7 +10,7 @@ use sqlite_nostd::{Connection, Context, Value};
 use serde_json as json;
 
 use crate::create_sqlite_text_fn;
-use crate::error::{PowerSyncError, RawPowerSyncError};
+use crate::error::PowerSyncError;
 
 fn powersync_diff_impl(
     _ctx: *mut sqlite::context,
@@ -23,8 +23,10 @@ fn powersync_diff_impl(
 }
 
 pub fn diff_objects(data_old: &str, data_new: &str) -> Result<String, PowerSyncError> {
-    let v_new: json::Value = json::from_str(data_new)?;
-    let v_old: json::Value = json::from_str(data_old)?;
+    let v_new: json::Value =
+        json::from_str(data_new).map_err(PowerSyncError::json_argument_error)?;
+    let v_old: json::Value =
+        json::from_str(data_old).map_err(PowerSyncError::json_argument_error)?;
 
     if let (json::Value::Object(mut left), json::Value::Object(mut right)) = (v_new, v_old) {
         // Remove all null values
@@ -56,7 +58,7 @@ pub fn diff_objects(data_old: &str, data_new: &str) -> Result<String, PowerSyncE
 
         Ok(json::Value::Object(left).to_string())
     } else {
-        Err(RawPowerSyncError::ExpectedJsonObject.into())
+        return Err(PowerSyncError::argument_error("expected two JSON objects"));
     }
 }
 

crates/core/src/error.rs

@@ -27,14 +27,44 @@ impl PowerSyncError {
         .into()
     }
 
-    pub fn argument_error(desc: &'static str) -> Self {
-        RawPowerSyncError::ArgumentError { desc }.into()
+    pub fn argument_error(desc: impl Into<Cow<'static, str>>) -> Self {
+        RawPowerSyncError::ArgumentError {
+            desc: desc.into(),
+            cause: PowerSyncErrorCause::Unknown,
+        }
+        .into()
+    }
+
+    pub fn json_argument_error(cause: serde_json::Error) -> Self {
+        RawPowerSyncError::ArgumentError {
+            desc: "".into(),
+            cause: PowerSyncErrorCause::Json(cause),
+        }
+        .into()
+    }
+
+    pub fn json_local_error(cause: serde_json::Error) -> Self {
+        RawPowerSyncError::LocalDataError {
+            cause: PowerSyncErrorCause::Json(cause),
+        }
+        .into()
     }
 
     pub fn state_error(desc: &'static str) -> Self {
         RawPowerSyncError::StateError { desc }.into()
     }
 
+    pub fn unknown_internal() -> Self {
+        Self::internal(PowerSyncErrorCause::Unknown)
+    }
+
+    pub fn internal(cause: impl Into<PowerSyncErrorCause>) -> Self {
+        RawPowerSyncError::Internal {
+            cause: cause.into(),
+        }
+        .into()
+    }
+
     pub fn apply_to_ctx(self, description: &str, ctx: *mut context) {
         let mut desc = self.description(ctx.db_handle());
         desc.insert_str(0, description);
@@ -61,15 +91,10 @@ impl PowerSyncError {
 
         match self.inner.as_ref() {
             Sqlite { code, .. } => *code,
-            InvalidPendingStatement { .. }
-            | InvalidBucketPriority
-            | ExpectedJsonObject
-            | ArgumentError { .. }
-            | StateError { .. }
-            | JsonObjectTooBig
-            | CrudVtabOutsideOfTransaction => ResultCode::MISUSE,
-            MissingClientId | Internal => ResultCode::ABORT,
-            JsonError { .. } | BsonError { .. } => ResultCode::CONSTRAINT_DATATYPE,
+            InvalidBucketPriority | ArgumentError { .. } | StateError { .. } => ResultCode::MISUSE,
+            MissingClientId | SyncProtocolError { .. } => ResultCode::ABORT,
+            LocalDataError { .. } => ResultCode::CORRUPT,
+            Internal { .. } => ResultCode::INTERNAL,
         }
     }
 }
@@ -98,48 +123,85 @@ impl From<ResultCode> for PowerSyncError {
     }
 }
 
-impl From<serde_json::Error> for PowerSyncError {
-    fn from(value: serde_json::Error) -> Self {
-        RawPowerSyncError::JsonError(value).into()
-    }
-}
-
-impl From<BsonError> for PowerSyncError {
-    fn from(value: BsonError) -> Self {
-        RawPowerSyncError::from(value).into()
-    }
-}
-
+/// A structured enumeration of possible errors that can occur in the core extension.
 #[derive(Error, Debug)]
 pub enum RawPowerSyncError {
+    /// An internal call to SQLite made by the core extension has failed. We store the original
+    /// result code and an optional context describing what the core extension was trying to do when
+    /// the error occurred.
+    ///
+    /// We don't call `sqlite3_errstr` at the time the error is created. Instead, we stop using the
+    /// database, bubble the error up to the outermost function/vtab definition and then use
+    /// [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("invalid argument: {desc}")]
-    ArgumentError { desc: &'static str },
+    /// A user (e.g. the one calling a PowerSync function, likely an SDK) has provided invalid
+    /// arguments.
+    ///
+    /// This always indicates an error in how the core extension is used.
+    #[error("invalid argument: {desc}. {cause}")]
+    ArgumentError {
+        desc: Cow<'static, str>,
+        cause: PowerSyncErrorCause,
+    },
+    /// A PowerSync function or vtab was used in a state where it's unavailable.
+    ///
+    /// This always indicates an error in how the core extension is used.
     #[error("invalid state: {desc}")]
     StateError { desc: &'static str },
-    #[error("Function required a JSON object, but got another type of JSON value")]
-    ExpectedJsonObject,
+    /// We've received a sync line we couldn't parse, or in a state where it doesn't make sense
+    /// (e.g. a checkpoint diff before we've ever received a checkpoint).
+    ///
+    /// This interrupts a sync iteration as we cannot reasonably continue afterwards (the client and
+    /// server are necessarily in different states).
+    #[error("Sync protocol error: {desc}. {cause}")]
+    SyncProtocolError {
+        desc: &'static str,
+        cause: PowerSyncErrorCause,
+    },
+    /// There's invalid local data in the database (like malformed JSON in the oplog table).
+    #[error("invalid local data")]
+    LocalDataError { cause: PowerSyncErrorCause },
     #[error("No client_id found in ps_kv")]
     MissingClientId,
-    #[error("Invalid pending statement for raw table: {description}")]
-    InvalidPendingStatement { description: Cow<'static, str> },
     #[error("Invalid bucket priority value")]
     InvalidBucketPriority,
-    #[error("Internal PowerSync error")]
-    Internal,
-    #[error("Error decoding JSON: {0}")]
-    JsonError(serde_json::Error),
-    #[error("Error decoding BSON")]
-    BsonError {
-        #[from]
-        source: BsonError,
-    },
-    #[error("Too many arguments passed to json_object_fragment")]
-    JsonObjectTooBig,
-    #[error("No tx_id")]
-    CrudVtabOutsideOfTransaction,
+    #[error("Internal PowerSync error. {cause}")]
+    Internal { cause: PowerSyncErrorCause },
+}
+
+#[derive(Debug)]
+pub enum PowerSyncErrorCause {
+    Json(serde_json::Error),
+    Bson(BsonError),
+    Unknown,
+}
+
+impl From<serde_json::Error> for PowerSyncErrorCause {
+    fn from(value: serde_json::Error) -> Self {
+        return PowerSyncErrorCause::Json(value);
+    }
+}
+
+impl From<BsonError> for PowerSyncErrorCause {
+    fn from(value: BsonError) -> Self {
+        return PowerSyncErrorCause::Bson(value);
+    }
+}
+
+impl Display for PowerSyncErrorCause {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        write!(f, "cause: ")?;
+
+        match self {
+            PowerSyncErrorCause::Json(error) => error.fmt(f),
+            PowerSyncErrorCause::Bson(error) => error.fmt(f),
+            PowerSyncErrorCause::Unknown => write!(f, "unknown"),
+        }
+    }
 }

crates/core/src/json_merge.rs

@@ -8,7 +8,7 @@ use sqlite_nostd as sqlite;
 use sqlite_nostd::{Connection, Context, Value};
 
 use crate::create_sqlite_text_fn;
-use crate::error::{PowerSyncError, RawPowerSyncError};
+use crate::error::PowerSyncError;
 
 /// Given any number of JSON TEXT arguments, merge them into a single JSON object.
 ///
@@ -25,7 +25,7 @@ fn powersync_json_merge_impl(
     for arg in args {
         let chunk = arg.text();
         if chunk.is_empty() || !chunk.starts_with('{') || !chunk.ends_with('}') {
-            return Err(RawPowerSyncError::ExpectedJsonObject.into());
+            return Err(PowerSyncError::argument_error("Expected json object"));
         }
 
         // Strip outer braces

crates/core/src/migrations.rs

@@ -8,7 +8,7 @@ use sqlite::ResultCode;
 use sqlite_nostd as sqlite;
 use sqlite_nostd::{Connection, Context};
 
-use crate::error::{PowerSyncError, RawPowerSyncError};
+use crate::error::PowerSyncError;
 use crate::fix_data::apply_v035_fix;
 use crate::sync::BucketPriority;
 
@@ -31,7 +31,7 @@ CREATE TABLE IF NOT EXISTS ps_migration(id INTEGER PRIMARY KEY, down_migrations
         local_db.prepare_v2("SELECT ifnull(max(id), 0) as version FROM ps_migration")?;
     let rc = current_version_stmt.step()?;
     if rc != ResultCode::ROW {
-        return Err(PowerSyncError::from(RawPowerSyncError::Internal));
+        return Err(PowerSyncError::unknown_internal());
     }
 
     let mut current_version = current_version_stmt.column_int(0);

crates/core/src/operations.rs

@@ -17,7 +17,8 @@ pub fn insert_operation(db: *mut sqlite::sqlite3, data: &str) -> Result<(), Powe
         buckets: Vec<DataLine<'a>>,
     }
 
-    let batch: BucketBatch = serde_json::from_str(data)?;
+    let batch: BucketBatch =
+        serde_json::from_str(data).map_err(PowerSyncError::json_argument_error)?;
     let adapter = StorageAdapter::new(db)?;
 
     for line in &batch.buckets {

crates/core/src/schema/management.rs

@@ -132,7 +132,8 @@ SELECT name, internal_name, local_only FROM powersync_tables WHERE name NOT IN (
 
 fn update_indexes(db: *mut sqlite::sqlite3, schema: &str) -> Result<(), PowerSyncError> {
     let mut statements: Vec<String> = alloc::vec![];
-    let schema = serde_json::from_str::<Schema>(schema)?;
+    let schema =
+        serde_json::from_str::<Schema>(schema).map_err(PowerSyncError::json_argument_error)?;
     let mut expected_index_names: Vec<String> = vec![];
 
     {
@@ -205,7 +206,8 @@ SELECT
             AND sqlite_master.name NOT IN (SELECT value FROM json_each(?))
 ",
         )?;
-        let json_names = serde_json::to_string(&expected_index_names)?;
+        let json_names =
+            serde_json::to_string(&expected_index_names).map_err(PowerSyncError::internal)?;
         statement.bind_text(1, &json_names, sqlite::Destructor::STATIC)?;
 
         while statement.step()? == ResultCode::ROW {

crates/core/src/sync/interface.rs

@@ -11,7 +11,7 @@ use sqlite::{ResultCode, Value};
 use sqlite_nostd::{self as sqlite, ColumnType};
 use sqlite_nostd::{Connection, Context};
 
-use crate::error::{PowerSyncError, RawPowerSyncError};
+use crate::error::PowerSyncError;
 use crate::schema::Schema;
 use crate::state::DatabaseState;
 
@@ -132,12 +132,12 @@ pub fn register(db: *mut sqlite::sqlite3, state: Arc<DatabaseState>) -> Result<(
             debug_assert!(!ctx.db_handle().get_autocommit());
 
             let controller = unsafe { ctx.user_data().cast::<SqlController>().as_mut() }
-                .ok_or_else(|| PowerSyncError::from(RawPowerSyncError::Internal))?;
+                .ok_or_else(|| PowerSyncError::unknown_internal())?;
 
             let args = sqlite::args!(argc, argv);
             let [op, payload] = args else {
                 // This should be unreachable, we register the function with two arguments.
-                return Err(PowerSyncError::from(RawPowerSyncError::Internal));
+                return Err(PowerSyncError::unknown_internal());
             };
 
             if op.value_type() != ColumnType::Text {
@@ -150,7 +150,8 @@ pub fn register(db: *mut sqlite::sqlite3, state: Arc<DatabaseState>) -> Result<(
             let event = match op {
                 "start" => SyncControlRequest::StartSyncStream({
                     if payload.value_type() == ColumnType::Text {
-                        serde_json::from_str(payload.text())?
+                        serde_json::from_str(payload.text())
+                            .map_err(PowerSyncError::json_argument_error)?
                     } else {
                         StartSyncStream::default()
                     }
@@ -182,7 +183,8 @@ pub fn register(db: *mut sqlite::sqlite3, state: Arc<DatabaseState>) -> Result<(
             };
 
             let instructions = controller.client.push_event(event)?;
-            let formatted = serde_json::to_string(&instructions)?;
+            let formatted =
+                serde_json::to_string(&instructions).map_err(PowerSyncError::internal)?;
             ctx.result_text_transient(&formatted);
 
             Ok(())

crates/core/src/sync/storage_adapter.rs

@@ -192,7 +192,8 @@ impl StorageAdapter {
                 };
 
                 // TODO: Avoid this serialization, it's currently used to bind JSON SQL parameters.
-                let serialized_args = serde_json::to_string(&args)?;
+                let serialized_args =
+                    serde_json::to_string(&args).map_err(PowerSyncError::internal)?;
                 let mut sync = SyncOperation::new(
                     state,
                     self.db,