From a256b83734e97e1536cf8831e8340787c8905cd4 Mon Sep 17 00:00:00 2001 From: sneak Date: Sun, 28 Jun 2026 10:17:51 +0200 Subject: [PATCH] Fix errcheck lint failures with proper error handling Handle every error return flagged by golangci-lint's errcheck rather than discarding it: - run.go: add a cleanup() helper that logs a warning (and ignores ErrNotExist) when removing a temp file fails, so leaked scratch files are surfaced; use it for all best-effort removals. - copy.go / extract.go: log a warning on deferred Close() failures for the source file, destination DB, and result-set rows. - extract.go: on the rollback path, ignore the benign sql.ErrTxDone (already committed) and log any other rollback failure. - verify.go: add killCat() which ignores os.ErrProcessDone (zstdcat already exited via SIGPIPE) and logs any unexpected kill failure. make check is clean (0 lint issues, tests pass). --- internal/bsdaily/copy.go | 6 +++++- internal/bsdaily/extract.go | 22 ++++++++++++++++++---- internal/bsdaily/run.go | 29 +++++++++++++++++++---------- internal/bsdaily/verify.go | 20 +++++++++++++++++--- 4 files changed, 59 insertions(+), 18 deletions(-) diff --git a/internal/bsdaily/copy.go b/internal/bsdaily/copy.go index 6532b78..901ca0b 100644 --- a/internal/bsdaily/copy.go +++ b/internal/bsdaily/copy.go @@ -21,7 +21,11 @@ func CopyFile(src, dst string) (err error) { if err != nil { return fmt.Errorf("opening source %s: %w", src, err) } - defer srcFile.Close() + defer func() { + if cerr := srcFile.Close(); cerr != nil { + slog.Warn("failed to close source file", "src", src, "error", cerr) + } + }() srcInfo, err := srcFile.Stat() if err != nil { diff --git a/internal/bsdaily/extract.go b/internal/bsdaily/extract.go index 920f096..18dafc0 100644 --- a/internal/bsdaily/extract.go +++ b/internal/bsdaily/extract.go @@ -29,7 +29,11 @@ func ExtractDay(srcDBPath, dstDBPath string, targetDay time.Time) error { if err != nil { return fmt.Errorf("opening destination database: %w", err) } - defer db.Close() + defer func() { + if cerr := db.Close(); cerr != nil { + slog.Warn("failed to close destination database", "path", dstDBPath, "error", cerr) + } + }() // Attach source database if _, err := db.Exec("ATTACH DATABASE ? AS src", srcDBPath); err != nil { @@ -42,7 +46,11 @@ func ExtractDay(srcDBPath, dstDBPath string, targetDay time.Time) error { if err != nil { return fmt.Errorf("reading source schema: %w", err) } - defer rows.Close() + defer func() { + if cerr := rows.Close(); cerr != nil { + slog.Warn("failed to close schema rows", "error", cerr) + } + }() var ddlStatements []string for rows.Next() { @@ -69,7 +77,9 @@ func ExtractDay(srcDBPath, dstDBPath string, targetDay time.Time) error { } defer func() { if err != nil { - tx.Rollback() + if rerr := tx.Rollback(); rerr != nil && !errors.Is(rerr, sql.ErrTxDone) { + slog.Warn("failed to roll back transaction", "error", rerr) + } } }() @@ -132,7 +142,11 @@ func ExtractDay(srcDBPath, dstDBPath string, targetDay time.Time) error { if err != nil { return fmt.Errorf("reading source indexes: %w", err) } - defer idxRows.Close() + defer func() { + if cerr := idxRows.Close(); cerr != nil { + slog.Warn("failed to close index rows", "error", cerr) + } + }() var idxStatements []string for idxRows.Next() { diff --git a/internal/bsdaily/run.go b/internal/bsdaily/run.go index fa27311..2093d6f 100644 --- a/internal/bsdaily/run.go +++ b/internal/bsdaily/run.go @@ -9,6 +9,15 @@ import ( "time" ) +// cleanup removes a temporary file, logging a warning if removal fails so +// that leaked scratch files are surfaced rather than silently ignored. A +// missing file is not an error. +func cleanup(path string) { + if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + slog.Warn("failed to remove temporary file", "path", path, "error", err) + } +} + func Run(targetDates []time.Time) error { snapshotDir, snapshotDate, err := FindLatestDailySnapshot() if err != nil { @@ -100,7 +109,7 @@ func Run(targetDates []time.Time) error { if err := ExtractDay(dstDB, extractedDB, targetDay); err != nil { if errors.Is(err, ErrNoPosts) { slog.Warn("no posts found, skipping day", "date", dayStr) - os.Remove(extractedDB) + cleanup(extractedDB) skipped++ continue } @@ -109,7 +118,7 @@ func Run(targetDates []time.Time) error { // Dump to SQL and compress if err := os.MkdirAll(outputDir, 0755); err != nil { - os.Remove(extractedDB) + cleanup(extractedDB) return fmt.Errorf("creating output directory %s: %w", outputDir, err) } @@ -117,35 +126,35 @@ func Run(targetDates []time.Time) error { slog.Info("dumping and compressing", "tmp_output", outputTmp) if err := DumpAndCompress(extractedDB, outputTmp); err != nil { - os.Remove(outputTmp) - os.Remove(extractedDB) + cleanup(outputTmp) + cleanup(extractedDB) return fmt.Errorf("dump and compress for %s: %w", dayStr, err) } slog.Info("verifying compressed output") if err := VerifyOutput(outputTmp); err != nil { - os.Remove(outputTmp) - os.Remove(extractedDB) + cleanup(outputTmp) + cleanup(extractedDB) return fmt.Errorf("verification failed for %s: %w", dayStr, err) } // Atomic rename to final path slog.Info("renaming to final output", "from", outputTmp, "to", outputFinal) if err := os.Rename(outputTmp, outputFinal); err != nil { - os.Remove(outputTmp) - os.Remove(extractedDB) + cleanup(outputTmp) + cleanup(extractedDB) return fmt.Errorf("atomic rename for %s: %w", dayStr, err) } info, err := os.Stat(outputFinal) if err != nil { - os.Remove(extractedDB) + cleanup(extractedDB) return fmt.Errorf("stat final output: %w", err) } slog.Info("day completed", "date", dayStr, "path", outputFinal, "size_bytes", info.Size()) // Remove extracted DB to reclaim space immediately - os.Remove(extractedDB) + cleanup(extractedDB) processed++ } diff --git a/internal/bsdaily/verify.go b/internal/bsdaily/verify.go index 2e60558..c419c3d 100644 --- a/internal/bsdaily/verify.go +++ b/internal/bsdaily/verify.go @@ -1,12 +1,26 @@ package bsdaily import ( + "errors" "fmt" "log/slog" + "os" "os/exec" "strings" ) +// killCat terminates the zstdcat process, ignoring the benign case where it +// has already exited (e.g. after receiving SIGPIPE when head closed the pipe) +// and logging any other failure. +func killCat(cmd *exec.Cmd) { + if cmd.Process == nil { + return + } + if err := cmd.Process.Kill(); err != nil && !errors.Is(err, os.ErrProcessDone) { + slog.Warn("failed to kill zstdcat process", "error", err) + } +} + func VerifyOutput(path string) error { slog.Info("running zstdmt integrity check") testCmd := exec.Command("zstdmt", "--test", path) @@ -34,18 +48,18 @@ func VerifyOutput(path string) error { return fmt.Errorf("starting zstdcat: %w", err) } if err := headCmd.Start(); err != nil { - catCmd.Process.Kill() // Clean up if head fails to start + killCat(catCmd) // Clean up if head fails to start return fmt.Errorf("starting head: %w", err) } // Wait for head first (it will exit when it has enough lines) if err := headCmd.Wait(); err != nil { - catCmd.Process.Kill() + killCat(catCmd) return fmt.Errorf("head command failed: %w", err) } // Kill zstdcat since head closed the pipe (expected SIGPIPE) - catCmd.Process.Kill() + killCat(catCmd) _ = catCmd.Wait() // Reap the process content := headOut.String()