From 666034fd8adac80091ed438cbb6e8cdf59d0b870 Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Sun, 12 Jan 2025 21:56:43 -0300
Subject: [PATCH 01/11] sqlite, test: expose sqlite online backup api

---
 src/env_properties.h                 |   3 +
 src/node_sqlite.cc                   | 323 +++++++++++++++++++++++++++
 src/node_sqlite.h                    |   5 +
 test/parallel/test-sqlite-backup.mjs | 130 +++++++++++
 4 files changed, 461 insertions(+)
 create mode 100644 test/parallel/test-sqlite-backup.mjs

diff --git a/src/env_properties.h b/src/env_properties.h
index d9e18cbc4515ac..55116d463bb844 100644
--- a/src/env_properties.h
+++ b/src/env_properties.h
@@ -71,6 +71,9 @@
   V(__dirname_string, "__dirname")                                             \
   V(ack_string, "ack")                                                         \
   V(address_string, "address")                                                 \
+  V(source_db_string, "sourceDb")                                              \
+  V(target_db_string, "targetDb")                                              \
+  V(progress_string, "progress")                                               \
   V(aliases_string, "aliases")                                                 \
   V(alpn_callback_string, "ALPNCallback")                                      \
   V(args_string, "args")                                                       \
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
index 0b8f7ef2a21763..c1f4a8e09e141f 100644
--- a/src/node_sqlite.cc
+++ b/src/node_sqlite.cc
@@ -8,6 +8,7 @@
 #include "node_errors.h"
 #include "node_mem-inl.h"
 #include "sqlite3.h"
+#include "threadpoolwork-inl.h"
 #include "util-inl.h"
 
 #include <cinttypes>
@@ -29,6 +30,7 @@ using v8::FunctionCallback;
 using v8::FunctionCallbackInfo;
 using v8::FunctionTemplate;
 using v8::Global;
+using v8::HandleScope;
 using v8::Int32;
 using v8::Integer;
 using v8::Isolate;
@@ -40,6 +42,7 @@ using v8::NewStringType;
 using v8::Null;
 using v8::Number;
 using v8::Object;
+using v8::Promise;
 using v8::SideEffectType;
 using v8::String;
 using v8::TryCatch;
@@ -81,6 +84,23 @@ inline MaybeLocal<Object> CreateSQLiteError(Isolate* isolate,
   return e;
 }
 
+inline MaybeLocal<Object> CreateSQLiteError(Isolate* isolate, int errcode) {
+  const char* errstr = sqlite3_errstr(errcode);
+  Local<String> js_errmsg;
+  Local<Object> e;
+  Environment* env = Environment::GetCurrent(isolate);
+  if (!String::NewFromUtf8(isolate, errstr).ToLocal(&js_errmsg) ||
+      !CreateSQLiteError(isolate, errstr).ToLocal(&e) ||
+      e->Set(env->context(),
+             env->errcode_string(),
+             Integer::New(isolate, errcode))
+          .IsNothing() ||
+      e->Set(env->context(), env->errstr_string(), js_errmsg).IsNothing()) {
+    return MaybeLocal<Object>();
+  }
+  return e;
+}
+
 inline MaybeLocal<Object> CreateSQLiteError(Isolate* isolate, sqlite3* db) {
   int errcode = sqlite3_extended_errcode(db);
   const char* errstr = sqlite3_errstr(errcode);
@@ -128,6 +148,180 @@ inline void THROW_ERR_SQLITE_ERROR(Isolate* isolate, int errcode) {
   isolate->ThrowException(error);
 }
 
+class BackupJob : public ThreadPoolWork {
+ public:
+  explicit BackupJob(Environment* env,
+                     DatabaseSync* source,
+                     Local<Promise::Resolver> resolver,
+                     std::string source_db,
+                     std::string destination_name,
+                     std::string dest_db,
+                     int pages,
+                     Local<Function> progressFunc)
+      : ThreadPoolWork(env, "node_sqlite3.BackupJob"),
+        env_(env),
+        source_(source),
+        pages_(pages),
+        source_db_(source_db),
+        destination_name_(destination_name),
+        dest_db_(dest_db) {
+    resolver_.Reset(env->isolate(), resolver);
+    progressFunc_.Reset(env->isolate(), progressFunc);
+  }
+
+  void ScheduleBackup() {
+    Isolate* isolate = env()->isolate();
+    HandleScope handle_scope(isolate);
+
+    backup_status_ = sqlite3_open(destination_name_.c_str(), &pDest_);
+
+    Local<Promise::Resolver> resolver =
+        Local<Promise::Resolver>::New(env()->isolate(), resolver_);
+
+    Local<Object> e = Local<Object>();
+
+    if (backup_status_ != SQLITE_OK) {
+      if (!CreateSQLiteError(isolate, pDest_).ToLocal(&e)) {
+        Finalize();
+
+        return;
+      }
+
+      Finalize();
+
+      resolver->Reject(env()->context(), e).ToChecked();
+
+      return;
+    }
+
+    pBackup_ = sqlite3_backup_init(
+        pDest_, dest_db_.c_str(), source_->Connection(), source_db_.c_str());
+
+    if (pBackup_ == nullptr) {
+      if (!CreateSQLiteError(isolate, pDest_).ToLocal(&e)) {
+        Finalize();
+
+        return;
+      }
+
+      Finalize();
+
+      resolver->Reject(env()->context(), e).ToChecked();
+
+      return;
+    }
+
+    this->ScheduleWork();
+  }
+
+  void DoThreadPoolWork() override {
+    backup_status_ = sqlite3_backup_step(pBackup_, pages_);
+  }
+
+  void AfterThreadPoolWork(int status) override {
+    HandleScope handle_scope(env()->isolate());
+    Local<Promise::Resolver> resolver =
+        Local<Promise::Resolver>::New(env()->isolate(), resolver_);
+
+    if (!(backup_status_ == SQLITE_OK || backup_status_ == SQLITE_DONE ||
+          backup_status_ == SQLITE_BUSY || backup_status_ == SQLITE_LOCKED)) {
+      Local<Object> e;
+      if (!CreateSQLiteError(env()->isolate(), backup_status_).ToLocal(&e)) {
+        Finalize();
+
+        return;
+      }
+
+      Finalize();
+
+      resolver->Reject(env()->context(), e).ToChecked();
+
+      return;
+    }
+
+    int total_pages = sqlite3_backup_pagecount(pBackup_);
+    int remaining_pages = sqlite3_backup_remaining(pBackup_);
+
+    if (remaining_pages != 0) {
+      Local<Function> fn =
+          Local<Function>::New(env()->isolate(), progressFunc_);
+
+      if (!fn.IsEmpty()) {
+        Local<Value> argv[] = {
+            Integer::New(env()->isolate(), total_pages),
+            Integer::New(env()->isolate(), remaining_pages),
+        };
+
+        TryCatch try_catch(env()->isolate());
+        fn->Call(env()->context(), Null(env()->isolate()), 2, argv)
+            .FromMaybe(Local<Value>());
+
+        if (try_catch.HasCaught()) {
+          Finalize();
+
+          resolver->Reject(env()->context(), try_catch.Exception()).ToChecked();
+
+          return;
+        }
+      }
+
+      // There's still work to do
+      this->ScheduleWork();
+
+      return;
+    }
+
+    Local<String> message =
+        String::NewFromUtf8(
+            env()->isolate(), "Backup completed", NewStringType::kNormal)
+            .ToLocalChecked();
+
+    Local<Object> e =
+        CreateSQLiteError(env()->isolate(), pDest_).ToLocalChecked();
+
+    Finalize();
+
+    if (backup_status_ == SQLITE_OK) {
+      resolver->Resolve(env()->context(), message).ToChecked();
+    } else {
+      resolver->Reject(env()->context(), e).ToChecked();
+    }
+  }
+
+  void Finalize() {
+    Cleanup();
+    source_->RemoveBackup(this);
+  }
+
+  void Cleanup() {
+    if (pBackup_) {
+      sqlite3_backup_finish(pBackup_);
+      pBackup_ = nullptr;
+    }
+
+    if (pDest_) {
+      backup_status_ = sqlite3_errcode(pDest_);
+      sqlite3_close(pDest_);
+      pDest_ = nullptr;
+    }
+  }
+
+ private:
+  Environment* env() const { return env_; }
+
+  Environment* env_;
+  DatabaseSync* source_;
+  Global<Promise::Resolver> resolver_;
+  Global<Function> progressFunc_;
+  sqlite3* pDest_ = nullptr;
+  sqlite3_backup* pBackup_ = nullptr;
+  int pages_;
+  int backup_status_;
+  std::string source_db_;
+  std::string destination_name_;
+  std::string dest_db_;
+};
+
 class UserDefinedFunction {
  public:
   explicit UserDefinedFunction(Environment* env,
@@ -261,6 +455,10 @@ DatabaseSync::DatabaseSync(Environment* env,
   }
 }
 
+void DatabaseSync::RemoveBackup(BackupJob* job) {
+  backups_.erase(job);
+}
+
 void DatabaseSync::DeleteSessions() {
   // all attached sessions need to be deleted before the database is closed
   // https://www.sqlite.org/session/sqlite3session_create.html
@@ -273,6 +471,7 @@ void DatabaseSync::DeleteSessions() {
 DatabaseSync::~DatabaseSync() {
   if (IsOpen()) {
     FinalizeStatements();
+    FinalizeBackups();
     DeleteSessions();
     sqlite3_close_v2(connection_);
     connection_ = nullptr;
@@ -335,6 +534,14 @@ bool DatabaseSync::Open() {
   return true;
 }
 
+void DatabaseSync::FinalizeBackups() {
+  for (auto backup : backups_) {
+    backup->Cleanup();
+  }
+
+  backups_.clear();
+}
+
 void DatabaseSync::FinalizeStatements() {
   for (auto stmt : statements_) {
     stmt->Finalize();
@@ -533,6 +740,121 @@ void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
   CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
 }
 
+// database.backup(destination, { sourceDb, targetDb, rate, progress: (total,
+// remaining) => {} )
+void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
+  Environment* env = Environment::GetCurrent(args);
+
+  if (!args[0]->IsString()) {
+    THROW_ERR_INVALID_ARG_TYPE(
+        env->isolate(), "The \"destination\" argument must be a string.");
+    return;
+  }
+
+  int rate = 100;
+  std::string source_db = "main";
+  std::string dest_db = "main";
+
+  DatabaseSync* db;
+  ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
+
+  THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
+
+  Utf8Value destFilename(env->isolate(), args[0].As<String>());
+  Local<Function> progressFunc = Local<Function>();
+
+  if (args.Length() > 1) {
+    if (!args[1]->IsObject()) {
+      THROW_ERR_INVALID_ARG_TYPE(env->isolate(),
+                                 "The \"options\" argument must be an object.");
+      return;
+    }
+
+    Local<Object> options = args[1].As<Object>();
+
+    Local<Value> rateValue;
+    if (!options->Get(env->context(), env->rate_string()).ToLocal(&rateValue)) {
+      return;
+    }
+
+    if (!rateValue->IsUndefined()) {
+      if (!rateValue->IsInt32()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.rate\" argument must be an integer.");
+        return;
+      }
+
+      rate = rateValue.As<Int32>()->Value();
+    }
+
+    Local<Value> sourceDbValue;
+    if (!options->Get(env->context(), env->source_db_string())
+             .ToLocal(&sourceDbValue)) {
+      return;
+    }
+
+    if (!sourceDbValue->IsUndefined()) {
+      if (!sourceDbValue->IsString()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.sourceDb\" argument must be a string.");
+        return;
+      }
+
+      source_db =
+          Utf8Value(env->isolate(), sourceDbValue.As<String>()).ToString();
+    }
+
+    Local<Value> targetDbValue;
+    if (!options->Get(env->context(), env->target_db_string())
+             .ToLocal(&targetDbValue)) {
+      return;
+    }
+
+    if (!targetDbValue->IsUndefined()) {
+      if (!targetDbValue->IsString()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.targetDb\" argument must be a string.");
+        return;
+      }
+
+      dest_db =
+          Utf8Value(env->isolate(), targetDbValue.As<String>()).ToString();
+    }
+
+    Local<Value> progressValue;
+    if (!options->Get(env->context(), env->progress_string())
+             .ToLocal(&progressValue)) {
+      return;
+    }
+
+    if (!progressValue->IsUndefined()) {
+      if (!progressValue->IsFunction()) {
+        THROW_ERR_INVALID_ARG_TYPE(
+            env->isolate(),
+            "The \"options.progress\" argument must be a function.");
+        return;
+      }
+
+      progressFunc = progressValue.As<Function>();
+    }
+  }
+
+  Local<Promise::Resolver> resolver;
+  if (!Promise::Resolver::New(env->context()).ToLocal(&resolver)) {
+    return;
+  }
+
+  args.GetReturnValue().Set(resolver->GetPromise());
+
+  BackupJob* job = new BackupJob(
+      env, db, resolver, source_db, *destFilename, dest_db, rate, progressFunc);
+  db->backups_.insert(job);
+  job->ScheduleBackup();
+}
+
 void DatabaseSync::CustomFunction(const FunctionCallbackInfo<Value>& args) {
   DatabaseSync* db;
   ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
@@ -1718,6 +2040,7 @@ static void Initialize(Local<Object> target,
   SetProtoMethod(isolate, db_tmpl, "close", DatabaseSync::Close);
   SetProtoMethod(isolate, db_tmpl, "prepare", DatabaseSync::Prepare);
   SetProtoMethod(isolate, db_tmpl, "exec", DatabaseSync::Exec);
+  SetProtoMethod(isolate, db_tmpl, "backup", DatabaseSync::Backup);
   SetProtoMethod(isolate, db_tmpl, "function", DatabaseSync::CustomFunction);
   SetProtoMethod(
       isolate, db_tmpl, "createSession", DatabaseSync::CreateSession);
diff --git a/src/node_sqlite.h b/src/node_sqlite.h
index e78aa39abb3ba5..05fe0bdb143d99 100644
--- a/src/node_sqlite.h
+++ b/src/node_sqlite.h
@@ -43,6 +43,7 @@ class DatabaseOpenConfiguration {
 };
 
 class StatementSync;
+class BackupJob;
 
 class DatabaseSync : public BaseObject {
  public:
@@ -57,6 +58,7 @@ class DatabaseSync : public BaseObject {
   static void Close(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Prepare(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void Exec(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void Backup(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void CustomFunction(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void CreateSession(const v8::FunctionCallbackInfo<v8::Value>& args);
   static void ApplyChangeset(const v8::FunctionCallbackInfo<v8::Value>& args);
@@ -64,6 +66,8 @@ class DatabaseSync : public BaseObject {
       const v8::FunctionCallbackInfo<v8::Value>& args);
   static void LoadExtension(const v8::FunctionCallbackInfo<v8::Value>& args);
   void FinalizeStatements();
+  void RemoveBackup(BackupJob* backup);
+  void FinalizeBackups();
   void UntrackStatement(StatementSync* statement);
   bool IsOpen();
   sqlite3* Connection();
@@ -81,6 +85,7 @@ class DatabaseSync : public BaseObject {
   bool enable_load_extension_;
   sqlite3* connection_;
 
+  std::set<BackupJob*> backups_;
   std::set<sqlite3_session*> sessions_;
   std::unordered_set<StatementSync*> statements_;
 
diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
new file mode 100644
index 00000000000000..0460f2e5a128d5
--- /dev/null
+++ b/test/parallel/test-sqlite-backup.mjs
@@ -0,0 +1,130 @@
+import * as common from '../common/index.mjs';
+import tmpdir from '../common/tmpdir.js';
+import { join } from 'path';
+import { DatabaseSync } from 'node:sqlite';
+import { test } from 'node:test';
+import { writeFileSync } from 'fs';
+
+let cnt = 0;
+
+tmpdir.refresh();
+
+function nextDb() {
+  return join(tmpdir.path, `database-${cnt++}.db`);
+}
+
+function makeSourceDb() {
+  const database = new DatabaseSync(':memory:');
+
+  database.exec(`
+    CREATE TABLE data(
+      key INTEGER PRIMARY KEY,
+      value TEXT
+    ) STRICT
+  `);
+
+  const insert = database.prepare('INSERT INTO data (key, value) VALUES (?, ?)');
+
+  for (let i = 1; i <= 2; i++) {
+    insert.run(i, `value-${i}`);
+  }
+
+  return database;
+}
+
+test('throws exception when trying to start backup from a closed database', async (t) => {
+  t.assert.rejects(async () => {
+    const database = new DatabaseSync(':memory:');
+
+    database.close();
+
+    await database.backup('backup.db');
+  }, common.expectsError({
+    code: 'ERR_INVALID_STATE',
+    message: 'database is not open'
+  }));
+});
+
+test('database backup', async (t) => {
+  const progressFn = t.mock.fn();
+  const database = makeSourceDb();
+  const destDb = nextDb();
+
+  await database.backup(destDb, {
+    rate: 1,
+    progress: progressFn,
+  });
+
+  const backup = new DatabaseSync(destDb);
+  const rows = backup.prepare('SELECT * FROM data').all();
+
+  // The source database has two pages - using the default page size -,
+  // so the progress function should be called once (the last call is not made since
+  // the promise resolves)
+  t.assert.strictEqual(progressFn.mock.calls.length, 1);
+  t.assert.deepStrictEqual(rows, [
+    { __proto__: null, key: 1, value: 'value-1' },
+    { __proto__: null, key: 2, value: 'value-2' },
+  ]);
+});
+
+test('database backup fails when dest file is not writable', (t) => {
+  const readonlyDestDb = nextDb();
+  writeFileSync(readonlyDestDb, '', { mode: 0o444 });
+
+  const database = makeSourceDb();
+
+  t.assert.rejects(async () => {
+    await database.backup(readonlyDestDb);
+  }, common.expectsError({
+    code: 'ERR_SQLITE_ERROR',
+    message: 'attempt to write a readonly database'
+  }));
+});
+
+test('backup fails when progress function throws', async (t) => {
+  const database = makeSourceDb();
+  const destDb = nextDb();
+
+  const progressFn = t.mock.fn(() => {
+    throw new Error('progress error');
+  });
+
+  t.assert.rejects(async () => {
+    await database.backup(destDb, {
+      rate: 1,
+      progress: progressFn,
+    });
+  }, common.expectsError({
+    message: 'progress error'
+  }));
+});
+
+test('backup fails source db is invalid', async (t) => {
+  const database = makeSourceDb();
+  const destDb = nextDb();
+
+  const progressFn = t.mock.fn(() => {
+    throw new Error('progress error');
+  });
+
+  t.assert.rejects(async () => {
+    await database.backup(destDb, {
+      rate: 1,
+      progress: progressFn,
+      sourceDb: 'invalid',
+    });
+  }, common.expectsError({
+    message: 'unknown database invalid'
+  }));
+});
+
+test('backup fails when destination cannot be opened', async (t) => {
+  const database = makeSourceDb();
+
+  t.assert.rejects(async () => {
+    await database.backup('/invalid/path/to/db.sqlite');
+  }, common.expectsError({
+    message: 'unable to open database file'
+  }));
+});

From c73a3ccd4a0b81ef1d3a4b5e44c6e349210be209 Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Sun, 12 Jan 2025 23:49:34 -0300
Subject: [PATCH 02/11] sqlite: resolve backup promise with total transferred
 pages

---
 src/node_sqlite.cc | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
index c1f4a8e09e141f..15fff94459b2ce 100644
--- a/src/node_sqlite.cc
+++ b/src/node_sqlite.cc
@@ -271,18 +271,20 @@ class BackupJob : public ThreadPoolWork {
       return;
     }
 
-    Local<String> message =
-        String::NewFromUtf8(
-            env()->isolate(), "Backup completed", NewStringType::kNormal)
-            .ToLocalChecked();
+    Local<Object> e;
+    if (!CreateSQLiteError(env()->isolate(), pDest_).ToLocal(&e)) {
+      Finalize();
 
-    Local<Object> e =
-        CreateSQLiteError(env()->isolate(), pDest_).ToLocalChecked();
+      return;
+    }
 
     Finalize();
 
     if (backup_status_ == SQLITE_OK) {
-      resolver->Resolve(env()->context(), message).ToChecked();
+      resolver
+          ->Resolve(env()->context(),
+                    Integer::New(env()->isolate(), total_pages))
+          .ToChecked();
     } else {
       resolver->Reject(env()->context(), e).ToChecked();
     }

From 5ad3f4845f08b71022dd459e8267be462a8c986a Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Mon, 13 Jan 2025 00:01:06 -0300
Subject: [PATCH 03/11] sqlite,test: test DatabaseSync.prototype.backup
 interface

---
 test/parallel/test-sqlite-backup.mjs | 98 +++++++++++++++++++++++++---
 1 file changed, 88 insertions(+), 10 deletions(-)

diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
index 0460f2e5a128d5..e311feedcdbbc1 100644
--- a/test/parallel/test-sqlite-backup.mjs
+++ b/test/parallel/test-sqlite-backup.mjs
@@ -2,7 +2,7 @@ import * as common from '../common/index.mjs';
 import tmpdir from '../common/tmpdir.js';
 import { join } from 'path';
 import { DatabaseSync } from 'node:sqlite';
-import { test } from 'node:test';
+import { describe, test } from 'node:test';
 import { writeFileSync } from 'fs';
 
 let cnt = 0;
@@ -32,17 +32,82 @@ function makeSourceDb() {
   return database;
 }
 
-test('throws exception when trying to start backup from a closed database', async (t) => {
-  t.assert.rejects(async () => {
-    const database = new DatabaseSync(':memory:');
+describe('DatabaseSync.prototype.backup()', () => {
+  test('throws if path is not a string', async (t) => {
+    const database = makeSourceDb();
+
+    t.assert.rejects(async () => {
+      await database.backup();
+    }, common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "destination" argument must be a string.'
+    }));
+
+    t.assert.rejects(async () => {
+      await database.backup({});
+    }, common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "destination" argument must be a string.'
+    }));
+  });
 
-    database.close();
+  test('throws if options is not an object', async (t) => {
+    const database = makeSourceDb();
+
+    t.assert.rejects(async () => {
+      await database.backup('hello.db', 'invalid');
+    }, common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "options" argument must be an object.'
+    }));
+
+    t.assert.rejects(async () => {
+      await database.backup({});
+    }, common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "destination" argument must be a string.'
+    }));
+  });
 
-    await database.backup('backup.db');
-  }, common.expectsError({
-    code: 'ERR_INVALID_STATE',
-    message: 'database is not open'
-  }));
+  test('throws if any of provided options is invalid', async (t) => {
+    const database = makeSourceDb();
+
+    t.assert.rejects(async () => {
+      await database.backup('hello.db', {
+        sourceDb: 42
+      });
+    }, common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "options.sourceDb" argument must be a string.'
+    }));
+
+    t.assert.rejects(async () => {
+      await database.backup('hello.db', {
+        targetDb: 42
+      });
+    }, common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "options.targetDb" argument must be a string.'
+    }));
+
+    t.assert.rejects(async () => {
+      await database.backup('hello.db', {
+        rate: 'invalid'
+      });
+    }, common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "options.rate" argument must be an integer.'
+    }));
+
+    t.assert.rejects(async () => {
+      await database.backup('hello.db', {
+        progress: 'invalid'
+      });
+    }, common.expectsError({
+      code: 'ERR_INVALID_ARG_TYPE',
+      message: 'The "options.progress" argument must be a function.'
+    }));
+  });
 });
 
 test('database backup', async (t) => {
@@ -68,6 +133,19 @@ test('database backup', async (t) => {
   ]);
 });
 
+test('throws exception when trying to start backup from a closed database', async (t) => {
+  t.assert.rejects(async () => {
+    const database = new DatabaseSync(':memory:');
+
+    database.close();
+
+    await database.backup('backup.db');
+  }, common.expectsError({
+    code: 'ERR_INVALID_STATE',
+    message: 'database is not open'
+  }));
+});
+
 test('database backup fails when dest file is not writable', (t) => {
   const readonlyDestDb = nextDb();
   writeFileSync(readonlyDestDb, '', { mode: 0o444 });

From a050cd67b0cc89343392327fff7a99df217dd8ee Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Tue, 14 Jan 2025 12:32:56 -0300
Subject: [PATCH 04/11] sqlite: pass object to progress function instead of two
 args

---
 src/env_properties.h                 |  2 ++
 src/node_sqlite.cc                   | 28 ++++++++++++++++++++--------
 test/parallel/test-sqlite-backup.mjs | 21 +++++++++++++++++++++
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/src/env_properties.h b/src/env_properties.h
index 55116d463bb844..afb34577b01cbd 100644
--- a/src/env_properties.h
+++ b/src/env_properties.h
@@ -73,6 +73,8 @@
   V(address_string, "address")                                                 \
   V(source_db_string, "sourceDb")                                              \
   V(target_db_string, "targetDb")                                              \
+  V(total_pages_string, "totalPages")                                          \
+  V(remaining_pages_string, "remainingPages")                                  \
   V(progress_string, "progress")                                               \
   V(aliases_string, "aliases")                                                 \
   V(alpn_callback_string, "ALPNCallback")                                      \
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
index 15fff94459b2ce..34e89e1bc19a7a 100644
--- a/src/node_sqlite.cc
+++ b/src/node_sqlite.cc
@@ -247,13 +247,25 @@ class BackupJob : public ThreadPoolWork {
           Local<Function>::New(env()->isolate(), progressFunc_);
 
       if (!fn.IsEmpty()) {
-        Local<Value> argv[] = {
-            Integer::New(env()->isolate(), total_pages),
-            Integer::New(env()->isolate(), remaining_pages),
-        };
+        Local<Object> progress_info = Object::New(env()->isolate());
+
+        if (progress_info
+                ->Set(env()->context(),
+                      env()->total_pages_string(),
+                      Integer::New(env()->isolate(), total_pages))
+                .IsNothing() ||
+            progress_info
+                ->Set(env()->context(),
+                      env()->remaining_pages_string(),
+                      Integer::New(env()->isolate(), remaining_pages))
+                .IsNothing()) {
+          return;
+        }
+
+        Local<Value> argv[] = {progress_info};
 
         TryCatch try_catch(env()->isolate());
-        fn->Call(env()->context(), Null(env()->isolate()), 2, argv)
+        fn->Call(env()->context(), Null(env()->isolate()), 1, argv)
             .FromMaybe(Local<Value>());
 
         if (try_catch.HasCaught()) {
@@ -742,7 +754,7 @@ void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
   CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
 }
 
-// database.backup(destination, { sourceDb, targetDb, rate, progress: (total,
+// database.backup(path, { sourceDb, targetDb, rate, progress: (total,
 // remaining) => {} )
 void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
@@ -762,7 +774,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
 
   THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
 
-  Utf8Value destFilename(env->isolate(), args[0].As<String>());
+  Utf8Value destPath(env->isolate(), args[0].As<String>());
   Local<Function> progressFunc = Local<Function>();
 
   if (args.Length() > 1) {
@@ -852,7 +864,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(resolver->GetPromise());
 
   BackupJob* job = new BackupJob(
-      env, db, resolver, source_db, *destFilename, dest_db, rate, progressFunc);
+      env, db, resolver, source_db, *destPath, dest_db, rate, progressFunc);
   db->backups_.insert(job);
   job->ScheduleBackup();
 }
diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
index e311feedcdbbc1..0b6a4a73cfe7a8 100644
--- a/test/parallel/test-sqlite-backup.mjs
+++ b/test/parallel/test-sqlite-backup.mjs
@@ -127,6 +127,27 @@ test('database backup', async (t) => {
   // so the progress function should be called once (the last call is not made since
   // the promise resolves)
   t.assert.strictEqual(progressFn.mock.calls.length, 1);
+  t.assert.deepStrictEqual(progressFn.mock.calls[0].arguments, [{ totalPages: 2, remainingPages: 1 }]);
+  t.assert.deepStrictEqual(rows, [
+    { __proto__: null, key: 1, value: 'value-1' },
+    { __proto__: null, key: 2, value: 'value-2' },
+  ]);
+});
+
+test('database backup in a single call', async (t) => {
+  const progressFn = t.mock.fn();
+  const database = makeSourceDb();
+  const destDb = nextDb();
+
+  // Let rate to be default (100) to backup in a single call
+  await database.backup(destDb, {
+    progress: progressFn,
+  });
+
+  const backup = new DatabaseSync(destDb);
+  const rows = backup.prepare('SELECT * FROM data').all();
+
+  t.assert.strictEqual(progressFn.mock.calls.length, 0);
   t.assert.deepStrictEqual(rows, [
     { __proto__: null, key: 1, value: 'value-1' },
     { __proto__: null, key: 2, value: 'value-2' },

From 9d8538b145da4b6e3366cf88c54f05acfe252f8e Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Tue, 14 Jan 2025 14:05:16 -0300
Subject: [PATCH 05/11] fixup: improve tests

---
 test/parallel/test-sqlite-backup.mjs | 104 ++++++++++++---------------
 1 file changed, 46 insertions(+), 58 deletions(-)

diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
index 0b6a4a73cfe7a8..0e3ba789c764bb 100644
--- a/test/parallel/test-sqlite-backup.mjs
+++ b/test/parallel/test-sqlite-backup.mjs
@@ -1,9 +1,9 @@
-import * as common from '../common/index.mjs';
+import '../common/index.mjs';
 import tmpdir from '../common/tmpdir.js';
-import { join } from 'path';
+import { join } from 'node:path';
 import { DatabaseSync } from 'node:sqlite';
 import { describe, test } from 'node:test';
-import { writeFileSync } from 'fs';
+import { writeFileSync } from 'node:fs';
 
 let cnt = 0;
 
@@ -36,77 +36,70 @@ describe('DatabaseSync.prototype.backup()', () => {
   test('throws if path is not a string', async (t) => {
     const database = makeSourceDb();
 
-    t.assert.rejects(async () => {
-      await database.backup();
-    }, common.expectsError({
+    t.assert.throws(() => {
+      database.backup();
+    }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "destination" argument must be a string.'
-    }));
+    });
 
-    t.assert.rejects(async () => {
-      await database.backup({});
-    }, common.expectsError({
+    t.assert.rejects(() => {
+      database.backup({});
+    }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "destination" argument must be a string.'
-    }));
+    });
   });
 
-  test('throws if options is not an object', async (t) => {
+  test('throws if options is not an object', (t) => {
     const database = makeSourceDb();
 
-    t.assert.rejects(async () => {
-      await database.backup('hello.db', 'invalid');
-    }, common.expectsError({
+    t.assert.rejects(() => {
+      database.backup('hello.db', 'invalid');
+    }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "options" argument must be an object.'
-    }));
-
-    t.assert.rejects(async () => {
-      await database.backup({});
-    }, common.expectsError({
-      code: 'ERR_INVALID_ARG_TYPE',
-      message: 'The "destination" argument must be a string.'
-    }));
+    });
   });
 
-  test('throws if any of provided options is invalid', async (t) => {
+  test('throws if any of provided options is invalid', (t) => {
     const database = makeSourceDb();
 
-    t.assert.rejects(async () => {
-      await database.backup('hello.db', {
+    t.assert.rejects(() => {
+      database.backup('hello.db', {
         sourceDb: 42
       });
-    }, common.expectsError({
+    }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "options.sourceDb" argument must be a string.'
-    }));
+    });
 
-    t.assert.rejects(async () => {
-      await database.backup('hello.db', {
+    t.assert.rejects(() => {
+      database.backup('hello.db', {
         targetDb: 42
       });
-    }, common.expectsError({
+    }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "options.targetDb" argument must be a string.'
-    }));
+    });
 
-    t.assert.rejects(async () => {
-      await database.backup('hello.db', {
+    t.assert.rejects(() => {
+      database.backup('hello.db', {
         rate: 'invalid'
       });
-    }, common.expectsError({
+    }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "options.rate" argument must be an integer.'
-    }));
+    });
 
-    t.assert.rejects(async () => {
-      await database.backup('hello.db', {
+    t.assert.rejects(() => {
+      database.backup('hello.db', {
         progress: 'invalid'
       });
-    }, common.expectsError({
+    }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "options.progress" argument must be a function.'
-    }));
+    });
   });
 });
 
@@ -155,16 +148,16 @@ test('database backup in a single call', async (t) => {
 });
 
 test('throws exception when trying to start backup from a closed database', async (t) => {
-  t.assert.rejects(async () => {
+  t.assert.throws(() => {
     const database = new DatabaseSync(':memory:');
 
     database.close();
 
-    await database.backup('backup.db');
-  }, common.expectsError({
+    database.backup('backup.db');
+  }, {
     code: 'ERR_INVALID_STATE',
     message: 'database is not open'
-  }));
+  });
 });
 
 test('database backup fails when dest file is not writable', (t) => {
@@ -175,10 +168,10 @@ test('database backup fails when dest file is not writable', (t) => {
 
   t.assert.rejects(async () => {
     await database.backup(readonlyDestDb);
-  }, common.expectsError({
+  }, {
     code: 'ERR_SQLITE_ERROR',
     message: 'attempt to write a readonly database'
-  }));
+  });
 });
 
 test('backup fails when progress function throws', async (t) => {
@@ -194,28 +187,23 @@ test('backup fails when progress function throws', async (t) => {
       rate: 1,
       progress: progressFn,
     });
-  }, common.expectsError({
+  }, {
     message: 'progress error'
-  }));
+  });
 });
 
-test('backup fails source db is invalid', async (t) => {
+test('backup fails when source db is invalid', async (t) => {
   const database = makeSourceDb();
   const destDb = nextDb();
 
-  const progressFn = t.mock.fn(() => {
-    throw new Error('progress error');
-  });
-
   t.assert.rejects(async () => {
     await database.backup(destDb, {
       rate: 1,
-      progress: progressFn,
       sourceDb: 'invalid',
     });
-  }, common.expectsError({
+  }, {
     message: 'unknown database invalid'
-  }));
+  });
 });
 
 test('backup fails when destination cannot be opened', async (t) => {
@@ -223,7 +211,7 @@ test('backup fails when destination cannot be opened', async (t) => {
 
   t.assert.rejects(async () => {
     await database.backup('/invalid/path/to/db.sqlite');
-  }, common.expectsError({
+  }, {
     message: 'unable to open database file'
-  }));
+  });
 });

From 4abb1b21df2e35a5b856b9c3331c4f101549b48e Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Tue, 14 Jan 2025 14:20:12 -0300
Subject: [PATCH 06/11] sqlite: update interface

---
 src/env_properties.h                 |  2 --
 src/node_sqlite.cc                   | 10 +++++-----
 test/parallel/test-sqlite-backup.mjs | 20 ++++++++++----------
 3 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/src/env_properties.h b/src/env_properties.h
index afb34577b01cbd..a3a1f4ef1a5bad 100644
--- a/src/env_properties.h
+++ b/src/env_properties.h
@@ -71,8 +71,6 @@
   V(__dirname_string, "__dirname")                                             \
   V(ack_string, "ack")                                                         \
   V(address_string, "address")                                                 \
-  V(source_db_string, "sourceDb")                                              \
-  V(target_db_string, "targetDb")                                              \
   V(total_pages_string, "totalPages")                                          \
   V(remaining_pages_string, "remainingPages")                                  \
   V(progress_string, "progress")                                               \
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
index 34e89e1bc19a7a..6121310e58298f 100644
--- a/src/node_sqlite.cc
+++ b/src/node_sqlite.cc
@@ -754,8 +754,8 @@ void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
   CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
 }
 
-// database.backup(path, { sourceDb, targetDb, rate, progress: (total,
-// remaining) => {} )
+// database.backup(path, { source, target, rate, progress: ({ totalPages,
+// remainingPages }) => {} )
 void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
@@ -803,7 +803,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
     }
 
     Local<Value> sourceDbValue;
-    if (!options->Get(env->context(), env->source_db_string())
+    if (!options->Get(env->context(), env->source_string())
              .ToLocal(&sourceDbValue)) {
       return;
     }
@@ -812,7 +812,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
       if (!sourceDbValue->IsString()) {
         THROW_ERR_INVALID_ARG_TYPE(
             env->isolate(),
-            "The \"options.sourceDb\" argument must be a string.");
+            "The \"options.source\" argument must be a string.");
         return;
       }
 
@@ -821,7 +821,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
     }
 
     Local<Value> targetDbValue;
-    if (!options->Get(env->context(), env->target_db_string())
+    if (!options->Get(env->context(), env->target_string())
              .ToLocal(&targetDbValue)) {
       return;
     }
diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
index 0e3ba789c764bb..4ca0b0d8077a53 100644
--- a/test/parallel/test-sqlite-backup.mjs
+++ b/test/parallel/test-sqlite-backup.mjs
@@ -43,7 +43,7 @@ describe('DatabaseSync.prototype.backup()', () => {
       message: 'The "destination" argument must be a string.'
     });
 
-    t.assert.rejects(() => {
+    t.assert.throws(() => {
       database.backup({});
     }, {
       code: 'ERR_INVALID_ARG_TYPE',
@@ -54,7 +54,7 @@ describe('DatabaseSync.prototype.backup()', () => {
   test('throws if options is not an object', (t) => {
     const database = makeSourceDb();
 
-    t.assert.rejects(() => {
+    t.assert.throws(() => {
       database.backup('hello.db', 'invalid');
     }, {
       code: 'ERR_INVALID_ARG_TYPE',
@@ -65,25 +65,25 @@ describe('DatabaseSync.prototype.backup()', () => {
   test('throws if any of provided options is invalid', (t) => {
     const database = makeSourceDb();
 
-    t.assert.rejects(() => {
+    t.assert.throws(() => {
       database.backup('hello.db', {
-        sourceDb: 42
+        source: 42
       });
     }, {
       code: 'ERR_INVALID_ARG_TYPE',
-      message: 'The "options.sourceDb" argument must be a string.'
+      message: 'The "options.source" argument must be a string.'
     });
 
-    t.assert.rejects(() => {
+    t.assert.throws(() => {
       database.backup('hello.db', {
-        targetDb: 42
+        target: 42
       });
     }, {
       code: 'ERR_INVALID_ARG_TYPE',
       message: 'The "options.targetDb" argument must be a string.'
     });
 
-    t.assert.rejects(() => {
+    t.assert.throws(() => {
       database.backup('hello.db', {
         rate: 'invalid'
       });
@@ -92,7 +92,7 @@ describe('DatabaseSync.prototype.backup()', () => {
       message: 'The "options.rate" argument must be an integer.'
     });
 
-    t.assert.rejects(() => {
+    t.assert.throws(() => {
       database.backup('hello.db', {
         progress: 'invalid'
       });
@@ -199,7 +199,7 @@ test('backup fails when source db is invalid', async (t) => {
   t.assert.rejects(async () => {
     await database.backup(destDb, {
       rate: 1,
-      sourceDb: 'invalid',
+      source: 'invalid',
     });
   }, {
     message: 'unknown database invalid'

From 74a9ad4d53712d4e3ff5a8721c93dbe6f6501e34 Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Tue, 14 Jan 2025 15:19:57 -0300
Subject: [PATCH 07/11] doc: add backup docs

---
 doc/api/sqlite.md | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md
index 07916addeac91a..d01f03913696de 100644
--- a/doc/api/sqlite.md
+++ b/doc/api/sqlite.md
@@ -115,6 +115,26 @@ added: v22.5.0
 
 Constructs a new `DatabaseSync` instance.
 
+### `database.backup(destination[, options])`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* `destination` {string} The path where the backup will be created. If the file already exists, the contents will be
+  overwritten.
+* `options` {Object} Optional configuration for the backup. The
+  following properties are supported:
+  * `source` {string} Name of the source database. **Default:** `'main'`.
+  * `target` {string} NAme of the target database. **Default:** `'main'`.
+  * `rate` {number} Number of pages to be transmitted in each batch of the backup. **Default:** `100`.
+  * `progress` {Function} Callback function that will be called with the number of pages copied and the total number of
+    pages.
+
+This method makes a backup of the database. The returned {Promise} will resolve when the backup is completed and will
+reject if an error occurs. This method abstracts the [`sqlite3_backup_init()`][], [`sqlite3_backup_step()`][] and
+[`sqlite3_backup_finish()`][] functions.
+
 ### `database.close()`
 
 <!-- YAML
@@ -604,6 +624,9 @@ resolution handler passed to [`database.applyChangeset()`][]. See also
 [`SQLITE_DETERMINISTIC`]: https://www.sqlite.org/c3ref/c_deterministic.html
 [`SQLITE_DIRECTONLY`]: https://www.sqlite.org/c3ref/c_deterministic.html
 [`database.applyChangeset()`]: #databaseapplychangesetchangeset-options
+[`sqlite3_backup_finish()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupfinish
+[`sqlite3_backup_init()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupinit
+[`sqlite3_backup_step()`]: https://www.sqlite.org/c3ref/backup_finish.html#sqlite3backupstep
 [`sqlite3_changes64()`]: https://www.sqlite.org/c3ref/changes.html
 [`sqlite3_close_v2()`]: https://www.sqlite.org/c3ref/close.html
 [`sqlite3_create_function_v2()`]: https://www.sqlite.org/c3ref/create_function.html

From d58fd03f84eb457cf31a56866c0cf417388306ca Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Tue, 14 Jan 2025 15:24:39 -0300
Subject: [PATCH 08/11] test: use temp dir to test invalid paths

---
 test/parallel/test-sqlite-backup.mjs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
index 4ca0b0d8077a53..b960ae400cbc23 100644
--- a/test/parallel/test-sqlite-backup.mjs
+++ b/test/parallel/test-sqlite-backup.mjs
@@ -210,7 +210,7 @@ test('backup fails when destination cannot be opened', async (t) => {
   const database = makeSourceDb();
 
   t.assert.rejects(async () => {
-    await database.backup('/invalid/path/to/db.sqlite');
+    await database.backup(`${tmpdir.path}/invalid/backup.db`);
   }, {
     message: 'unable to open database file'
   });

From e3e7431e2abd7c04fb0e160de125333d9a0b6c3b Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Wed, 15 Jan 2025 20:54:56 -0300
Subject: [PATCH 09/11] doc: fixup: fix typos

---
 doc/api/sqlite.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md
index d01f03913696de..010e24a8fd264e 100644
--- a/doc/api/sqlite.md
+++ b/doc/api/sqlite.md
@@ -126,13 +126,13 @@ added: REPLACEME
 * `options` {Object} Optional configuration for the backup. The
   following properties are supported:
   * `source` {string} Name of the source database. **Default:** `'main'`.
-  * `target` {string} NAme of the target database. **Default:** `'main'`.
+  * `target` {string} Name of the target database. **Default:** `'main'`.
   * `rate` {number} Number of pages to be transmitted in each batch of the backup. **Default:** `100`.
   * `progress` {Function} Callback function that will be called with the number of pages copied and the total number of
     pages.
+* Returns: {Promise} A promise that resolves when the backup is completed and rejects if an error occurs.
 
-This method makes a backup of the database. The returned {Promise} will resolve when the backup is completed and will
-reject if an error occurs. This method abstracts the [`sqlite3_backup_init()`][], [`sqlite3_backup_step()`][] and
+This method makes a database backup. This method abstracts the [`sqlite3_backup_init()`][], [`sqlite3_backup_step()`][] and
 [`sqlite3_backup_finish()`][] functions.
 
 ### `database.close()`

From acec23e346752be1c21903d7b9edc00b481089ab Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Wed, 15 Jan 2025 21:25:59 -0300
Subject: [PATCH 10/11] fixup: improve formatting

---
 src/node_sqlite.cc | 115 +++++++++++++++++----------------------------
 1 file changed, 44 insertions(+), 71 deletions(-)

diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
index 6121310e58298f..76015f1e77a45b 100644
--- a/src/node_sqlite.cc
+++ b/src/node_sqlite.cc
@@ -173,7 +173,7 @@ class BackupJob : public ThreadPoolWork {
     Isolate* isolate = env()->isolate();
     HandleScope handle_scope(isolate);
 
-    backup_status_ = sqlite3_open(destination_name_.c_str(), &pDest_);
+    backup_status_ = sqlite3_open(destination_name_.c_str(), &dest_);
 
     Local<Promise::Resolver> resolver =
         Local<Promise::Resolver>::New(env()->isolate(), resolver_);
@@ -181,33 +181,26 @@ class BackupJob : public ThreadPoolWork {
     Local<Object> e = Local<Object>();
 
     if (backup_status_ != SQLITE_OK) {
-      if (!CreateSQLiteError(isolate, pDest_).ToLocal(&e)) {
-        Finalize();
-
+      if (!CreateSQLiteError(isolate, dest_).ToLocal(&e)) {
         return;
       }
 
       Finalize();
-
       resolver->Reject(env()->context(), e).ToChecked();
-
       return;
     }
 
-    pBackup_ = sqlite3_backup_init(
-        pDest_, dest_db_.c_str(), source_->Connection(), source_db_.c_str());
+    backup_ = sqlite3_backup_init(
+        dest_, dest_db_.c_str(), source_->Connection(), source_db_.c_str());
 
-    if (pBackup_ == nullptr) {
-      if (!CreateSQLiteError(isolate, pDest_).ToLocal(&e)) {
+    if (backup_ == nullptr) {
+      if (!CreateSQLiteError(isolate, dest_).ToLocal(&e)) {
         Finalize();
-
         return;
       }
 
       Finalize();
-
       resolver->Reject(env()->context(), e).ToChecked();
-
       return;
     }
 
@@ -215,7 +208,7 @@ class BackupJob : public ThreadPoolWork {
   }
 
   void DoThreadPoolWork() override {
-    backup_status_ = sqlite3_backup_step(pBackup_, pages_);
+    backup_status_ = sqlite3_backup_step(backup_, pages_);
   }
 
   void AfterThreadPoolWork(int status) override {
@@ -228,20 +221,16 @@ class BackupJob : public ThreadPoolWork {
       Local<Object> e;
       if (!CreateSQLiteError(env()->isolate(), backup_status_).ToLocal(&e)) {
         Finalize();
-
         return;
       }
 
       Finalize();
-
       resolver->Reject(env()->context(), e).ToChecked();
-
       return;
     }
 
-    int total_pages = sqlite3_backup_pagecount(pBackup_);
-    int remaining_pages = sqlite3_backup_remaining(pBackup_);
-
+    int total_pages = sqlite3_backup_pagecount(backup_);
+    int remaining_pages = sqlite3_backup_remaining(backup_);
     if (remaining_pages != 0) {
       Local<Function> fn =
           Local<Function>::New(env()->isolate(), progressFunc_);
@@ -263,35 +252,28 @@ class BackupJob : public ThreadPoolWork {
         }
 
         Local<Value> argv[] = {progress_info};
-
         TryCatch try_catch(env()->isolate());
         fn->Call(env()->context(), Null(env()->isolate()), 1, argv)
             .FromMaybe(Local<Value>());
-
         if (try_catch.HasCaught()) {
           Finalize();
-
           resolver->Reject(env()->context(), try_catch.Exception()).ToChecked();
-
           return;
         }
       }
 
       // There's still work to do
       this->ScheduleWork();
-
       return;
     }
 
     Local<Object> e;
-    if (!CreateSQLiteError(env()->isolate(), pDest_).ToLocal(&e)) {
+    if (!CreateSQLiteError(env()->isolate(), dest_).ToLocal(&e)) {
       Finalize();
-
       return;
     }
 
     Finalize();
-
     if (backup_status_ == SQLITE_OK) {
       resolver
           ->Resolve(env()->context(),
@@ -308,15 +290,15 @@ class BackupJob : public ThreadPoolWork {
   }
 
   void Cleanup() {
-    if (pBackup_) {
-      sqlite3_backup_finish(pBackup_);
-      pBackup_ = nullptr;
+    if (backup_) {
+      sqlite3_backup_finish(backup_);
+      backup_ = nullptr;
     }
 
-    if (pDest_) {
-      backup_status_ = sqlite3_errcode(pDest_);
-      sqlite3_close(pDest_);
-      pDest_ = nullptr;
+    if (dest_) {
+      backup_status_ = sqlite3_errcode(dest_);
+      sqlite3_close(dest_);
+      dest_ = nullptr;
     }
   }
 
@@ -327,8 +309,8 @@ class BackupJob : public ThreadPoolWork {
   DatabaseSync* source_;
   Global<Promise::Resolver> resolver_;
   Global<Function> progressFunc_;
-  sqlite3* pDest_ = nullptr;
-  sqlite3_backup* pBackup_ = nullptr;
+  sqlite3* dest_ = nullptr;
+  sqlite3_backup* backup_ = nullptr;
   int pages_;
   int backup_status_;
   std::string source_db_;
@@ -754,10 +736,11 @@ void DatabaseSync::Exec(const FunctionCallbackInfo<Value>& args) {
   CHECK_ERROR_OR_THROW(env->isolate(), db->connection_, r, SQLITE_OK, void());
 }
 
-// database.backup(path, { source, target, rate, progress: ({ totalPages,
-// remainingPages }) => {} )
 void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
+  DatabaseSync* db;
+  ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
   Environment* env = Environment::GetCurrent(args);
+  THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
 
   if (!args[0]->IsString()) {
     THROW_ERR_INVALID_ARG_TYPE(
@@ -769,12 +752,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
   std::string source_db = "main";
   std::string dest_db = "main";
 
-  DatabaseSync* db;
-  ASSIGN_OR_RETURN_UNWRAP(&db, args.This());
-
-  THROW_AND_RETURN_ON_BAD_STATE(env, !db->IsOpen(), "database is not open");
-
-  Utf8Value destPath(env->isolate(), args[0].As<String>());
+  Utf8Value dest_path(env->isolate(), args[0].As<String>());
   Local<Function> progressFunc = Local<Function>();
 
   if (args.Length() > 1) {
@@ -785,74 +763,69 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
     }
 
     Local<Object> options = args[1].As<Object>();
-
-    Local<Value> rateValue;
-    if (!options->Get(env->context(), env->rate_string()).ToLocal(&rateValue)) {
+    Local<Value> rate_v;
+    if (!options->Get(env->context(), env->rate_string()).ToLocal(&rate_v)) {
       return;
     }
 
-    if (!rateValue->IsUndefined()) {
-      if (!rateValue->IsInt32()) {
+    if (!rate_v->IsUndefined()) {
+      if (!rate_v->IsInt32()) {
         THROW_ERR_INVALID_ARG_TYPE(
             env->isolate(),
             "The \"options.rate\" argument must be an integer.");
         return;
       }
-
-      rate = rateValue.As<Int32>()->Value();
+      rate = rate_v.As<Int32>()->Value();
     }
 
-    Local<Value> sourceDbValue;
+    Local<Value> source_v;
     if (!options->Get(env->context(), env->source_string())
-             .ToLocal(&sourceDbValue)) {
+             .ToLocal(&source_v)) {
       return;
     }
 
-    if (!sourceDbValue->IsUndefined()) {
-      if (!sourceDbValue->IsString()) {
+    if (!source_v->IsUndefined()) {
+      if (!source_v->IsString()) {
         THROW_ERR_INVALID_ARG_TYPE(
             env->isolate(),
             "The \"options.source\" argument must be a string.");
         return;
       }
 
-      source_db =
-          Utf8Value(env->isolate(), sourceDbValue.As<String>()).ToString();
+      source_db = Utf8Value(env->isolate(), source_v.As<String>()).ToString();
     }
 
-    Local<Value> targetDbValue;
+    Local<Value> target_v;
     if (!options->Get(env->context(), env->target_string())
-             .ToLocal(&targetDbValue)) {
+             .ToLocal(&target_v)) {
       return;
     }
 
-    if (!targetDbValue->IsUndefined()) {
-      if (!targetDbValue->IsString()) {
+    if (!target_v->IsUndefined()) {
+      if (!target_v->IsString()) {
         THROW_ERR_INVALID_ARG_TYPE(
             env->isolate(),
             "The \"options.targetDb\" argument must be a string.");
         return;
       }
 
-      dest_db =
-          Utf8Value(env->isolate(), targetDbValue.As<String>()).ToString();
+      dest_db = Utf8Value(env->isolate(), target_v.As<String>()).ToString();
     }
 
-    Local<Value> progressValue;
+    Local<Value> progress_v;
     if (!options->Get(env->context(), env->progress_string())
-             .ToLocal(&progressValue)) {
+             .ToLocal(&progress_v)) {
       return;
     }
 
-    if (!progressValue->IsUndefined()) {
-      if (!progressValue->IsFunction()) {
+    if (!progress_v->IsUndefined()) {
+      if (!progress_v->IsFunction()) {
         THROW_ERR_INVALID_ARG_TYPE(
             env->isolate(),
             "The \"options.progress\" argument must be a function.");
         return;
       }
-
-      progressFunc = progressValue.As<Function>();
+      progressFunc = progress_v.As<Function>();
     }
   }
 
@@ -864,7 +837,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(resolver->GetPromise());
 
   BackupJob* job = new BackupJob(
-      env, db, resolver, source_db, *destPath, dest_db, rate, progressFunc);
+      env, db, resolver, source_db, *dest_path, dest_db, rate, progressFunc);
   db->backups_.insert(job);
   job->ScheduleBackup();
 }

From fa23c05a8f1a26cc876401516536b2ea56930595 Mon Sep 17 00:00:00 2001
From: Edy Silva <edigleyssonsilva@gmail.com>
Date: Wed, 15 Jan 2025 21:59:47 -0300
Subject: [PATCH 11/11] fixup: centralize backup error handling

---
 doc/api/sqlite.md                    |  4 +-
 src/node_sqlite.cc                   | 73 ++++++++++++----------------
 test/parallel/test-sqlite-backup.mjs |  2 +-
 3 files changed, 34 insertions(+), 45 deletions(-)

diff --git a/doc/api/sqlite.md b/doc/api/sqlite.md
index 010e24a8fd264e..08fea8d233aed2 100644
--- a/doc/api/sqlite.md
+++ b/doc/api/sqlite.md
@@ -132,8 +132,8 @@ added: REPLACEME
     pages.
 * Returns: {Promise} A promise that resolves when the backup is completed and rejects if an error occurs.
 
-This method makes a database backup. This method abstracts the [`sqlite3_backup_init()`][], [`sqlite3_backup_step()`][] and
-[`sqlite3_backup_finish()`][] functions.
+This method makes a database backup. This method abstracts the [`sqlite3_backup_init()`][], [`sqlite3_backup_step()`][]
+and [`sqlite3_backup_finish()`][] functions.
 
 ### `database.close()`
 
diff --git a/src/node_sqlite.cc b/src/node_sqlite.cc
index 76015f1e77a45b..85276c115b9c3b 100644
--- a/src/node_sqlite.cc
+++ b/src/node_sqlite.cc
@@ -172,35 +172,18 @@ class BackupJob : public ThreadPoolWork {
   void ScheduleBackup() {
     Isolate* isolate = env()->isolate();
     HandleScope handle_scope(isolate);
-
     backup_status_ = sqlite3_open(destination_name_.c_str(), &dest_);
-
     Local<Promise::Resolver> resolver =
         Local<Promise::Resolver>::New(env()->isolate(), resolver_);
-
-    Local<Object> e = Local<Object>();
-
     if (backup_status_ != SQLITE_OK) {
-      if (!CreateSQLiteError(isolate, dest_).ToLocal(&e)) {
-        return;
-      }
-
-      Finalize();
-      resolver->Reject(env()->context(), e).ToChecked();
+      HandleBackupError(resolver);
       return;
     }
 
     backup_ = sqlite3_backup_init(
         dest_, dest_db_.c_str(), source_->Connection(), source_db_.c_str());
-
     if (backup_ == nullptr) {
-      if (!CreateSQLiteError(isolate, dest_).ToLocal(&e)) {
-        Finalize();
-        return;
-      }
-
-      Finalize();
-      resolver->Reject(env()->context(), e).ToChecked();
+      HandleBackupError(resolver);
       return;
     }
 
@@ -218,14 +201,7 @@ class BackupJob : public ThreadPoolWork {
 
     if (!(backup_status_ == SQLITE_OK || backup_status_ == SQLITE_DONE ||
           backup_status_ == SQLITE_BUSY || backup_status_ == SQLITE_LOCKED)) {
-      Local<Object> e;
-      if (!CreateSQLiteError(env()->isolate(), backup_status_).ToLocal(&e)) {
-        Finalize();
-        return;
-      }
-
-      Finalize();
-      resolver->Reject(env()->context(), e).ToChecked();
+      HandleBackupError(resolver, backup_status_);
       return;
     }
 
@@ -234,10 +210,8 @@ class BackupJob : public ThreadPoolWork {
     if (remaining_pages != 0) {
       Local<Function> fn =
           Local<Function>::New(env()->isolate(), progressFunc_);
-
       if (!fn.IsEmpty()) {
         Local<Object> progress_info = Object::New(env()->isolate());
-
         if (progress_info
                 ->Set(env()->context(),
                       env()->total_pages_string(),
@@ -267,21 +241,14 @@ class BackupJob : public ThreadPoolWork {
       return;
     }
 
-    Local<Object> e;
-    if (!CreateSQLiteError(env()->isolate(), dest_).ToLocal(&e)) {
-      Finalize();
+    if (backup_status_ != SQLITE_DONE) {
+      HandleBackupError(resolver);
       return;
     }
 
-    Finalize();
-    if (backup_status_ == SQLITE_OK) {
-      resolver
-          ->Resolve(env()->context(),
-                    Integer::New(env()->isolate(), total_pages))
-          .ToChecked();
-    } else {
-      resolver->Reject(env()->context(), e).ToChecked();
-    }
+    resolver
+        ->Resolve(env()->context(), Integer::New(env()->isolate(), total_pages))
+        .ToChecked();
   }
 
   void Finalize() {
@@ -303,6 +270,28 @@ class BackupJob : public ThreadPoolWork {
   }
 
  private:
+  void HandleBackupError(Local<Promise::Resolver> resolver) {
+    Local<Object> e;
+    if (!CreateSQLiteError(env()->isolate(), dest_).ToLocal(&e)) {
+      Finalize();
+      return;
+    }
+
+    Finalize();
+    resolver->Reject(env()->context(), e).ToChecked();
+  }
+
+  void HandleBackupError(Local<Promise::Resolver> resolver, int errcode) {
+    Local<Object> e;
+    if (!CreateSQLiteError(env()->isolate(), errcode).ToLocal(&e)) {
+      Finalize();
+      return;
+    }
+
+    Finalize();
+    resolver->Reject(env()->context(), e).ToChecked();
+  }
+
   Environment* env() const { return env_; }
 
   Environment* env_;
@@ -805,7 +794,7 @@ void DatabaseSync::Backup(const FunctionCallbackInfo<Value>& args) {
       if (!target_v->IsString()) {
         THROW_ERR_INVALID_ARG_TYPE(
             env->isolate(),
-            "The \"options.targetDb\" argument must be a string.");
+            "The \"options.target\" argument must be a string.");
         return;
       }
 
diff --git a/test/parallel/test-sqlite-backup.mjs b/test/parallel/test-sqlite-backup.mjs
index b960ae400cbc23..19669803852c71 100644
--- a/test/parallel/test-sqlite-backup.mjs
+++ b/test/parallel/test-sqlite-backup.mjs
@@ -80,7 +80,7 @@ describe('DatabaseSync.prototype.backup()', () => {
       });
     }, {
       code: 'ERR_INVALID_ARG_TYPE',
-      message: 'The "options.targetDb" argument must be a string.'
+      message: 'The "options.target" argument must be a string.'
     });
 
     t.assert.throws(() => {