diff --git a/TODO.md b/TODO.md index e124826..44dd6e1 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,16 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-13: Made Ctrl-C or SIGTERM during a local index write end the + command as interrupted + ([issue #301](https://git.eeqj.de/sneak/vaultik/issues/301)). Once the + run is cancelled, `database/sql` rolls an open transaction back itself, + so the rollback in `WithTx` failed with `sql.ErrTxDone` and the process + exited 1 with `FATAL: failed to rollback transaction`, skipping the + interrupted exit and message. That result is no longer an error, and a + rollback that does fail is returned with the transaction's error + instead of ending the process. + - 2026-10-13: Made `snapshot remove` fail for a snapshot that does not exist ([issue #293](https://git.eeqj.de/sneak/vaultik/issues/293)). The command never checked the ID: it reported the snapshot removed diff --git a/internal/database/repositories.go b/internal/database/repositories.go index 64a3f66..4db15d1 100644 --- a/internal/database/repositories.go +++ b/internal/database/repositories.go @@ -3,6 +3,7 @@ package database import ( "context" "database/sql" + "errors" "fmt" ) @@ -60,22 +61,24 @@ func (r *Repositories) WithTx(ctx context.Context, fn TxFunc) error { defer func() { if p := recover(); p != nil { - rollbackErr := tx.Rollback() - if rollbackErr != nil { - Fatalf("failed to rollback transaction: %v", rollbackErr) - } + // The panic is the failure to report, so a rollback error + // is dropped. + _ = tx.Rollback() panic(p) - } else if err != nil { - rollbackErr := tx.Rollback() - if rollbackErr != nil { - Fatalf("failed to rollback transaction: %v", rollbackErr) - } } }() err = fn(ctx, tx) if err != nil { + // Once ctx is cancelled, database/sql rolls the transaction + // back itself and Rollback returns sql.ErrTxDone. + rollbackErr := tx.Rollback() + if rollbackErr != nil && !errors.Is(rollbackErr, sql.ErrTxDone) { + return fmt.Errorf("%w; rolling back transaction: %w", + err, rollbackErr) + } + return err } diff --git a/internal/database/repositories_test.go b/internal/database/repositories_test.go index f4e2199..f6c62f0 100644 --- a/internal/database/repositories_test.go +++ b/internal/database/repositories_test.go @@ -235,6 +235,64 @@ func TestRepositoriesTransactionRollback(t *testing.T) { } } +// When the context is cancelled during the body, database/sql rolls the +// transaction back itself, so WithTx's own rollback finds it already +// done. WithTx must still return the body's error. +func TestRepositoriesTransactionContextCancelled(t *testing.T) { + t.Parallel() + + db, cleanup := setupTestDB(t) + defer cleanup() + + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + repos := database.NewRepositories(db) + + err := repos.WithTx(ctx, func(ctx context.Context, tx *sql.Tx) error { + file := &database.File{ + Path: "/test/cancelled_file.txt", + MTime: time.Now().Truncate(time.Second), + Size: 1024, + Mode: 0644, + UID: 1000, + GID: 1000, + } + + err := repos.Files.Create(ctx, tx, file) + if err != nil { + return err + } + + cancel() + + // database/sql rolls back from a goroutine of its own; wait + // until it has, so the test does not depend on that race. + for { + _, err = tx.ExecContext(context.WithoutCancel(ctx), "SELECT 1") + if errors.Is(err, sql.ErrTxDone) { + return ctx.Err() + } + + time.Sleep(time.Millisecond) + } + }) + if !errors.Is(err, context.Canceled) { + t.Fatalf("expected context.Canceled, got: %v", err) + } + + file, err := repos.Files.GetByPath( + context.Background(), "/test/cancelled_file.txt", + ) + if err != nil { + t.Fatalf("error checking for file: %v", err) + } + + if file != nil { + t.Error("file should not exist after the cancelled transaction") + } +} + func TestRepositoriesReadTransaction(t *testing.T) { t.Parallel()