From f1de595acc30463fab73185eafdfb4ce4c9f2ddb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= <sybren@blender.org>
Date: Thu, 20 Mar 2025 13:32:29 +0100
Subject: [PATCH] Manager: log an error when database rollback fails

Log an error when a database transaction rollback fails. I wouldn't know
how to test, because usually stuff Just Works, but it's good to have
this logged anyway.
---
 internal/manager/persistence/db.go | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/internal/manager/persistence/db.go b/internal/manager/persistence/db.go
index 29c112fd..d68e7761 100644
--- a/internal/manager/persistence/db.go
+++ b/internal/manager/persistence/db.go
@@ -6,6 +6,7 @@ package persistence
 import (
 	"context"
 	"database/sql"
+	"errors"
 	"fmt"
 	"time"
 
@@ -163,7 +164,7 @@ func (db *DB) queries() *sqlc.Queries {
 type queriesTX struct {
 	queries  *sqlc.Queries
 	commit   func() error
-	rollback func() error
+	rollback func()
 }
 
 // queries returns the SQLC Queries struct, connected to this database.
@@ -182,12 +183,29 @@ func (db *DB) queriesWithTX() (*queriesTX, error) {
 	qtx := queriesTX{
 		queries:  sqlc.New(&loggingWrapper),
 		commit:   tx.Commit,
-		rollback: tx.Rollback,
+		rollback: rollbackWrapper(tx.Rollback),
 	}
 
 	return &qtx, nil
 }
 
+func rollbackWrapper(rollback func() error) func() {
+	return func() {
+		err := rollback()
+
+		// AThis function is typically called unconditionally via `defer` and so the
+		// most common case is that the transaction has already been committed, and
+		// thus ErrTxDone is returned here.
+
+		switch {
+		case err == nil: // Not really expected, but a good rollback is ok.
+		case errors.Is(err, sql.ErrTxDone): // Expected.
+		default:
+			log.Error().Msg("database: query rollback failed unexpectedly")
+		}
+	}
+}
+
 // now returns 'now' as reported by db.nowfunc.
 // It always converts the timestamp to UTC.
 func (db *DB) now() time.Time {