navidrome/core/stream/media_streamer_test.go
Deluan Quintão 97da9993d7
fix(stream): abort the response when a transcoded stream is truncated (#6035)
* fix(stream): abort the response when a transcoded stream is truncated

When a transcode failed after some audio had already been sent, Serve logged
the error and returned nil, so Go finished the chunked body normally and the
client received an apparently complete, silently short file. Symfonium users
hit this on large offline syncs, and the worst path, ffmpeg dying mid-write
behind the transcoding cache, produced no error and nothing in the log above
Debug: the cache writer was closed plainly, so readers drained the truncated
entry to a clean EOF.

The root cause of that silence is an fscache limitation: Close is the only way
to end a cache write, and Close always means "complete". This adopts the
deluan/fscache fork, which adds CloseWithError: on failure copyAndClose now
cancels the entry with the cause, so every attached reader fails mid-read with
the real error instead of EOF, a late Get for the entry is refused, and the
entry never reports a final size. The error travels inside the entry each
reader holds, which makes per-generation delivery automatic and needs no
bookkeeping on our side.

With the failure arriving in-band, one change in Serve covers every mode: an
io.Copy error after bytes are on the wire panics with http.ErrAbortHandler.
Go aborts the response without the terminating chunk (RST_STREAM on HTTP/2),
chi's Recoverer re-panics that value, and the deferred stream.Close() still
runs, so the transcode limiter slot is released as before.

Two behaviors improve as side effects. A transcoder that dies before its first
byte now yields a Subsonic error response instead of a 200 with an empty body,
since the failure reaches Serve as an error while the status is still
unsent; genuinely empty output (clean EOF, exit 0) keeps the 200. And a failed
entry's invalidation no longer defers its unlink past a replacement entry
re-creating the same file, because canceling already closed its readers.

* fix(cache): warn when the cache writer cannot report failures to readers

The CloseWithError capability comes from the fscache fork via a go.mod
replace directive, and a type assertion picks it up. If that directive is
ever lost, the assertion fails silently, readers of a dead writer go back to
draining a truncated entry to a clean EOF, and nothing says so.

Two layers against that: a warning on the failure path when the writer lacks
the capability, and a test that asserts the writer fscache returns carries
it, so losing the fork fails CI instead of a listener's download.

* build: point the fscache replace at the fork's master

deluan/fscache#1 is merged; pin the merge commit instead of the review
branch. Pinned by sha because the module proxy still resolves the fork's
master ref to its pre-merge commit.

* build: reference the upstream fscache PR in the replace comment

The replace itself must keep pointing at the fork: the commit only exists in
djherbis/fscache under refs/pull/22/head, which the Go module fetcher cannot
resolve (verified: unknown revision for both short and full sha). The same
commit is advertised on the fork's master, so that is the fetchable source.
2026-08-25 18:48:43 -04:00

195 lines
7.0 KiB
Go

package stream_test
import (
"bytes"
"context"
"errors"
"io"
"net/http"
"net/http/httptest"
"os"
"testing/iotest"
"time"
"github.com/go-chi/chi/v5/middleware"
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/core/stream"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/request"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("MediaStreamer", func() {
var streamer stream.MediaStreamer
var ds model.DataStore
ffmpeg := tests.NewMockFFmpeg("fake data")
ctx := log.NewContext(context.TODO())
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
cacheDir, _ := os.MkdirTemp("", "file_caches")
conf.Server.CacheFolder = conf.NewDir(cacheDir)
conf.Server.TranscodingCacheSize = "100MB"
ds = &tests.MockDataStore{MockedTranscoding: &tests.MockTranscodingRepo{}}
ds.MediaFile(ctx).(*tests.MockMediaFileRepo).SetData(model.MediaFiles{
{ID: "123", Path: "tests/fixtures/test.mp3", Suffix: "mp3", BitRate: 128, Duration: 257.0},
})
testCache := stream.NewTranscodingCache()
Eventually(func() bool { return testCache.Available(context.TODO()) }, 10*time.Second).Should(BeTrue())
streamer = stream.NewMediaStreamer(ds, ffmpeg, testCache)
})
AfterEach(func() {
_ = os.RemoveAll(conf.Server.CacheFolder.String())
})
Context("NewStream", func() {
var mf *model.MediaFile
BeforeEach(func() {
var err error
mf, err = ds.MediaFile(ctx).Get("123")
Expect(err).ToNot(HaveOccurred())
})
It("returns a seekable stream if format is 'raw'", func() {
s, err := streamer.NewStream(ctx, mf, stream.Request{Format: "raw"})
Expect(err).ToNot(HaveOccurred())
Expect(s.Seekable()).To(BeTrue())
})
It("returns a seekable stream if no format is specified (direct play)", func() {
s, err := streamer.NewStream(ctx, mf, stream.Request{})
Expect(err).ToNot(HaveOccurred())
Expect(s.Seekable()).To(BeTrue())
})
It("returns a NON seekable stream if transcode is required", func() {
s, err := streamer.NewStream(ctx, mf, stream.Request{Format: "mp3", BitRate: 64})
Expect(err).To(BeNil())
Expect(s.Seekable()).To(BeFalse())
Expect(s.Duration()).To(Equal(float32(257.0)))
})
It("rejects transcode requests beyond MaxConcurrent with ErrTooManyTranscodes", func() {
// Use an ffmpeg whose Read blocks indefinitely so the cache's
// background copy can't drain the source and release the slot —
// keeping the single transcode slot pinned for this test.
pr, pw := io.Pipe()
DeferCleanup(func() { _ = pw.Close() })
blockingFFmpeg := tests.NewMockFFmpeg("")
blockingFFmpeg.Reader = pr
conf.Server.Transcoding.MaxConcurrent = 1
conf.Server.Transcoding.MaxConcurrentPerUser = 0
tightCache := stream.NewTranscodingCache()
Eventually(func() bool { return tightCache.Available(context.TODO()) }, 10*time.Second).Should(BeTrue())
tightStreamer := stream.NewMediaStreamer(ds, blockingFFmpeg, tightCache)
userCtx := request.WithUsername(ctx, "alice")
s1, err := tightStreamer.NewStream(userCtx, mf, stream.Request{Format: "mp3", BitRate: 64})
Expect(err).ToNot(HaveOccurred())
defer s1.Close()
// Different cache key so it doesn't dedupe with the first request.
_, err = tightStreamer.NewStream(userCtx, mf, stream.Request{Format: "mp3", BitRate: 96})
Expect(errors.Is(err, stream.ErrTooManyTranscodes)).To(BeTrue())
})
It("releases the slot once the stream is closed", func() {
conf.Server.Transcoding.MaxConcurrent = 1
conf.Server.Transcoding.MaxConcurrentPerUser = 0
tightCache := stream.NewTranscodingCache()
Eventually(func() bool { return tightCache.Available(context.TODO()) }, 10*time.Second).Should(BeTrue())
tightStreamer := stream.NewMediaStreamer(ds, ffmpeg, tightCache)
userCtx := request.WithUsername(ctx, "alice")
s1, err := tightStreamer.NewStream(userCtx, mf, stream.Request{Format: "mp3", BitRate: 64})
Expect(err).ToNot(HaveOccurred())
_, _ = io.ReadAll(s1)
_ = s1.Close()
Eventually(func() bool { return ffmpeg.IsClosed() }, "3s").Should(BeTrue())
// Slot should now be free for a different transcode.
s2, err := tightStreamer.NewStream(userCtx, mf, stream.Request{Format: "mp3", BitRate: 96})
Expect(err).ToNot(HaveOccurred())
defer s2.Close()
})
It("does not consume a slot for raw streams", func() {
conf.Server.Transcoding.MaxConcurrent = 1
conf.Server.Transcoding.MaxConcurrentPerUser = 0
tightCache := stream.NewTranscodingCache()
Eventually(func() bool { return tightCache.Available(context.TODO()) }, 10*time.Second).Should(BeTrue())
tightStreamer := stream.NewMediaStreamer(ds, ffmpeg, tightCache)
userCtx := request.WithUsername(ctx, "alice")
// First, saturate the single transcode slot.
s1, err := tightStreamer.NewStream(userCtx, mf, stream.Request{Format: "mp3", BitRate: 64})
Expect(err).ToNot(HaveOccurred())
defer s1.Close()
// Raw stream must still succeed.
s2, err := tightStreamer.NewStream(userCtx, mf, stream.Request{Format: "raw"})
Expect(err).ToNot(HaveOccurred())
defer s2.Close()
})
It("returns a seekable stream if the file is complete in the cache", func() {
s, err := streamer.NewStream(ctx, mf, stream.Request{Format: "mp3", BitRate: 32})
Expect(err).To(BeNil())
_, _ = io.ReadAll(s)
_ = s.Close()
Eventually(func() bool { return ffmpeg.IsClosed() }, "3s").Should(BeTrue())
s, err = streamer.NewStream(ctx, mf, stream.Request{Format: "mp3", BitRate: 32})
Expect(err).To(BeNil())
Expect(s.Seekable()).To(BeTrue())
})
})
Context("Serve", func() {
var mf *model.MediaFile
BeforeEach(func() {
var err error
mf, err = ds.MediaFile(ctx).Get("123")
Expect(err).ToNot(HaveOccurred())
})
It("keeps empty output a non-error, so callers still reply 200 with an empty body", func() {
s := stream.NewStream(mf, "mp3", 128, io.NopCloser(bytes.NewReader(nil)))
w := httptest.NewRecorder()
r := httptest.NewRequest(http.MethodGet, "/", nil)
n, err := s.Serve(ctx, w, r)
Expect(err).ToNot(HaveOccurred())
Expect(n).To(BeZero())
Expect(w.Code).To(Equal(http.StatusOK))
})
It("aborts the response when the source fails after sending data", func() {
src := io.NopCloser(io.MultiReader(
bytes.NewReader(bytes.Repeat([]byte("a"), 64*1024)),
iotest.ErrReader(errors.New("transcoder died")),
))
server := httptest.NewServer(serveHandler(stream.NewStream(mf, "mp3", 128, src)))
DeferCleanup(server.Close)
resp, err := http.Get(server.URL)
Expect(err).ToNot(HaveOccurred())
defer resp.Body.Close()
// A client-side read failure is the only observable proof the response was aborted.
_, err = io.ReadAll(resp.Body)
Expect(err).To(HaveOccurred())
})
})
})
// Serve runs behind the real server's Recoverer, which must let ErrAbortHandler through.
func serveHandler(s *stream.Stream) http.Handler {
return middleware.Recoverer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
_, _ = s.Serve(r.Context(), w, r)
}))
}