navidrome/server/subsonic/stream.go
Deluan Quintão 3f7226d253
fix(server): improve transcoding failure diagnostics and error responses (#5227)
* fix(server): capture ffmpeg stderr and warn on empty transcoded output

When ffmpeg fails during transcoding (e.g., missing codec like libopus),
the error was silently discarded because stderr was sent to io.Discard
and the HTTP response returned 200 OK with a 0-byte body.

- Capture ffmpeg stderr in a bounded buffer (4KB) and include it in the
  error message when the process exits with a non-zero status code
- Log a warning when transcoded output is 0 bytes, guiding users to
  check codec support and enable Trace logging for details
- Remove log level guard so transcoding errors are always logged, not
  just at Debug level

Signed-off-by: Deluan <deluan@navidrome.org>

* fix(server): return proper error responses for empty transcoded output

Instead of returning HTTP 200 with 0-byte body when transcoding fails,
return a Subsonic error response (for stream/download/getTranscodeStream)
or HTTP 500 (for public shared streams). This gives clients a clear
signal that the request failed rather than a misleading empty success.

Signed-off-by: Deluan <deluan@navidrome.org>

* test(e2e): add tests for empty transcoded stream error responses

Add E2E tests verifying that stream and download endpoints return
Subsonic error responses when transcoding produces empty output.
Extend spyStreamer with SimulateEmptyStream and SimulateError fields
to support failure injection in tests.

Signed-off-by: Deluan <deluan@navidrome.org>

* refactor(server): extract stream serving logic into Stream.Serve method

Extract the duplicated non-seekable stream serving logic (header setup,
estimateContentLength, HEAD draining, io.Copy with error/empty detection)
from server/subsonic/stream.go and server/public/handle_streams.go into a
single Stream.Serve method on core/stream. Both callers now delegate to it,
eliminating ~30 lines of near-identical code.

* fix(server): return 200 with empty body for stream/download on empty transcoded output

Don't return a Subsonic error response when transcoding produces empty
output on stream/download endpoints — just log the error and return 200
with an empty body. The getTranscodeStream and public share endpoints
still return HTTP 500 for empty output. Stream.Serve now returns
(int64, error) so callers can check the byte count.

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-03-18 12:39:03 -04:00

132 lines
3.7 KiB
Go

package subsonic
import (
"fmt"
"net/http"
"strconv"
"strings"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/server/subsonic/responses"
"github.com/navidrome/navidrome/utils/req"
)
func (api *Router) Stream(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
ctx := r.Context()
p := req.Params(r)
id, err := p.String("id")
if err != nil {
return nil, err
}
maxBitRate := p.IntOr("maxBitRate", 0)
format, _ := p.String("format")
timeOffset := p.IntOr("timeOffset", 0)
mf, err := api.ds.MediaFile(ctx).Get(id)
if err != nil {
return nil, err
}
streamReq := api.transcodeDecision.ResolveRequest(ctx, mf, format, maxBitRate, timeOffset)
stream, err := api.streamer.NewStream(ctx, mf, streamReq)
if err != nil {
return nil, err
}
// Make sure the stream will be closed at the end, to avoid leakage
defer func() {
if err := stream.Close(); err != nil && log.IsGreaterOrEqualTo(log.LevelDebug) {
log.Error("Error closing stream", "id", id, "file", stream.Name(), err)
}
}()
w.Header().Set("X-Content-Type-Options", "nosniff")
w.Header().Set("X-Content-Duration", strconv.FormatFloat(float64(stream.Duration()), 'G', -1, 32))
_, err = stream.Serve(ctx, w, r)
return nil, err
}
func (api *Router) Download(w http.ResponseWriter, r *http.Request) (*responses.Subsonic, error) {
ctx := r.Context()
username, _ := request.UsernameFrom(ctx)
p := req.Params(r)
id, err := p.String("id")
if err != nil {
return nil, err
}
if !conf.Server.EnableDownloads {
log.Warn(ctx, "Downloads are disabled", "user", username, "id", id)
return nil, newError(responses.ErrorAuthorizationFail, "downloads are disabled")
}
entity, err := model.GetEntityByID(ctx, api.ds, id)
if err != nil {
return nil, err
}
maxBitRate := p.IntOr("bitrate", 0)
format, _ := p.String("format")
if format == "" {
if conf.Server.AutoTranscodeDownload {
// if we are not provided a format, see if we have requested transcoding for this client
// This must be enabled via a config option. For the UI, we are always given an option.
// This will impact other clients which do not use the UI
transcoding, ok := request.TranscodingFrom(ctx)
if !ok {
format = "raw"
} else {
format = transcoding.TargetFormat
maxBitRate = transcoding.DefaultBitRate
}
} else {
format = "raw"
}
}
setHeaders := func(name string) {
name = strings.ReplaceAll(name, ",", "_")
disposition := fmt.Sprintf("attachment; filename=\"%s.zip\"", name)
w.Header().Set("Content-Disposition", disposition)
w.Header().Set("Content-Type", "application/zip")
}
switch v := entity.(type) {
case *model.MediaFile:
streamReq := api.transcodeDecision.ResolveRequest(ctx, v, format, maxBitRate, 0)
stream, err := api.streamer.NewStream(ctx, v, streamReq)
if err != nil {
return nil, err
}
// Make sure the stream will be closed at the end, to avoid leakage
defer func() {
if err := stream.Close(); err != nil && log.IsGreaterOrEqualTo(log.LevelDebug) {
log.Error("Error closing stream", "id", id, "file", stream.Name(), err)
}
}()
disposition := fmt.Sprintf("attachment; filename=\"%s\"", stream.Name())
w.Header().Set("Content-Disposition", disposition)
_, err = stream.Serve(ctx, w, r)
return nil, err
case *model.Album:
setHeaders(v.Name)
return nil, api.archiver.ZipAlbum(ctx, id, format, maxBitRate, w)
case *model.Artist:
setHeaders(v.Name)
return nil, api.archiver.ZipArtist(ctx, id, format, maxBitRate, w)
case *model.Playlist:
setHeaders(v.Name)
return nil, api.archiver.ZipPlaylist(ctx, id, format, maxBitRate, w)
default:
return nil, model.ErrNotFound
}
}