From f0cb428c525a13ee0a868616b82065bcbb42cb6d Mon Sep 17 00:00:00 2001 From: Pekka Enberg Date: Tue, 9 Apr 2024 20:31:12 +0300 Subject: [PATCH] Throw proper error when querying a closed database --- index.js | 15 ++++++++--- integration-tests/tests/async.test.js | 22 ++++++++++++++++ integration-tests/tests/sync.test.js | 22 ++++++++++++++++ promise.js | 11 ++++++-- src/database.rs | 38 ++++++++++++++++++++++----- src/errors.rs | 15 ++++++++++- 6 files changed, 109 insertions(+), 14 deletions(-) diff --git a/index.js b/index.js index fd723a7..6cbf2e2 100644 --- 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 index 5b5a79d..d7ff90a 100644 --- 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 index e53e050..b36f12a 100644 --- 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 index 3b525bd..81db5e0 100644 --- 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 index ba02262..15b4427 100644 --- 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> = cx.this()?; let sql = cx.argument::(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> = cx.this()?; let sql = cx.argument::(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> { + fn get_conn(&self, cx: &mut FunctionContext) -> Option>> { 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 index 28ba7a7..8088cdf 100644 --- 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 { + 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 { 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)? } _ => {