diff --git a/internal/config/cachesize.go b/internal/config/cachesize.go index 263035a..854d729 100644 --- a/internal/config/cachesize.go +++ b/internal/config/cachesize.go @@ -36,7 +36,9 @@ type FreeSpaceProbeFunc func(path string) (uint64, error) // the given path, as available to unprivileged processes. func defaultFreeSpaceProbe(path string) (uint64, error) { var stat syscall.Statfs_t - if err := syscall.Statfs(path, &stat); err != nil { + + err := syscall.Statfs(path, &stat) + if err != nil { return 0, err } @@ -82,9 +84,10 @@ func (c *Config) resolveCacheMaxBytes( if !c.cacheMaxBytesExplicit { cacheDir := filepath.Join(c.StateDir, "cache") - if err := os.MkdirAll(cacheDir, cacheDirPerms); err != nil { + err := os.MkdirAll(cacheDir, cacheDirPerms) + if err != nil { return fmt.Errorf("config key %q: cannot create cache directory %q: %w", - "cache_max_bytes", cacheDir, err) + keyCacheMaxBytes, cacheDir, err) } limit, err := ComputeDefaultCacheMaxBytes(cacheDir, probe) diff --git a/internal/config/config.go b/internal/config/config.go index 80a089b..6f96908 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -134,7 +134,8 @@ func New(_ fx.Lifecycle, params Params) (*Config, error) { return nil, err } - if err := c.resolveCacheMaxBytes(log, defaultFreeSpaceProbe); err != nil { + err = c.resolveCacheMaxBytes(log, defaultFreeSpaceProbe) + if err != nil { return nil, err } diff --git a/internal/imgcache/eviction.go b/internal/imgcache/eviction.go index f359364..0deb579 100644 --- a/internal/imgcache/eviction.go +++ b/internal/imgcache/eviction.go @@ -130,7 +130,8 @@ func (c *Cache) evictBatch(ctx context.Context, excessBytes int64) (int64, error break } - if err := c.evictCandidate(ctx, candidate); err != nil { + err := c.evictCandidate(ctx, candidate) + if err != nil { c.log.Warn("failed to evict cache entry", "cache_key", candidate.cacheKey, "content_hash", candidate.contentHash, @@ -209,7 +210,9 @@ func (c *Cache) variantCandidates(ctx context.Context) ([]evictionCandidate, err candidate := evictionCandidate{isVariant: true} var key string - if err := rows.Scan(&key, &candidate.sizeBytes, &candidate.lastAccessedAt); err != nil { + + err := rows.Scan(&key, &candidate.sizeBytes, &candidate.lastAccessedAt) + if err != nil { return nil, fmt.Errorf("failed to scan variant candidate: %w", err) } @@ -217,7 +220,8 @@ func (c *Cache) variantCandidates(ctx context.Context) ([]evictionCandidate, err candidates = append(candidates, candidate) } - if err := rows.Err(); err != nil { + err = rows.Err() + if err != nil { return nil, fmt.Errorf("variant candidate iteration failed: %w", err) } @@ -246,7 +250,9 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro var candidate evictionCandidate var hash string - if err := rows.Scan(&hash, &candidate.sizeBytes, &candidate.lastAccessedAt); err != nil { + + err := rows.Scan(&hash, &candidate.sizeBytes, &candidate.lastAccessedAt) + if err != nil { return nil, fmt.Errorf("failed to scan source candidate: %w", err) } @@ -254,7 +260,8 @@ func (c *Cache) sourceCandidates(ctx context.Context) ([]evictionCandidate, erro candidates = append(candidates, candidate) } - if err := rows.Err(); err != nil { + err = rows.Err() + if err != nil { return nil, fmt.Errorf("source candidate iteration failed: %w", err) } @@ -271,7 +278,8 @@ func (c *Cache) evictVariant(ctx context.Context, cacheKey VariantKey) error { return fmt.Errorf("failed to delete variant accounting row: %w", err) } - if err := c.variants.DeleteWithMeta(cacheKey); err != nil { + err = c.variants.DeleteWithMeta(cacheKey) + if err != nil { return err } @@ -315,17 +323,20 @@ func (c *Cache) evictSourceBlob(ctx context.Context, contentHash ContentHash) er defer func() { _ = tx.Rollback() }() - if _, err := tx.ExecContext(ctx, - `DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash)); err != nil { + _, err = tx.ExecContext(ctx, + `DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash)) + if err != nil { return fmt.Errorf("failed to delete source metadata rows: %w", err) } - if _, err := tx.ExecContext(ctx, - `DELETE FROM source_content WHERE content_hash = ?`, string(contentHash)); err != nil { + _, err = tx.ExecContext(ctx, + `DELETE FROM source_content WHERE content_hash = ?`, string(contentHash)) + if err != nil { return fmt.Errorf("failed to delete source content row: %w", err) } - if err := tx.Commit(); err != nil { + err = tx.Commit() + if err != nil { return fmt.Errorf("failed to commit eviction transaction: %w", err) } @@ -335,13 +346,15 @@ func (c *Cache) evictSourceBlob(ctx context.Context, contentHash ContentHash) er // Only after the rows are gone may the files be removed. for _, reference := range references { - if err := c.srcMetadata.Delete(reference.host, reference.pathHash); err != nil { + err := c.srcMetadata.Delete(reference.host, reference.pathHash) + if err != nil { c.log.Warn("failed to delete metadata sidecar", "host", reference.host, "path_hash", reference.pathHash, "error", err) } } - if err := c.srcContent.Delete(contentHash); err != nil { + err = c.srcContent.Delete(contentHash) + if err != nil { return err } @@ -368,7 +381,9 @@ func (c *Cache) sourceReferences( var reference sourceReference var pathHash string - if err := rows.Scan(&reference.host, &pathHash); err != nil { + + err := rows.Scan(&reference.host, &pathHash) + if err != nil { return nil, fmt.Errorf("failed to scan source reference: %w", err) } @@ -376,7 +391,8 @@ func (c *Cache) sourceReferences( references = append(references, reference) } - if err := rows.Err(); err != nil { + err = rows.Err() + if err != nil { return nil, fmt.Errorf("source reference iteration failed: %w", err) } @@ -465,7 +481,8 @@ func (c *Cache) evictionLoop(interval time.Duration) { // runEvictionPass runs one eviction pass, logging failures instead of // propagating them (the loop must keep running). func (c *Cache) runEvictionPass(ctx context.Context) { - if err := c.EvictToLimit(ctx); err != nil { + err := c.EvictToLimit(ctx) + if err != nil { c.log.Warn("cache eviction pass failed", "error", err) } } @@ -473,7 +490,8 @@ func (c *Cache) runEvictionPass(ctx context.Context) { // runReconciliationPass runs one reconciliation pass, logging failures // instead of propagating them (the loop must keep running). func (c *Cache) runReconciliationPass(ctx context.Context) { - if err := c.reconcileAccounting(ctx); err != nil { + err := c.reconcileAccounting(ctx) + if err != nil { c.log.Warn("cache accounting reconciliation failed", "error", err) } } @@ -494,19 +512,23 @@ func (c *Cache) reconcileAccounting(ctx context.Context) error { return nil } - if err := c.reconcileVariantFiles(ctx); err != nil { + err := c.reconcileVariantFiles(ctx) + if err != nil { return err } - if err := c.reconcileVariantRows(ctx); err != nil { + err = c.reconcileVariantRows(ctx) + if err != nil { return err } - if err := c.reconcileSourceFiles(ctx); err != nil { + err = c.reconcileSourceFiles(ctx) + if err != nil { return err } - if err := c.reconcileSourceRows(ctx); err != nil { + err = c.reconcileSourceRows(ctx) + if err != nil { return err } @@ -611,8 +633,9 @@ func (c *Cache) reconcileVariantRows(ctx context.Context) error { continue } - if _, err := c.db.ExecContext(ctx, - `DELETE FROM variant_content WHERE cache_key = ?`, string(key)); err != nil { + _, err := c.db.ExecContext(ctx, + `DELETE FROM variant_content WHERE cache_key = ?`, string(key)) + if err != nil { return fmt.Errorf("failed to drop stale variant accounting row: %w", err) } @@ -635,14 +658,17 @@ func (c *Cache) allVariantKeys(ctx context.Context) ([]VariantKey, error) { for rows.Next() { var key string - if err := rows.Scan(&key); err != nil { + + err := rows.Scan(&key) + if err != nil { return nil, fmt.Errorf("failed to scan variant key: %w", err) } keys = append(keys, VariantKey(key)) } - if err := rows.Err(); err != nil { + err = rows.Err() + if err != nil { return nil, fmt.Errorf("variant key iteration failed: %w", err) } @@ -693,12 +719,14 @@ func (c *Cache) removeUntrackedSourceFile( return nil } - if _, err := c.db.ExecContext(ctx, - `DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash)); err != nil { + _, err = c.db.ExecContext(ctx, + `DELETE FROM source_metadata WHERE content_hash = ?`, string(contentHash)) + if err != nil { return fmt.Errorf("failed to delete metadata rows for untracked blob: %w", err) } - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + err = os.Remove(path) + if err != nil && !os.IsNotExist(err) { return fmt.Errorf("failed to remove untracked source file: %w", err) } @@ -722,7 +750,8 @@ func (c *Cache) reconcileSourceRows(ctx context.Context) error { // The blob file is already gone; evictSourceBlob removes the // rows and sidecars and tolerates the missing file. - if err := c.evictSourceBlob(ctx, hash); err != nil { + err := c.evictSourceBlob(ctx, hash) + if err != nil { return err } @@ -745,14 +774,17 @@ func (c *Cache) allSourceContentHashes(ctx context.Context) ([]ContentHash, erro for rows.Next() { var hash string - if err := rows.Scan(&hash); err != nil { + + err := rows.Scan(&hash) + if err != nil { return nil, fmt.Errorf("failed to scan content hash: %w", err) } hashes = append(hashes, ContentHash(hash)) } - if err := rows.Err(); err != nil { + err = rows.Err() + if err != nil { return nil, fmt.Errorf("content hash iteration failed: %w", err) } @@ -771,7 +803,8 @@ func (c *Cache) sweepStaleTempFile(path string, entry fs.DirEntry) { return } - if err := os.Remove(path); err != nil && !os.IsNotExist(err) { + err = os.Remove(path) + if err != nil && !os.IsNotExist(err) { c.log.Warn("failed to remove stale temp file", "path", path, "error", err) return diff --git a/internal/imgcache/storage.go b/internal/imgcache/storage.go index 0a64bcb..893b176 100644 --- a/internal/imgcache/storage.go +++ b/internal/imgcache/storage.go @@ -66,7 +66,8 @@ func (s *ContentStorage) Store(r io.Reader) (ContentHash, int64, error) { hash := ContentHash(hex.EncodeToString(h[:])) size := int64(len(data)) - if err := s.writeIfAbsent(hash, data); err != nil { + err = s.writeIfAbsent(hash, data) + if err != nil { return "", 0, err } @@ -81,8 +82,9 @@ func (s *ContentStorage) Store(r io.Reader) (ContentHash, int64, error) { // left untouched. func (s *ContentStorage) StoreHashed( hash ContentHash, data []byte, -) (size int64, err error) { - if err := s.writeIfAbsent(hash, data); err != nil { +) (int64, error) { + err := s.writeIfAbsent(hash, data) + if err != nil { return 0, err } @@ -151,18 +153,21 @@ func (s *ContentStorage) Exists(hash ContentHash) bool { // writeIfAbsent writes data to the path derived from hash, unless // content already exists there, via a temp-file-plus-rename so // concurrent readers never observe a partial file. -func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) (err error) { +func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) error { // Build path: /// path := s.hashToPath(hash) // Check if already exists - if _, statErr := os.Stat(path); statErr == nil { + _, statErr := os.Stat(path) + if statErr == nil { return nil } // Create directory structure dir := filepath.Dir(path) - if err := os.MkdirAll(dir, StorageDirPerm); err != nil { + + err := os.MkdirAll(dir, StorageDirPerm) + if err != nil { return fmt.Errorf("failed to create directory: %w", err) } @@ -174,24 +179,32 @@ func (s *ContentStorage) writeIfAbsent(hash ContentHash, data []byte) (err error tmpPath := tmpFile.Name() - defer func() { - if err != nil { - _ = os.Remove(tmpPath) - } - }() - - if _, err := tmpFile.Write(data); err != nil { + // Each failure path below unlinks the temp file explicitly. This + // replaces a deferred cleanup that read a named result, which the + // canonical config does not permit; the set of paths that remove + // tmpPath, and the order relative to Close, is unchanged. This + // mirrors how MetadataStorage.Store and VariantStorage.Store below + // already express the same cleanup. + _, err = tmpFile.Write(data) + if err != nil { _ = tmpFile.Close() + _ = os.Remove(tmpPath) return fmt.Errorf("failed to write content: %w", err) } - if err := tmpFile.Close(); err != nil { + err = tmpFile.Close() + if err != nil { + _ = os.Remove(tmpPath) + return fmt.Errorf("failed to close temp file: %w", err) } // Atomic rename - if err := os.Rename(filepath.Clean(tmpPath), filepath.Clean(path)); err != nil { + err = os.Rename(filepath.Clean(tmpPath), filepath.Clean(path)) + if err != nil { + _ = os.Remove(tmpPath) + return fmt.Errorf("failed to rename temp file: %w", err) } @@ -547,13 +560,14 @@ func (s *VariantStorage) Delete(key VariantKey) error { // DeleteWithMeta removes the content at the given key together with // its .meta sidecar file. A missing file is not an error. func (s *VariantStorage) DeleteWithMeta(key VariantKey) error { - if err := s.Delete(key); err != nil { + err := s.Delete(key) + if err != nil { return err } metaPath := s.keyToPath(key) + ".meta" - err := os.Remove(metaPath) + err = os.Remove(metaPath) if err != nil && !os.IsNotExist(err) { return fmt.Errorf("failed to delete variant metadata: %w", err) }