fix(streaming): surface why a transcode decision failed (#5820)

* fix(subsonic): surface the reason a transcode decision failed

getTranscodeDecision returned a bare "failed to make transcode decision"
with no clue why, and the probe command ran ffprobe with -v quiet, so even
the server log bottomed out at "exit status 1". A user whose files had been
moved by an external tool only saw the opaque error.

ProbeAudioStream now returns a typed ProbeError that separates the file path
from the reason: ffprobe runs with -v error so its stderr diagnostic is
captured, and a missing or unreadable file is reported as "file not found"
rather than ffprobe's misleading "Invalid data found". The handler logs the
full detail (including the path) and returns the reason to the client with
the server path stripped out.

Reported-by: Tolriq (Symfonium)

* refactor(ffmpeg): use errors.AsType for ExitError match

probeErrorReason used the older var+errors.As form while the rest of the
codebase (and its sibling transcodeFailureReason) uses the generic
errors.AsType. Switch to it for consistency; behavior is unchanged.

* fix(subsonic): return error 70 when the source file is missing

A getTranscodeDecision probe failure was always reported as generic error 0.
When the source file is gone (moved or deleted out from under the DB), that is
a not-found condition, so return the standard Subsonic error 70 ("data not
found") instead — matching what the endpoint already returns for an unknown
mediaId. Files that exist but are corrupt or unreadable stay error 0.

ProbeError now wraps the underlying cause and implements Unwrap, so the handler
detects the case with errors.Is(err, fs.ErrNotExist).

* fix(ffmpeg): keep probe error paths out of client-facing reasons

Addresses review feedback on the ProbeError type: the Reason field doubled as
both the log detail and the client message, so an ffprobe launch failure (a
*os.PathError from fork/exec) could leak the ffprobe binary path to clients,
and an unexpected stat error was reduced to "file not accessible" in the log.

Split the two concerns: Reason now holds only a path-free, client-safe string
(built at construction), while Error() logs the full underlying cause. Launch
failures return a generic "could not read file" instead of the raw exec error.
SafeReason no longer does substring path-stripping (removing the empty-Path
edge case); the stripping happens once, against ffprobe's stderr.

* fix(subsonic): don't report a broken ffprobe as a missing media file

Two issues from review of the previous commit:

Code 70 was selected with errors.Is(err, fs.ErrNotExist), but a launch failure
of a deleted ffprobe binary is an *os.PathError that also wraps fs.ErrNotExist.
A server-side ffprobe problem was therefore reported to clients as a missing
media file. ProbeError now carries an explicit NotFound flag, set only on the
file-access branch, and the handler keys the code off that instead of the chain.

ffprobe can also exit 0 while yielding no audio stream (an audio-suffixed
container holding only video). That parse failure was returned unwrapped, so
clients got "internal error"; it is now wrapped in a ProbeError too.

core/ffmpeg/ffmpeg.go

@@ -7,6 +7,7 @@ import (
 	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -67,7 +68,7 @@ var ErrAnimatedWebPUnsupported = errors.New("ffmpeg lacks libwebp_anim encoder
 const (
 	extractImageCmd     = "ffmpeg -i %s -map 0:v -map -0:V -vcodec copy -f image2pipe -"
 	probeCmd            = "ffmpeg %s -f ffmetadata"
-	probeAudioStreamCmd = "ffprobe -v quiet -select_streams a:0 -print_format json -show_streams -show_format %s"
+	probeAudioStreamCmd = "ffprobe -v error -select_streams a:0 -print_format json -show_streams -show_format %s"
 )
 
 type ffmpeg struct{}
@@ -159,16 +160,80 @@ func (e *ffmpeg) ProbeAudioStream(ctx context.Context, filePath string) (*AudioP
 		return nil, err
 	}
 	if err := fileExists(filePath); err != nil {
-		return nil, err
+		return nil, &ProbeError{Path: filePath, Reason: fileAccessReason(err),
+			NotFound: errors.Is(err, fs.ErrNotExist), err: err}
 	}
 	args := createFFmpegCommand(probeAudioStreamCmd, filePath, 0, 0)
 	log.Trace(ctx, "Executing ffprobe command", "args", args)
 	cmd := exec.CommandContext(ctx, args[0], args[1:]...) // #nosec
 	output, err := cmd.Output()
 	if err != nil {
-		return nil, fmt.Errorf("running ffprobe on %q: %w", filePath, err)
+		return nil, &ProbeError{Path: filePath, Reason: probeClientReason(err, filePath), err: err}
 	}
-	return parseProbeOutput(output)
+	result, err := parseProbeOutput(output)
+	if err != nil {
+		return nil, &ProbeError{Path: filePath, Reason: err.Error(), err: err}
+	}
+	return result, nil
+}
+
+// ProbeError reports an ffprobe failure. Reason is a path-free message safe to
+// expose to clients; the wrapped cause carries the full detail for logging.
+// NotFound marks the media file itself as missing — a launch failure of a
+// deleted ffprobe binary also wraps fs.ErrNotExist, so callers must not infer
+// it from the error chain.
+type ProbeError struct {
+	Path     string
+	Reason   string
+	NotFound bool
+	err      error
+}
+
+func (e *ProbeError) Error() string {
+	if e.err == nil {
+		return fmt.Sprintf("probe failed on %q: %s", e.Path, e.Reason)
+	}
+	return fmt.Sprintf("probe failed on %q: %s", e.Path, probeDetail(e.err))
+}
+
+// Unwrap exposes the underlying cause so callers can test it with errors.Is
+// (e.g. fs.ErrNotExist to detect a missing file).
+func (e *ProbeError) Unwrap() error { return e.err }
+
+// SafeReason returns the path-free reason, safe to send to clients.
+func (e *ProbeError) SafeReason() string { return e.Reason }
+
+// fileAccessReason maps a stat failure to a clear, path-free reason, so a moved
+// or unreadable file reads as "file not found" rather than a raw ffprobe message.
+func fileAccessReason(err error) string {
+	switch {
+	case errors.Is(err, fs.ErrNotExist):
+		return "file not found"
+	case errors.Is(err, fs.ErrPermission):
+		return "permission denied"
+	default:
+		return "file not accessible"
+	}
+}
+
+// probeDetail returns the full diagnostic for logging (may contain paths):
+// ffprobe's stderr when present, otherwise the raw error text.
+func probeDetail(err error) string {
+	if exitErr, ok := errors.AsType[*exec.ExitError](err); ok && len(exitErr.Stderr) > 0 {
+		return strings.TrimSpace(string(exitErr.Stderr))
+	}
+	return err.Error()
+}
+
+// probeClientReason returns a path-free reason for an ffprobe execution failure:
+// ffprobe's stderr with the file path stripped, or a generic reason when ffprobe
+// couldn't run at all (its launch error may embed the binary path).
+func probeClientReason(err error, path string) string {
+	exitErr, ok := errors.AsType[*exec.ExitError](err)
+	if !ok || len(exitErr.Stderr) == 0 {
+		return "could not read file"
+	}
+	return strings.TrimSpace(strings.ReplaceAll(string(exitErr.Stderr), path, "the file"))
 }
 
 type probeOutput struct {

core/ffmpeg/ffmpeg_test.go

@@ -2,6 +2,7 @@ package ffmpeg
 
 import (
 	"context"
+	"errors"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -553,6 +554,65 @@ var _ = Describe("ffmpeg", func() {
 		})
 	})
 
+	Describe("ProbeError", func() {
+		It("uses the underlying cause in Error() so logs keep the full detail", func() {
+			e := &ProbeError{Path: "/music/foo.flac",
+				err: errors.New("/music/foo.flac: Invalid data found when processing input")}
+			Expect(e.Error()).To(ContainSubstring("/music/foo.flac"))
+			Expect(e.Error()).To(ContainSubstring("Invalid data found when processing input"))
+		})
+
+		It("returns the path-free reason from SafeReason()", func() {
+			e := &ProbeError{Path: "/music/foo.flac", Reason: "the file: Invalid data found when processing input"}
+			Expect(e.SafeReason()).To(Equal("the file: Invalid data found when processing input"))
+			Expect(e.SafeReason()).ToNot(ContainSubstring("/music/foo.flac"))
+		})
+
+		It("unwraps to the underlying cause so errors.Is detects a missing file", func() {
+			e := &ProbeError{Path: "/music/foo.flac", Reason: "file not found", err: os.ErrNotExist}
+			Expect(errors.Is(e, os.ErrNotExist)).To(BeTrue())
+		})
+	})
+
+	Describe("probeClientReason", func() {
+		It("strips the file path from ffprobe stderr", func() {
+			if runtime.GOOS == "windows" {
+				Skip("uses /bin/sh")
+			}
+			_, err := exec.Command("/bin/sh", "-c", "echo '/music/foo.flac: Invalid data found' >&2; exit 1").Output()
+			Expect(err).To(HaveOccurred())
+			Expect(probeClientReason(err, "/music/foo.flac")).To(Equal("the file: Invalid data found"))
+		})
+
+		It("returns a generic reason for launch failures, without leaking the binary path", func() {
+			err := errors.New("fork/exec /opt/navidrome/bin/ffprobe: no such file or directory")
+			Expect(probeClientReason(err, "/music/foo.flac")).To(Equal("could not read file"))
+		})
+	})
+
+	Describe("probeDetail", func() {
+		It("surfaces ffprobe stderr for logging", func() {
+			if runtime.GOOS == "windows" {
+				Skip("uses /bin/sh")
+			}
+			_, err := exec.Command("/bin/sh", "-c", "echo 'boom detail' >&2; exit 1").Output()
+			Expect(err).To(HaveOccurred())
+			Expect(probeDetail(err)).To(Equal("boom detail"))
+		})
+	})
+
+	Describe("fileAccessReason", func() {
+		It("reports a missing file as 'file not found', not a raw stat message", func() {
+			_, err := os.Stat("/no/such/dir/really-missing.flac")
+			Expect(err).To(HaveOccurred())
+			Expect(fileAccessReason(err)).To(Equal("file not found"))
+		})
+
+		It("falls back to a generic reason for other access errors", func() {
+			Expect(fileAccessReason(errors.New("boom"))).To(Equal("file not accessible"))
+		})
+	})
+
 	Describe("FFmpeg", func() {
 		Context("when FFmpeg is available", func() {
 			var ff FFmpeg
@@ -566,6 +626,16 @@ var _ = Describe("ffmpeg", func() {
 				}
 			})
 
+			It("ProbeAudioStream returns a not-found ProbeError for a missing file", func() {
+				_, err := ff.ProbeAudioStream(GinkgoT().Context(), "/no/such/dir/really-missing.flac")
+				Expect(err).To(HaveOccurred())
+				Expect(errors.Is(err, os.ErrNotExist)).To(BeTrue())
+				var pe *ProbeError
+				Expect(errors.As(err, &pe)).To(BeTrue())
+				Expect(pe.SafeReason()).To(Equal("file not found"))
+				Expect(pe.NotFound).To(BeTrue())
+			})
+
 			It("should interrupt transcoding when context is cancelled", func() {
 				ctx, cancel := context.WithTimeout(GinkgoT().Context(), 5*time.Second)
 				defer cancel()

server/subsonic/transcode.go

@@ -8,6 +8,7 @@ import (
 	"slices"
 	"strconv"
 
+	"github.com/navidrome/navidrome/core/ffmpeg"
 	"github.com/navidrome/navidrome/core/stream"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
@@ -315,7 +316,8 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request)
 	decision, err := api.transcodeDecision.MakeDecision(ctx, mf, clientInfo, stream.TranscodeOptions{})
 	if err != nil {
 		log.Error(ctx, "Failed to make transcode decision", "mediaID", mediaID, err)
-		return nil, newError(responses.ErrorGeneric, "failed to make transcode decision")
+		code, reason := transcodeFailure(err)
+		return nil, newError(code, "failed to make transcode decision: %s", reason)
 	}
 
 	// Only create a token when there is a valid playback path
@@ -346,6 +348,19 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request)
 	return response, nil
 }
 
+// transcodeFailure maps a decision error to a Subsonic error code and a reason
+// safe to send to clients, omitting server file paths.
+func transcodeFailure(err error) (int32, string) {
+	pe, ok := errors.AsType[*ffmpeg.ProbeError](err)
+	if !ok {
+		return responses.ErrorGeneric, "internal error"
+	}
+	if pe.NotFound {
+		return responses.ErrorDataNotFound, pe.SafeReason()
+	}
+	return responses.ErrorGeneric, pe.SafeReason()
+}
+
 // GetTranscodeStream handles the OpenSubsonic getTranscodeStream endpoint.
 // It streams media using the decision encoded in the transcodeParams JWT token.
 // All errors are returned as proper HTTP status codes (not Subsonic error responses).

server/subsonic/transcode_test.go

@@ -4,12 +4,16 @@ import (
 	"bytes"
 	"context"
 	"errors"
+	"fmt"
+	"io/fs"
 	"net/http"
 	"net/http/httptest"
 
+	"github.com/navidrome/navidrome/core/ffmpeg"
 	"github.com/navidrome/navidrome/core/stream"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/model/request"
+	"github.com/navidrome/navidrome/server/subsonic/responses"
 	"github.com/navidrome/navidrome/tests"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
@@ -77,6 +81,47 @@ var _ = Describe("Transcode endpoints", func() {
 			Expect(err.Error()).To(ContainSubstring("error retrieving media file"))
 		})
 
+		It("enriches the decision error with the reason, without leaking the file path", func() {
+			mockMFRepo.SetData(model.MediaFiles{{ID: "song-1", Suffix: "flac"}})
+			mockTD.decisionErr = fmt.Errorf("probing media file song-1: %w",
+				&ffmpeg.ProbeError{Path: "/music/secret/foo.flac", Reason: "the file: Invalid data found when processing input"})
+			r := newJSONPostRequest("mediaId=song-1&mediaType=song", "{}")
+			_, err := router.GetTranscodeDecision(w, r)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("failed to make transcode decision"))
+			Expect(err.Error()).To(ContainSubstring("Invalid data found when processing input"))
+			Expect(err.Error()).ToNot(ContainSubstring("/music/secret"))
+			var subErr subError
+			Expect(errors.As(err, &subErr)).To(BeTrue())
+			Expect(subErr.code).To(Equal(responses.ErrorGeneric))
+		})
+
+		It("returns ErrorDataNotFound when the source file is missing on disk", func() {
+			mockMFRepo.SetData(model.MediaFiles{{ID: "song-1", Suffix: "flac"}})
+			mockTD.decisionErr = fmt.Errorf("probing media file song-1: %w",
+				&ffmpeg.ProbeError{Path: "/music/gone.flac", Reason: "file not found", NotFound: true})
+			r := newJSONPostRequest("mediaId=song-1&mediaType=song", "{}")
+			_, err := router.GetTranscodeDecision(w, r)
+			Expect(err).To(HaveOccurred())
+			Expect(err.Error()).To(ContainSubstring("file not found"))
+			var subErr subError
+			Expect(errors.As(err, &subErr)).To(BeTrue())
+			Expect(subErr.code).To(Equal(responses.ErrorDataNotFound))
+		})
+
+		It("keeps ErrorGeneric when ffprobe is missing, even though the cause wraps fs.ErrNotExist", func() {
+			mockMFRepo.SetData(model.MediaFiles{{ID: "song-1", Suffix: "flac"}})
+			pe := &ffmpeg.ProbeError{Path: "/music/song.flac", Reason: "could not read file"}
+			mockTD.decisionErr = fmt.Errorf("probing media file song-1: %w (%w)", pe, fs.ErrNotExist)
+			Expect(errors.Is(mockTD.decisionErr, fs.ErrNotExist)).To(BeTrue())
+			r := newJSONPostRequest("mediaId=song-1&mediaType=song", "{}")
+			_, err := router.GetTranscodeDecision(w, r)
+			Expect(err).To(HaveOccurred())
+			var subErr subError
+			Expect(errors.As(err, &subErr)).To(BeTrue())
+			Expect(subErr.code).To(Equal(responses.ErrorGeneric))
+		})
+
 		It("returns error when body is empty", func() {
 			r := newJSONPostRequest("mediaId=song-1&mediaType=song", "")
 			_, err := router.GetTranscodeDecision(w, r)
@@ -516,6 +561,7 @@ func newJSONPostRequest(queryParams string, jsonBody string) *http.Request {
 // mockTranscodeDecision is a test double for stream.TranscodeDecider
 type mockTranscodeDecision struct {
 	decision       *stream.TranscodeDecision
+	decisionErr    error
 	token          string
 	tokenErr       error
 	resolvedReq    stream.Request
@@ -525,6 +571,9 @@ type mockTranscodeDecision struct {
 
 func (m *mockTranscodeDecision) MakeDecision(_ context.Context, _ *model.MediaFile, ci *stream.ClientInfo, _ stream.TranscodeOptions) (*stream.TranscodeDecision, error) {
 	m.capturedClient = ci
+	if m.decisionErr != nil {
+		return nil, m.decisionErr
+	}
 	if m.decision != nil {
 		return m.decision, nil
 	}