Log os.Remove cleanup errors in service (0a)

The cleanup paths in writeService.UploadMedia, copyFile, and the podcast
episode download/persist flow previously swallowed os.Remove errors.
When unlink fails due to permission or I/O issues, operators had no
breadcrumb — only an already-gone file is benign. Each call now goes
through a small helper that logs a warning unless the error is
fs.ErrNotExist.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

2 files changed, 33 insertions, 9 deletions

diff --git a/player-server/internal/service/podcast_episode.go b/player-server/internal/service/podcast_episode.go
index 6b14ee5..a2ebc72 100644
--- a/player-server/internal/service/podcast_episode.go
+++ b/player-server/internal/service/podcast_episode.go
@@ -2,8 +2,10 @@ package service
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -22,6 +24,16 @@ func newPodcastEpisodeService(svc *podcastService) *podcastEpisodeService {
 	return &podcastEpisodeService{svc: svc}
 }
 
+// removeAndLog unlinks path and logs a warning if the unlink fails with
+// anything other than fs.ErrNotExist. NotExist means the file was already
+// cleaned up elsewhere, but permission/I/O errors are real signal worth
+// surfacing rather than silently dropping.
+func (s *podcastEpisodeService) removeAndLog(path string) {
+	if err := os.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) {
+		s.svc.logger.Warn("cleanup unlink failed", "path", path, "err", err)
+	}
+}
+
 // ListEpisodes returns podcast episodes visible to the user within a set.
 func (s *podcastEpisodeService) ListEpisodes(ctx context.Context, setID, userID int64, limit, offset int) ([]model.PodcastEpisodeWithStatus, error) {
 	if err := s.svc.helper.checkSetPermission(ctx, setID, userID, ""); err != nil {
@@ -152,11 +164,11 @@ func (s *podcastEpisodeService) downloadEnclosure(ctx context.Context, episode *
 	n, err := io.Copy(f, resp.Body)
 	if err != nil {
 		f.Close()
-		os.Remove(path)
+		s.removeAndLog(path)
 		return 0, fmt.Errorf("write file: %w", err)
 	}
 	if err := f.Close(); err != nil {
-		os.Remove(path)
+		s.removeAndLog(path)
 		return 0, fmt.Errorf("close file: %w", err)
 	}
 
@@ -168,7 +180,7 @@ func (s *podcastEpisodeService) downloadEnclosure(ctx context.Context, episode *
 func (s *podcastEpisodeService) persistDownloadedEpisode(ctx context.Context, episode *model.PodcastEpisode, set *model.Set, path string, n int64) (*model.Media, func(), error) {
 	relPath, err := filepath.Rel(filepath.Join(s.svc.mediaRoot, set.RootPath), path)
 	if err != nil {
-		os.Remove(path)
+		s.removeAndLog(path)
 		return nil, nil, fmt.Errorf("relative episode path: %w", err)
 	}
 	relPath = filepath.ToSlash(relPath)
@@ -183,13 +195,13 @@ func (s *podcastEpisodeService) persistDownloadedEpisode(ctx context.Context, ep
 	}
 	mediaID, err := s.svc.store.CreateMedia(ctx, media)
 	if err != nil {
-		os.Remove(path)
+		s.removeAndLog(path)
 		return nil, nil, fmt.Errorf("create media: %w", err)
 	}
 	media.ID = mediaID
 
 	cleanup := func() {
-		os.Remove(path)
+		s.removeAndLog(path)
 		_ = s.svc.store.HardDeleteMedia(ctx, media.ID)
 	}
 
diff --git a/player-server/internal/service/write.go b/player-server/internal/service/write.go
index d76dd87..04e0a28 100644
--- a/player-server/internal/service/write.go
+++ b/player-server/internal/service/write.go
@@ -5,6 +5,8 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
+	"log/slog"
 	mrand "math/rand"
 	"os"
 	"path/filepath"
@@ -18,6 +20,16 @@ import (
 	"codeberg.org/snonux/player/internal/thumb"
 )
 
+// removeAndLog unlinks path and logs a warning when the unlink fails with
+// something other than fs.ErrNotExist. NotExist is fine — the file was already
+// cleaned up elsewhere — but permission/I/O errors are real operator signal
+// that would otherwise vanish if we kept discarding os.Remove's error.
+func removeAndLog(path string) {
+	if err := os.Remove(path); err != nil && !errors.Is(err, fs.ErrNotExist) {
+		slog.Default().Warn("cleanup unlink failed", "path", path, "err", err)
+	}
+}
+
 // writeService handles mutations such as upload, soft-delete and restore.
 type writeService struct {
 	store     repository.WriteServiceStore
@@ -85,12 +97,12 @@ func (s *writeService) UploadMedia(ctx context.Context, setID, userID int64, fil
 
 	media, err := s.saveUploadedMedia(ctx, setID, path, data, size)
 	if err != nil {
-		os.Remove(path)
+		removeAndLog(path)
 		return nil, err
 	}
 
 	if err := ImportMediaFile(ctx, s.store, media, s.prober, s.thumbGen); err != nil {
-		os.Remove(path)
+		removeAndLog(path)
 		s.store.HardDeleteMedia(ctx, media.ID)
 		return nil, err
 	}
@@ -248,11 +260,11 @@ func copyFile(src, dst string) error {
 	_, copyErr := io.Copy(out, in)
 	closeErr := out.Close()
 	if copyErr != nil {
-		_ = os.Remove(dst)
+		removeAndLog(dst)
 		return copyErr
 	}
 	if closeErr != nil {
-		_ = os.Remove(dst)
+		removeAndLog(dst)
 		return fmt.Errorf("close %q: %w", dst, closeErr)
 	}
 	return nil