What was wrong: three failures ended the process, skipping deferred cleanup: a failed rollback in WithReadTx (including after Ctrl-C, when the rollback finds the transaction already rolled back), a failed close of query results in internal/database, and a failed close or remove of a blob's temp file in the upload step.
What changed:
WithReadTx handles its rollback exactly as WithTx does since #301.
The query functions close their rows with defer func() { _ = rows.Close() }(). This loses nothing: each reads its rows to the end, where database/sql closes them itself and returns any close failure from rows.Err(), which each already returns (the Rows.Close documentation says so). The deferred close only does work on paths already returning an error. database.Fatalf had no callers left and is removed.
cleanupBlobTempFile returns its close or remove failure, and handleBlobReady returns it, joined with the upload or metadata error when there is one. When the close fails the remove is skipped, but the packer closes and removes the temp file itself whenever the upload step returns an error.
Judgement call: a temp file that cannot be closed or removed after a successful upload now fails the backup with that error, where it used to end the process, rather than being logged and skipped.
Not tested: no test makes closing query results fail; the SQLite driver fails that close only with the error the last read already returned.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/345.
**What was wrong:** three failures ended the process, skipping deferred cleanup: a failed rollback in `WithReadTx` (including after Ctrl-C, when the rollback finds the transaction already rolled back), a failed close of query results in `internal/database`, and a failed close or remove of a blob's temp file in the upload step.
**What changed:**
- `WithReadTx` handles its rollback exactly as `WithTx` does since https://git.eeqj.de/sneak/vaultik/issues/301.
- The query functions close their rows with `defer func() { _ = rows.Close() }()`. This loses nothing: each reads its rows to the end, where `database/sql` closes them itself and returns any close failure from `rows.Err()`, which each already returns (the `Rows.Close` documentation says so). The deferred close only does work on paths already returning an error. `database.Fatalf` had no callers left and is removed.
- `cleanupBlobTempFile` returns its close or remove failure, and `handleBlobReady` returns it, joined with the upload or metadata error when there is one. When the close fails the remove is skipped, but the packer closes and removes the temp file itself whenever the upload step returns an error.
Judgement call: a temp file that cannot be closed or removed after a successful upload now fails the backup with that error, where it used to end the process, rather than being logged and skipped.
Not tested: no test makes closing query results fail; the SQLite driver fails that close only with the error the last read already returned.
Model: opus-5-5
WithReadTx ended the process when its rollback failed, including after
Ctrl-C, when database/sql has already rolled the transaction back and
Rollback returns sql.ErrTxDone. It now handles the rollback as WithTx
does. The query functions ended the process when closing their rows
failed; they now ignore the deferred close, because database/sql reports
a close failure through rows.Err(), which each returns, once the rows
are read to the end. The unused database.Fatalf is removed.
cleanupBlobTempFile returns its close or remove failure and the upload
step returns it, so such a failure now fails the backup.
No test makes a rows close fail: SQLite fails it only with the error the
last read already returned.
Model: opus-5-5
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #345.
What was wrong: three failures ended the process, skipping deferred cleanup: a failed rollback in
WithReadTx(including after Ctrl-C, when the rollback finds the transaction already rolled back), a failed close of query results ininternal/database, and a failed close or remove of a blob's temp file in the upload step.What changed:
WithReadTxhandles its rollback exactly asWithTxdoes since #301.defer func() { _ = rows.Close() }(). This loses nothing: each reads its rows to the end, wheredatabase/sqlcloses them itself and returns any close failure fromrows.Err(), which each already returns (theRows.Closedocumentation says so). The deferred close only does work on paths already returning an error.database.Fatalfhad no callers left and is removed.cleanupBlobTempFilereturns its close or remove failure, andhandleBlobReadyreturns it, joined with the upload or metadata error when there is one. When the close fails the remove is skipped, but the packer closes and removes the temp file itself whenever the upload step returns an error.Judgement call: a temp file that cannot be closed or removed after a successful upload now fails the backup with that error, where it used to end the process, rather than being logged and skipped.
Not tested: no test makes closing query results fail; the SQLite driver fails that close only with the error the last read already returned.
Model: opus-5-5
Review passed.
Model: opus-5-5