Skip to content

Improve error handling #211

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
simolus3 merged 3 commits into from
Aug 5, 2026
Merged

Improve error handling #211

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
00b8451
Improve error handling
simolus3 Aug 5, 2026
9bda735
Use result alias
simolus3 Aug 5, 2026
429b2e1
Guard against sqlite3_sql returning null (it shouldn't)
simolus3 Aug 5, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 26 additions & 30 deletions crates/core/src/crud_vtab.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,18 @@ use core::ffi::{CStr, c_char, c_int, c_void};
use serde::Serialize;
use serde_json::value::RawValue;

use powersync_sqlite_nostd::ManagedStmt;
use powersync_sqlite_nostd::{self as sqlite, ColumnType};
use sqlite::{Connection, ResultCode, Value};
use sqlite::{ResultCode, Value};

use crate::error::PowerSyncError;
use crate::ext::SafeManagedStmt;
use crate::error::{PowerSyncError, Result};
use crate::schema::TableInfoFlags;
use crate::state::DatabaseState;
use crate::sync::storage_adapter::{
LAST_APPLIED_CHECKPOINT_REQUEST_ID_KEY, LAST_SEEN_CHECKPOINT_REQUEST_ID_KEY,
TARGET_CHECKPOINT_REQUEST_ID_KEY,
};
use crate::utils::MAX_OP_ID;
use crate::utils::database::{Database, Statement};
use crate::vtab_util::*;

const MANUAL_NAME: &CStr = c"powersync_crud_";
Expand All @@ -42,7 +41,7 @@ const SIMPLE_NAME: &CStr = c"powersync_crud";
#[repr(C)]
struct VirtualTable {
base: sqlite::vtab,
db: *mut sqlite::sqlite3,
db: Database,
current_tx: Option<ActiveCrudTransaction>,
is_simple: bool,
state: Rc<DatabaseState>,
Expand All @@ -60,13 +59,13 @@ enum CrudTransactionMode {

#[derive(Default)]
struct ManualCrudTransactionMode {
stmt: Option<ManagedStmt>,
stmt: Option<Statement>,
}

#[derive(Default)]
struct SimpleCrudTransactionMode {
stmt: Option<ManagedStmt>,
set_updated_rows: Option<ManagedStmt>,
stmt: Option<Statement>,
set_updated_rows: Option<Statement>,
had_writes: bool,
}

Expand All @@ -84,7 +83,7 @@ impl VirtualTable {
}
}

fn handle_insert(&mut self, args: &[*mut sqlite::value]) -> Result<(), PowerSyncError> {
fn handle_insert(&mut self, args: &[*mut sqlite::value]) -> Result<()> {
let current_tx = self
.current_tx
.as_mut()
Expand Down Expand Up @@ -182,13 +181,13 @@ impl VirtualTable {
Ok(())
}

fn begin(&mut self) -> Result<(), PowerSyncError> {
fn begin(&mut self) -> Result<()> {
let db = self.db;

// language=SQLite
let statement =
db.prepare_v2("UPDATE ps_tx SET next_tx = next_tx + 1 WHERE id = 1 RETURNING next_tx")?;
let tx_id = if statement.step()? == ResultCode::ROW {
let tx_id = if statement.step()? {
statement.column_int64(0) - 1
} else {
return Err(PowerSyncError::unknown_internal());
Expand All @@ -212,7 +211,7 @@ impl VirtualTable {
}

impl ManualCrudTransactionMode {
fn raw_crud_statement(&mut self, db: *mut sqlite::sqlite3) -> Result<&ManagedStmt, ResultCode> {
fn raw_crud_statement(&mut self, db: Database) -> Result<&Statement> {
prepare_lazy(&mut self.stmt, || {
const SQL: &str = formatcp!(
"\
Expand All @@ -223,39 +222,33 @@ SELECT * FROM insertion WHERE (NOT (?3 & {})) OR data->>'op' != 'PATCH' OR data-
TableInfoFlags::IGNORE_EMPTY_UPDATE
);

db.prepare_v3(SQL, 0)
db.prepare_v2(SQL)
})
}
}

impl SimpleCrudTransactionMode {
fn raw_crud_statement(&mut self, db: *mut sqlite::sqlite3) -> Result<&ManagedStmt, ResultCode> {
fn raw_crud_statement(&mut self, db: Database) -> Result<&Statement> {
prepare_lazy(&mut self.stmt, || {
// language=SQLite
db.prepare_v3("INSERT INTO ps_crud(tx_id, data) VALUES (?, ?)", 0)
db.prepare_v2("INSERT INTO ps_crud(tx_id, data) VALUES (?, ?)")
})
}

fn set_updated_rows_statement(
&mut self,
db: *mut sqlite::sqlite3,
) -> Result<&ManagedStmt, ResultCode> {
fn set_updated_rows_statement(&mut self, db: Database) -> Result<&Statement> {
prepare_lazy(&mut self.set_updated_rows, || {
// language=SQLite
db.prepare_v3(
"INSERT OR IGNORE INTO ps_updated_rows(row_type, row_id) VALUES(?, ?)",
0,
)
db.prepare_v2("INSERT OR IGNORE INTO ps_updated_rows(row_type, row_id) VALUES(?, ?)")
})
}

fn record_local_write(&mut self, db: *mut sqlite::sqlite3) -> Result<(), ResultCode> {
fn record_local_write(&mut self, db: Database) -> Result<()> {
if !self.had_writes {
// Also clear the seen/applied high-water marks: checkpoint request ids observed before
// this write can't acknowledge it, and stale values may predate a request counter
// restart. Keeping them around could open the apply gate for a newly allocated target
// id that compares below a stale seen value.
db.exec_safe(formatcp!(
db.exec_safe_str(formatcp!(
"INSERT OR REPLACE INTO ps_kv(key, value) VALUES('{TARGET_CHECKPOINT_REQUEST_ID_KEY}', {MAX_OP_ID});
DELETE FROM ps_kv WHERE key IN ('{LAST_SEEN_CHECKPOINT_REQUEST_ID_KEY}', '{LAST_APPLIED_CHECKPOINT_REQUEST_ID_KEY}')"
))?;
Expand All @@ -268,9 +261,9 @@ DELETE FROM ps_kv WHERE key IN ('{LAST_SEEN_CHECKPOINT_REQUEST_ID_KEY}', '{LAST_

/// A variant of `Option.get_or_insert` that handles insertions returning errors.
fn prepare_lazy(
stmt: &mut Option<ManagedStmt>,
prepare: impl FnOnce() -> Result<ManagedStmt, ResultCode>,
) -> Result<&ManagedStmt, ResultCode> {
stmt: &mut Option<Statement>,
prepare: impl FnOnce() -> Result<Statement>,
) -> Result<&Statement> {
if let None = stmt {
*stmt = Some(prepare()?);
}
Expand Down Expand Up @@ -312,7 +305,7 @@ extern "C" fn connect(
zErrMsg: core::ptr::null_mut(),
},
state: DatabaseState::clone_from(aux),
db,
db: db.into(),
current_tx: None,
is_simple,
}));
Expand Down Expand Up @@ -403,7 +396,10 @@ static MODULE: sqlite::module = sqlite::module {
xIntegrity: None,
};

pub fn register(db: *mut sqlite::sqlite3, state: Rc<DatabaseState>) -> Result<(), ResultCode> {
pub fn register(
db: *mut sqlite::sqlite3,
state: Rc<DatabaseState>,
) -> core::result::Result<(), ResultCode> {
sqlite::convert_rc(sqlite::create_module_v2(
db,
SIMPLE_NAME.as_ptr(),
Expand Down
11 changes: 4 additions & 7 deletions crates/core/src/diff.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,19 @@ use sqlite::ResultCode;

use crate::constants::SUBTYPE_JSON;
use crate::create_sqlite_text_fn;
use crate::error::PowerSyncError;
use crate::error::{PowerSyncError, Result};
use powersync_sqlite_nostd::bindings::SQLITE_RESULT_SUBTYPE;
use serde_json as json;

fn powersync_diff_impl(
ctx: *mut sqlite::context,
args: &[*mut sqlite::value],
) -> Result<String, PowerSyncError> {
fn powersync_diff_impl(ctx: *mut sqlite::context, args: &[*mut sqlite::value]) -> Result<String> {
let data_old = args[0].text();
let data_new = args[1].text();

ctx.result_subtype(SUBTYPE_JSON);
diff_objects(data_old, data_new)
}

pub fn diff_objects(data_old: &str, data_new: &str) -> Result<String, PowerSyncError> {
pub fn diff_objects(data_old: &str, data_new: &str) -> Result<String> {
let v_new: json::Value = json::from_str(data_new).map_err(PowerSyncError::as_argument_error)?;
let v_old: json::Value = json::from_str(data_old).map_err(PowerSyncError::as_argument_error)?;

Expand Down Expand Up @@ -64,7 +61,7 @@ pub fn diff_objects(data_old: &str, data_new: &str) -> Result<String, PowerSyncE

create_sqlite_text_fn!(powersync_diff, powersync_diff_impl, "powersync_diff");

pub fn register(db: *mut sqlite::sqlite3) -> Result<(), ResultCode> {
pub fn register(db: *mut sqlite::sqlite3) -> core::result::Result<(), ResultCode> {
db.create_function_v2(
"powersync_diff",
2,
Expand Down
92 changes: 62 additions & 30 deletions crates/core/src/error.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
use core::{error::Error, ffi::c_int, fmt::Display};
use core::{
error::Error,
ffi::{CStr, c_int},
fmt::Display,
};

use alloc::{
borrow::Cow,
borrow::{Cow, ToOwned},
boxed::Box,
ffi::{CString, NulError},
string::{String, ToString},
};
use num_traits::FromPrimitive;
Expand All @@ -14,6 +19,8 @@ use crate::{
constants::{CORE_PKG_VERSION, MIN_SQLITE_VERSION_NUMBER},
};

pub type Result<T> = core::result::Result<T, PowerSyncError>;

/// A [RawPowerSyncError], but boxed.
///
/// We allocate errors in boxes to avoid large [Result] types (given the large size of the
Expand All @@ -36,12 +43,12 @@ impl PowerSyncError {
pub fn from_sqlite(
db: *mut sqlite3,
code: ResultCode,
context: impl Into<Cow<'static, str>>,
stmt: Option<impl Into<SqlText>>,
) -> Self {
RawPowerSyncError::Sqlite(SqliteError {
code,
errstr: Self::errstr(db),
context: Some(context.into()),
statement: stmt.map(|e| e.into()),
})
.into()
}
Expand Down Expand Up @@ -108,6 +115,14 @@ impl PowerSyncError {
return RawPowerSyncError::DownMigrationDidNotUpdateVersion { current_version }.into();
}

pub fn context(self, context: String) -> Self {
RawPowerSyncError::Context {
inner: self,
context,
}
.into()
}

/// 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) {
Expand All @@ -132,6 +147,8 @@ impl PowerSyncError {
| SqliteVersionMismatch { .. } => ResultCode::ABORT,
LocalDataError { .. } => ResultCode::CORRUPT,
Internal { .. } => ResultCode::INTERNAL,
CString { .. } => ResultCode::FORMAT,
Context { inner, context: _ } => inner.sqlite_error_code(),
}
}

Expand All @@ -149,7 +166,7 @@ impl PowerSyncError {
}
}

pub fn check_sqlite3_version() -> Result<(), PowerSyncError> {
pub fn check_sqlite3_version() -> Result<()> {
let actual_version = sqlite::libversion_number();

if actual_version < MIN_SQLITE_VERSION_NUMBER {
Expand Down Expand Up @@ -180,14 +197,9 @@ impl From<RawPowerSyncError> for PowerSyncError {
}
}

impl From<ResultCode> for PowerSyncError {
fn from(value: ResultCode) -> Self {
return RawPowerSyncError::Sqlite(SqliteError {
code: value,
errstr: None,
context: None,
})
.into();
impl From<NulError> for PowerSyncError {
fn from(value: NulError) -> Self {
RawPowerSyncError::CString { inner: value }.into()
}
}

Expand Down Expand Up @@ -250,19 +262,47 @@ pub enum RawPowerSyncError {
},
#[error("This function may only be called in transactions.")]
MustBeCalledInTransaction,
#[error("Allocating c string: {inner}")]
CString {
#[from]
inner: NulError,
},
#[error("{inner} (context: {context})")]
Context {
inner: PowerSyncError,
context: String,
},
}

#[derive(Debug)]
pub struct SqliteError {
code: ResultCode,
errstr: Option<String>,
context: Option<Cow<'static, str>>,
statement: Option<SqlText>,
}

#[derive(Debug)]
pub enum SqlText {
Rust(String),
C(CString),
}

impl From<&str> for SqlText {
fn from(value: &str) -> Self {
Self::Rust(value.to_string())
}
}

impl From<&CStr> for SqlText {
fn from(value: &CStr) -> Self {
Self::C(value.to_owned())
}
}

impl Display for SqliteError {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
if let Some(context) = &self.context {
write!(f, "{}: ", context)?;
if let Some(stmt) = &self.statement {
write!(f, "statement {}: ", stmt)?;
}

write!(f, "internal SQLite call returned {}", self.code)?;
Expand All @@ -274,20 +314,12 @@ impl Display for SqliteError {
}
}

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()
})
impl Display for SqlText {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
match self {
SqlText::Rust(s) => f.write_str(s),
SqlText::C(cstring) => f.write_str(&cstring.to_string_lossy()),
}
}
}

Expand Down
Loading
Loading