Repository: denoland/deno
Pull request: fix(ext/node): Add SQLite error details (#28289)

Improves SQLite error reporting by including  `err_code`, `err_str`, matching Node.js behavior.

Fixes #28289


Changed files:
filename: ext/node/ops/sqlite/database.rs
patch: @@ -23,7 +23,7 @@ use super::session::SessionOptions;
 use super::Session;
 use super::SqliteError;
 use super::StatementSync;
-
+use crate::ops::sqlite::SqliteResultExt;
 const SQLITE_DBCONFIG_ENABLE_LOAD_EXTENSION: i32 = 1005;
 const SQLITE_DBCONFIG_ENABLE_ATTACH_WRITE: i32 = 1021;
 
@@ -173,7 +173,8 @@ impl DatabaseSync {
       let db = open_db(state, options.read_only, &location)?;
 
       if options.enable_foreign_key_constraints {
-        db.execute("PRAGMA foreign_keys = ON", [])?;
+        db.execute("PRAGMA foreign_keys = ON", [])
+          .with_enhanced_errors(&db)?;
       }
       Some(db)
     } else {
@@ -200,7 +201,8 @@ impl DatabaseSync {
 
     let db = open_db(state, self.options.read_only, &self.location)?;
     if self.options.enable_foreign_key_constraints {
-      db.execute("PRAGMA foreign_keys = ON", [])?;
+      db.execute("PRAGMA foreign_keys = ON", [])
+        .with_enhanced_errors(&db)?;
     }
 
     *self.conn.borrow_mut() = Some(db);
@@ -229,7 +231,7 @@ impl DatabaseSync {
     let db = self.conn.borrow();
     let db = db.as_ref().ok_or(SqliteError::InUse)?;
 
-    db.execute_batch(sql)?;
+    db.execute_batch(sql).with_enhanced_errors(db)?;
 
     Ok(())
   }

filename: ext/node/ops/sqlite/mod.rs
patch: @@ -5,6 +5,7 @@ mod session;
 mod statement;
 
 pub use database::DatabaseSync;
+use rusqlite::ffi as libsqlite3_sys;
 pub use session::Session;
 pub use statement::StatementSync;
 
@@ -65,3 +66,104 @@ pub enum SqliteError {
   #[error("Invalid callback: {0}")]
   InvalidCallback(&'static str),
 }
+
+pub trait SqliteResultExt<T> {
+  fn with_enhanced_errors(
+    self,
+    db: &rusqlite::Connection,
+  ) -> Result<T, SqliteError>;
+}
+
+impl<T> SqliteResultExt<T> for Result<T, rusqlite::Error> {
+  fn with_enhanced_errors(
+    self,
+    db: &rusqlite::Connection,
+  ) -> Result<T, SqliteError> {
+    match self {
+      Ok(value) => Ok(value),
+      Err(error) => {
+        // SAFETY: lifetime of the connection is guaranteed by the rusqlite API.
+        let handle = unsafe { db.handle() };
+        // SAFETY: error conversion does not perform additional dereferencing beyond what is documented.
+        Err(unsafe { SqliteError::from_rusqlite_with_details(error, handle) })
+      }
+    }
+  }
+}
+
+impl SqliteError {
+  pub const ERROR_CODE_GENERIC: i32 = 1;
+
+  pub const ERROR_STR_UNKNOWN: &str = "unknown error";
+
+  pub fn create_enhanced_error<T>(
+    extended_code: i32,
+    message: &str,
+    db_handle: Option<*mut libsqlite3_sys::sqlite3>,
+  ) -> Result<T, Self> {
+    let rusqlite_error = rusqlite::Error::SqliteFailure(
+      rusqlite::ffi::Error {
+        code: rusqlite::ErrorCode::Unknown,
+        extended_code,
+      },
+      Some(message.to_string()),
+    );
+
+    let handle = db_handle.unwrap_or(std::ptr::null_mut());
+    // SAFETY: error conversion does not perform additional dereferencing beyond what is documented.
+    Err(unsafe {
+      SqliteError::from_rusqlite_with_details(rusqlite_error, handle)
+    })
+  }
+
+  /// Creates a `SqliteError` from a rusqlite error and a raw SQLite handle.
+  ///
+  /// # Safety
+  ///
+  /// Caller must ensure `handle` is non-null and points to a valid, initialized sqlite3 instance.
+  pub unsafe fn from_rusqlite_with_details(
+    error: rusqlite::Error,
+    handle: *mut libsqlite3_sys::sqlite3,
+  ) -> Self {
+    let message = error.to_string();
+
+    let err_code = match &error {
+      rusqlite::Error::SqliteFailure(ffi_error, _) => ffi_error.code as i32,
+      _ => {
+        if !handle.is_null() {
+          // SAFETY: We've verified that handle is not null in the previous condition.
+          unsafe { libsqlite3_sys::sqlite3_errcode(handle) }
+        } else {
+          Self::ERROR_CODE_GENERIC
+        }
+      }
+    };
+
+    // SAFETY: We're using sqlite3_errstr which returns a static string.
+    let err_str = unsafe {
+      let ptr = libsqlite3_sys::sqlite3_errstr(err_code);
+      if !ptr.is_null() {
+        std::ffi::CStr::from_ptr(ptr).to_string_lossy().into_owned()
+      } else {
+        Self::ERROR_STR_UNKNOWN.to_string()
+      }
+    };
+
+    let encoded_message = format!(
+      "{}\n  {{\n  code: 'ERR_SQLITE_ERROR',\n  errcode: {},\n  errstr: '{}'\n}}",
+      message,
+      err_code,
+      err_str
+    );
+
+    let custom_error = rusqlite::Error::SqliteFailure(
+      rusqlite::ffi::Error {
+        code: rusqlite::ErrorCode::Unknown,
+        extended_code: err_code,
+      },
+      Some(encoded_message),
+    );
+
+    SqliteError::SqliteError(custom_error)
+  }
+}

filename: ext/node/ops/sqlite/session.rs
patch: @@ -38,7 +38,11 @@ impl Drop for Session {
 impl Session {
   fn delete(&self) -> Result<(), SqliteError> {
     if self.freed.get() {
-      return Err(SqliteError::SessionClosed);
+      return SqliteError::create_enhanced_error(
+        ffi::SQLITE_MISUSE,
+        &SqliteError::SessionClosed.to_string(),
+        None,
+      );
     }
 
     self.freed.set(true);
@@ -58,7 +62,11 @@ impl Session {
   #[fast]
   fn close(&self) -> Result<(), SqliteError> {
     if self.db.borrow().is_none() {
-      return Err(SqliteError::AlreadyClosed);
+      return SqliteError::create_enhanced_error(
+        ffi::SQLITE_MISUSE,
+        &SqliteError::AlreadyClosed.to_string(),
+        None,
+      );
     }
 
     self.delete()
@@ -71,10 +79,18 @@ impl Session {
   #[buffer]
   fn changeset(&self) -> Result<Box<[u8]>, SqliteError> {
     if self.db.borrow().is_none() {
-      return Err(SqliteError::AlreadyClosed);
+      return SqliteError::create_enhanced_error(
+        ffi::SQLITE_MISUSE,
+        &SqliteError::AlreadyClosed.to_string(),
+        None,
+      );
     }
     if self.freed.get() {
-      return Err(SqliteError::SessionClosed);
+      return SqliteError::create_enhanced_error(
+        ffi::SQLITE_MISUSE,
+        &SqliteError::SessionClosed.to_string(),
+        None,
+      );
     }
 
     session_buffer_op(self.inner, ffi::sqlite3session_changeset)
@@ -86,10 +102,18 @@ impl Session {
   #[buffer]
   fn patchset(&self) -> Result<Box<[u8]>, SqliteError> {
     if self.db.borrow().is_none() {
-      return Err(SqliteError::AlreadyClosed);
+      return SqliteError::create_enhanced_error(
+        ffi::SQLITE_MISUSE,
+        &SqliteError::AlreadyClosed.to_string(),
+        None,
+      );
     }
     if self.freed.get() {
-      return Err(SqliteError::SessionClosed);
+      return SqliteError::create_enhanced_error(
+        ffi::SQLITE_MISUSE,
+        &SqliteError::SessionClosed.to_string(),
+        None,
+      );
     }
 
     session_buffer_op(self.inner, ffi::sqlite3session_patchset)
@@ -111,7 +135,11 @@ fn session_buffer_op(
   // by sqlite3 and will be freed later.
   let r = unsafe { f(s, &mut n_buffer, &mut p_buffer) };
   if r != ffi::SQLITE_OK {
-    return Err(SqliteError::SessionChangesetFailed);
+    return SqliteError::create_enhanced_error(
+      r,
+      &SqliteError::SessionChangesetFailed.to_string(),
+      None,
+    );
   }
 
   if n_buffer == 0 {

filename: ext/node/ops/sqlite/statement.rs
patch: @@ -142,7 +142,17 @@ impl StatementSync {
           } else if value.abs() <= MAX_SAFE_JS_INTEGER {
             v8::Number::new(scope, value as f64).into()
           } else {
-            return Err(SqliteError::NumberTooLarge(index, value));
+            let db = self.db.borrow();
+            let db = db.as_ref().ok_or(SqliteError::InUse)?;
+            let handle = db.handle();
+
+            return SqliteError::create_enhanced_error::<
+              v8::Local<'a, v8::Value>,
+            >(
+              ffi::SQLITE_TOOBIG,
+              &SqliteError::NumberTooLarge(index, value).to_string(),
+              Some(handle),
+            );
           }
         }
         ffi::SQLITE_FLOAT => {
@@ -265,16 +275,33 @@ impl StatementSync {
       let value: v8::Local<v8::BigInt> = value.try_into().unwrap();
       let (as_int, lossless) = value.i64_value();
       if !lossless {
-        return Err(SqliteError::FailedBind(
-          "BigInt value is too large to bind",
-        ));
+        let db = self.db.borrow();
+        let db = db.as_ref().ok_or(SqliteError::InUse)?;
+        // SAFETY: lifetime of the connection is guaranteed by the rusqlite API.
+        let handle = unsafe { db.handle() };
+
+        return SqliteError::create_enhanced_error(
+          ffi::SQLITE_TOOBIG,
+          &SqliteError::FailedBind("BigInt value is too large to bind")
+            .to_string(),
+          Some(handle),
+        );
       }
 
       // SAFETY: `self.inner` is a valid pointer to a sqlite3_stmt
       // as it lives as long as the StatementSync instance.
       unsafe { ffi::sqlite3_bind_int64(raw, index, as_int) }
     } else {
-      return Err(SqliteError::FailedBind("Unsupported type"));
+      let db = self.db.borrow();
+      let db = db.as_ref().ok_or(SqliteError::InUse)?;
+      // SAFETY: lifetime of the connection is guaranteed by the rusqlite API.
+      let handle = unsafe { db.handle() };
+
+      return SqliteError::create_enhanced_error(
+        ffi::SQLITE_MISMATCH,
+        &SqliteError::FailedBind("Unsupported type").to_string(),
+        Some(handle),
+      );
     };
 
     self.check_error_code(r)
@@ -284,6 +311,8 @@ impl StatementSync {
     if r != ffi::SQLITE_OK {
       let db = self.db.borrow();
       let db = db.as_ref().ok_or(SqliteError::InUse)?;
+      // SAFETY: lifetime of the connection is guaranteed by the rusqlite API.
+      let handle = unsafe { db.handle() };
 
       // SAFETY: lifetime of the connection is guaranteed by reference
       // counting.
@@ -294,7 +323,7 @@ impl StatementSync {
         let err_str = unsafe { std::ffi::CStr::from_ptr(err_str) }
           .to_string_lossy()
           .into_owned();
-        return Err(SqliteError::SqliteSysError(err_str));
+        return SqliteError::create_enhanced_error(r, &err_str, Some(handle));
       }
     }
 
@@ -337,7 +366,17 @@ impl StatementSync {
 
             let e = bare_named_params.insert(bare_name, i);
             if e.is_some() {
-              return Err(SqliteError::FailedBind("Duplicate named parameter"));
+              let db = self.db.borrow();
+              let db = db.as_ref().ok_or(SqliteError::InUse)?;
+              // SAFETY: lifetime of the connection is guaranteed by the rusqlite API.
+              let handle = unsafe { db.handle() };
+
+              return SqliteError::create_enhanced_error(
+                ffi::SQLITE_ERROR,
+                &SqliteError::FailedBind("Duplicate named parameter")
+                  .to_string(),
+                Some(handle),
+              );
             }
           }
         }
@@ -359,7 +398,17 @@ impl StatementSync {
             }
 
             if r == 0 {
-              return Err(SqliteError::FailedBind("Named parameter not found"));
+              let db = self.db.borrow();
+              let db = db.as_ref().ok_or(SqliteError::InUse)?;
+              // SAFETY: lifetime of the connection is guaranteed by the rusqlite API.
+              let handle = unsafe { db.handle() };
+
+              return SqliteError::create_enhanced_error(
+                ffi::SQLITE_RANGE,
+                &SqliteError::FailedBind("Named parameter not found")
+                  .to_string(),
+                Some(handle),
+              );
             }
           }
 
@@ -653,7 +702,15 @@ impl StatementSync {
     unsafe {
       let raw = ffi::sqlite3_expanded_sql(self.inner);
       if raw.is_null() {
-        return Err(SqliteError::InvalidExpandedSql);
+        let db = self.db.borrow();
+        let db = db.as_ref().ok_or(SqliteError::InUse)?;
+        let handle = db.handle();
+
+        return SqliteError::create_enhanced_error(
+          ffi::SQLITE_ERROR,
+          &SqliteError::InvalidExpandedSql.to_string(),
+          Some(handle),
+        );
       }
       let sql = std::ffi::CStr::from_ptr(raw as _)
         .to_string_lossy()