fix: a restore with pg_restore 17 into Postgres 16 isn't stopped by a setting the server lacks

pg_restore 17 opens with `SET transaction_timeout = 0`, which Postgres 16 refuses.
Under --exit-on-error that one harmless statement ended every restore: the
image ships pg_restore 17.11 and the bundled server is pg16, so a restore
from the bucket (and the local restore) could never have loaded a dump. The
backup round trip, now that CI has pg_dump, found it.

pg_restore now runs to the end and its errors are judged afterwards: the
known harmless one is ignored, and any other fails the restore with that
error's words rather than the "Command was:" line that followed it.

Also sorts an import the lint lane flagged.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

backend/app/services/backup_service.py

@@ -347,17 +347,50 @@ def wipe_db(*, db_url: str) -> None:
     )
 
 
+# A pg_restore newer than the server sets parameters the server doesn't have
+# (pg_restore 17 against Postgres 16: `SET transaction_timeout = 0`). The
+# statement fails and nothing depends on it, so its error is not a failed
+# restore.
+_HARMLESS_RESTORE_ERRORS = (
+    'unrecognized configuration parameter "transaction_timeout"',
+)
+
+
+def _restore_errors(stderr: str) -> list[str]:
+    """pg_restore's error lines, less the harmless ones."""
+    return [
+        line.strip() for line in stderr.splitlines()
+        if "error:" in line and not any(h in line for h in _HARMLESS_RESTORE_ERRORS)
+    ]
+
+
 def restore_db(*, db_url: str, sql_path: Path) -> None:
-    """Wipe the public schema, then load the custom-format dump. pg_restore
-    stops at the first error, which RestoreFailed carries; the database is
-    then partly loaded, and the caller decides what to put back."""
+    """Wipe the public schema, then load the custom-format dump. Any error
+    pg_restore reports, other than a harmless one, raises RestoreFailed with
+    the first; the database is then partly loaded, and the caller decides what
+    to put back."""
     wipe_db(db_url=db_url)
-    # Custom-format (-Fc) dumps are restored with pg_restore, not psql.
-    _run_reporting(
-        _gently(["pg_restore", "--no-owner", "--no-acl", "--exit-on-error",
-                 "-d", _libpq_url(db_url), str(sql_path)]),
-        _RESTORE_SUBPROCESS_TIMEOUT_S,
-    )
+    # Custom-format (-Fc) dumps are restored with pg_restore, not psql. It runs
+    # on past errors (it can't skip the harmless one and stop at the rest), so
+    # its errors are judged once it is done.
+    cmd = _gently(["pg_restore", "--no-owner", "--no-acl",
+                   "-d", _libpq_url(db_url), str(sql_path)])
+    with tempfile.TemporaryFile() as err:
+        proc = subprocess.run(cmd, stdout=subprocess.DEVNULL, stderr=err,
+                              timeout=_RESTORE_SUBPROCESS_TIMEOUT_S)
+        if proc.returncode == 0:
+            return
+        err.seek(0)
+        said = err.read().decode(errors="replace")
+    errors = _restore_errors(said)
+    if errors:
+        raise RestoreFailed(errors[0] if len(errors) == 1
+                            else f"{errors[0]} ({len(errors)} errors in all)")
+    if not any(h in said for h in _HARMLESS_RESTORE_ERRORS):
+        # A failure with no error line to show for it.
+        lines = said.strip().splitlines()
+        raise RestoreFailed(lines[-1].strip() if lines
+                            else f"pg_restore exited with {proc.returncode}")
 
 
 def restore_images(*, images_root: Path, tar_path: Path) -> None:

tests/test_backup_restore.py

@@ -198,7 +198,7 @@ def test_a_restore_out_of_time_queues_its_next_chunk(db_sync, tmp_path, run_env,
 def test_a_restart_resumes_the_file_phase_and_drops_the_stale_chunk(
     db_sync, tmp_path, run_env,
 ):
-    from backend.app.tasks.backup import resume_offsite_runs, restore_offsite_task
+    from backend.app.tasks.backup import restore_offsite_task, resume_offsite_runs
     _file(tmp_path, "a.jpg", b"aa")
     _image(db_sync, tmp_path / "a.jpg", b"aa")
     now = datetime.now(UTC)

tests/test_backup_service.py

@@ -173,11 +173,52 @@ def test_restore_db_drops_schema_then_loads(tmp_path, fake_subprocess):
     assert "-c" in fake_subprocess[0]
     assert "DROP SCHEMA IF EXISTS public CASCADE" in fake_subprocess[0][-1]
     assert "pg_terminate_backend" in fake_subprocess[0][-1]
-    assert "pg_restore" in fake_subprocess[1] and "--exit-on-error" in fake_subprocess[1]
+    assert "pg_restore" in fake_subprocess[1]
     assert "-d" in fake_subprocess[1]
     assert str(dump_path) in fake_subprocess[1]
 
 
+def _pg_restore_says(monkeypatch, stderr_text: bytes):
+    class _Done:
+        returncode = 1
+
+    class _Ok:
+        returncode = 0
+
+    def run(cmd, stderr=None, **kw):
+        if "pg_restore" in cmd:
+            stderr.write(stderr_text)
+            return _Done()
+        return _Ok()
+
+    monkeypatch.setattr("subprocess.run", run)
+
+
+_NEWER_CLIENT = (
+    b'pg_restore: error: could not execute query: ERROR:  unrecognized configuration '
+    b'parameter "transaction_timeout"\nCommand was: SET transaction_timeout = 0;\n'
+)
+
+
+def test_a_newer_pg_restore_setting_what_the_server_lacks_is_not_a_failure(
+    tmp_path, monkeypatch,
+):
+    _pg_restore_says(monkeypatch, _NEWER_CLIENT
+                     + b"pg_restore: warning: errors ignored on restore: 1\n")
+    backup_service.restore_db(db_url="postgresql://u@h/d", sql_path=tmp_path / "x.dump")
+
+
+def test_a_real_error_beside_the_harmless_one_fails_the_restore_with_its_words(
+    tmp_path, monkeypatch,
+):
+    _pg_restore_says(monkeypatch, _NEWER_CLIENT
+                     + b'pg_restore: error: could not execute query: ERROR:  relation '
+                       b'"tag" does not exist\nCommand was: COPY tag FROM stdin;\n'
+                     + b"pg_restore: warning: errors ignored on restore: 2\n")
+    with pytest.raises(backup_service.RestoreFailed, match='relation "tag" does not exist'):
+        backup_service.restore_db(db_url="postgresql://u@h/d", sql_path=tmp_path / "x.dump")
+
+
 def test_a_failed_restore_says_what_pg_restore_said(tmp_path, monkeypatch):
     class _Done:
         def __init__(self, code):