Throw proper error when querying a closed database

tursodatabase · Apr 10, 2024 · f0cb428 · f0cb428
1 parent 6b19d4b
commit f0cb428
Show file tree

Hide file tree

Showing 6 changed files with 109 additions and 14 deletions.
diff --git a/index.js b/index.js
@@ -48,6 +48,13 @@ const {
 
 const SqliteError = require("./sqlite-error");
 
+function convertError(err) {
+  if (err.libsqlError) {
+    return new SqliteError(err.message, err.code, err.rawCode);
+  }
+  return err;
+}
+
 /**
  * Database represents a connection that can prepare and execute SQL statements.
  */
@@ -106,7 +113,7 @@ class Database {
       const stmt = databasePrepareSync.call(this.db, sql);
       return new Statement(stmt);  
     } catch (err) {
-      throw new SqliteError(err.message, err.code, err.rawCode);
+      throw convertError(err);
     }
   }
 
@@ -207,7 +214,7 @@ class Database {
     try {
       databaseExecSync.call(this.db, sql);
     } catch (err) {
-      throw new SqliteError(err.message, err.code, err.rawCode);
+      throw convertError(err);
     }
   }
 
@@ -263,9 +270,9 @@ class Statement {
         return statementRun.call(this.stmt, bindParameters[0]);
       } else {
         return statementRun.call(this.stmt, bindParameters.flat());
-      }
+      }  
     } catch (err) {
-      throw new SqliteError(err.message, err.code, err.rawCode);
+      throw convertError(err);
     }
   }
 

diff --git a/integration-tests/tests/async.test.js b/integration-tests/tests/async.test.js
@@ -258,6 +258,28 @@ test.serial("errors", async (t) => {
   t.is(noTableError.rawCode, 1)
 });
 
+test.serial("Database.prepare() after close()", async (t) => {
+  const db = t.context.db;
+  await db.close();
+  await t.throwsAsync(async () => {    
+    await db.prepare("SELECT 1");
+  }, { 
+    instanceOf: TypeError,
+    message: "The database connection is not open"
+  });
+});
+
+test.serial("Database.exec() after close()", async (t) => {
+  const db = t.context.db;
+  await db.close();
+  await t.throwsAsync(async () => {    
+    await db.exec("SELECT 1");
+  }, { 
+    instanceOf: TypeError,
+    message: "The database connection is not open"
+  });
+});
+
 const connect = async (path_opt) => {
   const path = path_opt ?? "hello.db";
   const provider = process.env.PROVIDER;

diff --git a/integration-tests/tests/sync.test.js b/integration-tests/tests/sync.test.js
@@ -275,6 +275,28 @@ test.serial("errors", async (t) => {
   }
 });
 
+test.serial("Database.prepare() after close()", async (t) => {
+  const db = t.context.db;
+  db.close();
+  t.throws(() => {    
+    db.prepare("SELECT 1");
+  }, { 
+    instanceOf: TypeError,
+    message: "The database connection is not open"
+  });
+});
+
+test.serial("Database.exec() after close()", async (t) => {
+  const db = t.context.db;
+  db.close();
+  t.throws(() => {    
+    db.exec("SELECT 1");
+  }, { 
+    instanceOf: TypeError,
+    message: "The database connection is not open"
+  });
+});
+
 const connect = async (path_opt) => {
   const path = path_opt ?? "hello.db";
   const provider = process.env.PROVIDER;

diff --git a/promise.js b/promise.js
@@ -9,6 +9,13 @@ if (0) {
 
 const SqliteError = require("./sqlite-error");
 
+function convertError(err) {
+  if (err.libsqlError) {
+    return new SqliteError(err.message, err.code, err.rawCode);
+  }
+  return err;
+}
+
 function requireNative() {
   if (process.env.LIBSQL_JS_DEV) {
     return load(__dirname)
@@ -104,7 +111,7 @@ class Database {
     return databasePrepareAsync.call(this.db, sql).then((stmt) => {
       return new Statement(stmt);
     }).catch((err) => {
-      throw new SqliteError(err.message, err.code, err.rawCode);
+      throw convertError(err);
     });
   }
 
@@ -203,7 +210,7 @@ class Database {
    */
   exec(sql) {
     return databaseExecAsync.call(this.db, sql).catch((err) => {
-      throw new SqliteError(err.message, err.code, err.rawCode);
+      throw convertError(err);
     });
   }
 

diff --git a/src/database.rs b/src/database.rs
@@ -6,7 +6,7 @@ use std::time::Duration;
 use tokio::sync::Mutex;
 use tracing::trace;
 
-use crate::errors::throw_libsql_error;
+use crate::errors::{throw_database_closed_error, throw_libsql_error};
 use crate::runtime;
 use crate::Statement;
 
@@ -170,7 +170,10 @@ impl Database {
         let db: Handle<'_, JsBox<Database>> = cx.this()?;
         let sql = cx.argument::<JsString>(0)?.value(&mut cx);
         trace!("Executing SQL statement (sync): {}", sql);
-        let conn = db.get_conn();
+        let conn = match db.get_conn(&mut cx) {
+            Some(conn) => conn,
+            None => throw_database_closed_error(&mut cx)?,
+        };
         let rt = runtime(&mut cx)?;
         let result = rt.block_on(async { conn.lock().await.execute_batch(&sql).await });
         result.or_else(|err| throw_libsql_error(&mut cx, err))?;
@@ -183,7 +186,16 @@ impl Database {
         trace!("Executing SQL statement (async): {}", sql);
         let (deferred, promise) = cx.promise();
         let channel = cx.channel();
-        let conn = db.get_conn();
+        let conn = match db.get_conn(&mut cx) {
+            Some(conn) => conn,
+            None => {
+                deferred.settle_with(&channel, |mut cx| {
+                    throw_database_closed_error(&mut cx)?;
+                    Ok(cx.undefined())
+                });
+                return Ok(promise);
+            }
+        };
         let rt = runtime(&mut cx)?;
         rt.spawn(async move {
             match conn.lock().await.execute_batch(&sql).await {
@@ -205,7 +217,10 @@ impl Database {
         let db: Handle<'_, JsBox<Database>> = cx.this()?;
         let sql = cx.argument::<JsString>(0)?.value(&mut cx);
         trace!("Preparing SQL statement (sync): {}", sql);
-        let conn = db.get_conn();
+        let conn = match db.get_conn(&mut cx) {
+            Some(conn) => conn,
+            None => throw_database_closed_error(&mut cx)?,
+        };
         let rt = runtime(&mut cx)?;
         let result = rt.block_on(async { conn.lock().await.prepare(&sql).await });
         let stmt = result.or_else(|err| throw_libsql_error(&mut cx, err))?;
@@ -227,7 +242,16 @@ impl Database {
         let channel = cx.channel();
         let safe_ints = *db.default_safe_integers.borrow();
         let rt = runtime(&mut cx)?;
-        let conn = db.get_conn();
+        let conn = match db.get_conn(&mut cx) {
+            Some(conn) => conn,
+            None => {
+                deferred.settle_with(&channel, |mut cx| {
+                    throw_database_closed_error(&mut cx)?;
+                    Ok(cx.undefined())
+                });
+                return Ok(promise);
+            }
+        };
         rt.spawn(async move {
             match conn.lock().await.prepare(&sql).await {
                 Ok(stmt) => {
@@ -263,9 +287,9 @@ impl Database {
         self.default_safe_integers.replace(toggle);
     }
 
-    fn get_conn(&self) -> Arc<Mutex<libsql::Connection>> {
+    fn get_conn(&self, cx: &mut FunctionContext) -> Option<Arc<Mutex<libsql::Connection>>> {
         let conn = self.conn.borrow();
-        conn.as_ref().unwrap().clone()
+        conn.as_ref().map(|conn| conn.clone())
     }
 }
 

diff --git a/src/errors.rs b/src/errors.rs
@@ -1,4 +1,15 @@
-use neon::{context::Context, object::Object, result::NeonResult, types::JsError};
+use neon::{
+    context::Context,
+    handle::Handle,
+    object::Object,
+    result::NeonResult,
+    types::{JsError, JsFunction, JsObject},
+};
+
+pub fn throw_database_closed_error<'a, C: Context<'a>, T>(cx: &mut C) -> NeonResult<T> {
+    let err = JsError::type_error(cx, "The database connection is not open")?;
+    cx.throw(err)
+}
 
 pub fn throw_libsql_error<'a, C: Context<'a>, T>(cx: &mut C, err: libsql::Error) -> NeonResult<T> {
     match err {
@@ -9,6 +20,8 @@ pub fn throw_libsql_error<'a, C: Context<'a>, T>(cx: &mut C, err: libsql::Error)
             err.set(cx, "rawCode", code_num).unwrap();
             let code = cx.string(convert_sqlite_code(code));
             err.set(cx, "code", code).unwrap();
+            let val = cx.boolean(true);
+            err.set(cx, "libsqlError", val).unwrap();
             cx.throw(err)?
         }
         _ => {