From c1ae7419975f7d664320f66ea3acc8663bbf76cf Mon Sep 17 00:00:00 2001
From: Erlend Egeberg Aasland <erlend.aasland@innova.no>
Date: Wed, 14 Apr 2021 13:45:49 +0200
Subject: [PATCH] bpo-43265: Improve sqlite3.Connection.backup error handling
 (GH-24586)

---
 Lib/sqlite3/test/backup.py                    |  5 +-
 .../2021-02-19-22-24-33.bpo-43265.MyAzCH.rst  |  3 +
 Modules/_sqlite/connection.c                  | 95 ++++++++-----------
 3 files changed, 46 insertions(+), 57 deletions(-)
 create mode 100644 Misc/NEWS.d/next/Library/2021-02-19-22-24-33.bpo-43265.MyAzCH.rst

diff --git a/Lib/sqlite3/test/backup.py b/Lib/sqlite3/test/backup.py
index cbe24df2e9c..4e30594bec6 100644
--- a/Lib/sqlite3/test/backup.py
+++ b/Lib/sqlite3/test/backup.py
@@ -149,10 +149,7 @@ class BackupTests(unittest.TestCase):
         with self.assertRaises(sqlite.OperationalError) as cm:
             with sqlite.connect(':memory:') as bck:
                 self.cx.backup(bck, name='non-existing')
-        self.assertIn(
-            str(cm.exception),
-            ['SQL logic error', 'SQL logic error or missing database']
-        )
+        self.assertIn("unknown database", str(cm.exception))
 
         self.cx.execute("ATTACH DATABASE ':memory:' AS attached_db")
         self.cx.execute('CREATE TABLE attached_db.foo (key INTEGER)')
diff --git a/Misc/NEWS.d/next/Library/2021-02-19-22-24-33.bpo-43265.MyAzCH.rst b/Misc/NEWS.d/next/Library/2021-02-19-22-24-33.bpo-43265.MyAzCH.rst
new file mode 100644
index 00000000000..3e7f34ea564
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2021-02-19-22-24-33.bpo-43265.MyAzCH.rst
@@ -0,0 +1,3 @@
+Improve :meth:`sqlite3.Connection.backup` error handling. The error message
+for non-existant target database names is now ``unknown database <database
+name>`` instead of ``SQL logic error``. Patch by Erlend E. Aasland.
diff --git a/Modules/_sqlite/connection.c b/Modules/_sqlite/connection.c
index 9bf2a35ab0e..6d3ccb9b941 100644
--- a/Modules/_sqlite/connection.c
+++ b/Modules/_sqlite/connection.c
@@ -1601,7 +1601,6 @@ pysqlite_connection_backup_impl(pysqlite_Connection *self,
 /*[clinic end generated code: output=306a3e6a38c36334 input=30ae45fc420bfd3b]*/
 {
     int rc;
-    int callback_error = 0;
     int sleep_ms = (int)(sleep * 1000.0);
     sqlite3 *bck_conn;
     sqlite3_backup *bck_handle;
@@ -1643,60 +1642,50 @@ pysqlite_connection_backup_impl(pysqlite_Connection *self,
     bck_handle = sqlite3_backup_init(bck_conn, "main", self->db, name);
     Py_END_ALLOW_THREADS
 
-    if (bck_handle) {
-        do {
-            Py_BEGIN_ALLOW_THREADS
-            rc = sqlite3_backup_step(bck_handle, pages);
-            Py_END_ALLOW_THREADS
-
-            if (progress != Py_None) {
-                PyObject *res;
-
-                res = PyObject_CallFunction(progress, "iii", rc,
-                                            sqlite3_backup_remaining(bck_handle),
-                                            sqlite3_backup_pagecount(bck_handle));
-                if (res == NULL) {
-                    /* User's callback raised an error: interrupt the loop and
-                       propagate it. */
-                    callback_error = 1;
-                    rc = -1;
-                } else {
-                    Py_DECREF(res);
-                }
-            }
-
-            /* Sleep for a while if there are still further pages to copy and
-               the engine could not make any progress */
-            if (rc == SQLITE_BUSY || rc == SQLITE_LOCKED) {
-                Py_BEGIN_ALLOW_THREADS
-                sqlite3_sleep(sleep_ms);
-                Py_END_ALLOW_THREADS
-            }
-        } while (rc == SQLITE_OK || rc == SQLITE_BUSY || rc == SQLITE_LOCKED);
-
-        Py_BEGIN_ALLOW_THREADS
-        rc = sqlite3_backup_finish(bck_handle);
-        Py_END_ALLOW_THREADS
-    } else {
-        rc = _pysqlite_seterror(bck_conn, NULL);
-    }
-
-    if (!callback_error && rc != SQLITE_OK) {
-        /* We cannot use _pysqlite_seterror() here because the backup APIs do
-           not set the error status on the connection object, but rather on
-           the backup handle. */
-        if (rc == SQLITE_NOMEM) {
-            (void)PyErr_NoMemory();
-        } else {
-            PyErr_SetString(pysqlite_OperationalError, sqlite3_errstr(rc));
-        }
-    }
-
-    if (!callback_error && rc == SQLITE_OK) {
-        Py_RETURN_NONE;
-    } else {
+    if (bck_handle == NULL) {
+        _pysqlite_seterror(bck_conn, NULL);
         return NULL;
     }
+
+    do {
+        Py_BEGIN_ALLOW_THREADS
+        rc = sqlite3_backup_step(bck_handle, pages);
+        Py_END_ALLOW_THREADS
+
+        if (progress != Py_None) {
+            int remaining = sqlite3_backup_remaining(bck_handle);
+            int pagecount = sqlite3_backup_pagecount(bck_handle);
+            PyObject *res = PyObject_CallFunction(progress, "iii", rc,
+                                                  remaining, pagecount);
+            if (res == NULL) {
+                /* Callback failed: abort backup and bail. */
+                Py_BEGIN_ALLOW_THREADS
+                sqlite3_backup_finish(bck_handle);
+                Py_END_ALLOW_THREADS
+                return NULL;
+            }
+            Py_DECREF(res);
+        }
+
+        /* Sleep for a while if there are still further pages to copy and
+           the engine could not make any progress */
+        if (rc == SQLITE_BUSY || rc == SQLITE_LOCKED) {
+            Py_BEGIN_ALLOW_THREADS
+            sqlite3_sleep(sleep_ms);
+            Py_END_ALLOW_THREADS
+        }
+    } while (rc == SQLITE_OK || rc == SQLITE_BUSY || rc == SQLITE_LOCKED);
+
+    Py_BEGIN_ALLOW_THREADS
+    rc = sqlite3_backup_finish(bck_handle);
+    Py_END_ALLOW_THREADS
+
+    if (rc != SQLITE_OK) {
+        _pysqlite_seterror(bck_conn, NULL);
+        return NULL;
+    }
+
+    Py_RETURN_NONE;
 }
 
 /*[clinic input]