sqlite: handle exception thrown in conflict handler

nodejs · Dec 24, 2024 · a06a068 · a06a068
1 parent e6001b3
commit a06a068
Show file tree

Hide file tree

Showing 3 changed files with 25 additions and 2 deletions.
diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md
@@ -251,6 +251,8 @@ added:
       `SQLITE_CHANGESET_DATA` or `SQLITE_CHANGESET_CONFLICT` conflicts).
     * `SQLITE_CHANGESET_ABORT`: Abort on conflict and roll back the database.
 
+    When an error is thrown in the conflict handler, applying the changeset is aborted and the database is rolled back.
+
     **Default**: A function that returns `SQLITE_CHANGESET_ABORT`.
 * Returns: {boolean} Whether the changeset was applied succesfully without being aborted.
 

diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
@@ -9,6 +9,7 @@
 #include "node_mem-inl.h"
 #include "sqlite3.h"
 #include "util-inl.h"
+#include "v8-exception.h"
 
 #include <cinttypes>
 
@@ -42,6 +43,7 @@ using v8::Number;
 using v8::Object;
 using v8::SideEffectType;
 using v8::String;
+using v8::TryCatch;
 using v8::Uint8Array;
 using v8::Value;
 
@@ -795,9 +797,14 @@ void DatabaseSync::ApplyChangeset(const FunctionCallbackInfo<Value>& args) {
       Local<Function> conflictFunc = conflictValue.As<Function>();
       conflictCallback = [env, conflictFunc](int conflictType) -> int {
         Local<Value> argv[] = {Integer::New(env->isolate(), conflictType)};
+        TryCatch try_catch(env->isolate());
         Local<Value> result =
             conflictFunc->Call(env->context(), Null(env->isolate()), 1, argv)
-                .ToLocalChecked();
+                .FromMaybe(Local<Value>());
+        if (try_catch.HasCaught()) {
+          try_catch.ReThrow();
+          return SQLITE_CHANGESET_ABORT;
+        }
         return result->Int32Value(env->context()).FromJust();
       };
     }

diff --git a/test/parallel/test-sqlite-session.js b/test/parallel/test-sqlite-session.js
@@ -323,7 +323,7 @@ suite('conflict resolution', () => {
     deepStrictEqual(t)(database2.prepare('SELECT key, value from data').all(), [{ key: 2, value: 'hello' }]);
   });
 
-  test("conflict resolution handler returns invalid value", (t) => {
+  test('conflict resolution handler returns invalid value', (t) => {
     const { database2, changeset } = prepareConflict();
     t.assert.throws(() => {
       database2.applyChangeset(changeset, {
@@ -336,6 +336,20 @@ suite('conflict resolution', () => {
       message: 'bad parameter or other API misuse'
     });
   });
+
+  test('conflict resolution handler throws', (t) => {
+    const { database2, changeset } = prepareConflict();
+    t.assert.throws(() => {
+      database2.applyChangeset(changeset, {
+        onConflict: () => {
+          throw new Error('some error');
+        }
+      });
+    }, {
+      name: 'Error',
+      message: 'some error'
+    });
+  });
 });
 
 test('database.createSession() - filter changes', (t) => {