Files

Jake b48d39bfb1 fix(rb-09): decoder propagates read and decode errors

decode_audio_file's packet loop was `Err(_) => break`, so any non-EOF
read error during playback dropped out silently with whatever samples
had accumulated. Per-packet decode errors were tallied and skipped,
contributing to the same outcome. A corrupt or truncated input
therefore came back as `Ok(partial_samples)` — no way for callers to
distinguish a clean decode from a compromised one.

Every SymphoniaError other than the explicit EOF
(`IoError(UnexpectedEof)`) now maps to `AudioDecodeFailed`. Decoder
errors bubble via `?` rather than being counted. `ResetRequired`
promotes to an error rather than a silent break.

Extracted an internal `decode_media_stream(mss, hint)` so tests can
inject a custom `MediaSource`. Added `FlakyCursor` — a seekable cursor
that returns a synthetic I/O error after N bytes — and a regression
test that confirms mid-stream read failure surfaces as `Err` instead of
returning partial audio. Happy-path and missing-file tests added for
coverage.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

2026-04-22 10:24:18 +01:00

2.2 KiB

Raw Blame History

RB-09 MAJOR: decoder returns partial audio on read/decode errors

Severity: MAJOR Path: crates/audio/src/decode.rs:58-79 Source: 2026-04-22 code review Labels: release-blocker, major, audio, data-integrity Status: RESOLVED (2026-04-22)

Resolution

decode_audio_file now propagates every SymphoniaError other than the explicit end-of-stream UnexpectedEof:

SymphoniaError::ResetRequired → error (mid-stream discontinuity).
Any other packet-read error → KonError::AudioDecodeFailed.
decoder.decode(&packet) errors → bubble via ? instead of counter-then-skip.

The decode logic was refactored into an internal decode_media_stream(mss, hint) so tests can inject a custom MediaSource. The regression test FlakyCursor returns a valid WAV header followed by an injected io::Error after 1024 bytes; the mid_stream_io_error_propagates_instead_of_returning_partial_audio test asserts the caller receives Err, not an Ok with a truncated samples vector. Companion tests cover the happy path and the file-does-not-exist path.

The optional decode_audio_file_best_effort variant suggested in the original issue was not added — no caller needs it today.

Problem

decode_audio_file:

Breaks the read loop on packet-read errors (truncated / corrupt inputs)
Counts and skips per-packet decoder errors
Still returns Ok if any samples were produced before the break

A corrupt or truncated input file is silently accepted as partial audio. Callers have no way to distinguish "file decoded cleanly" from "file was bad and we handed you half of it".

Acceptance

Propagate read and decode errors to the caller (return Err) — match the pattern used in read_wav (fixed in the 2026-04-22 quick-wins batch, commit b665754).
Optional: expose a decode_audio_file_best_effort variant if anyone genuinely wants the partial-audio-on-error behaviour. Today no caller needs it.
Regression tests: (a) truncated MP3; (b) corrupted FLAC; (c) valid file continues to decode successfully.

Fix scope

Medium. Error-propagation pattern is the same as the read_wav fix, but the symphonia packet-loop has several skip branches to audit.

Dependencies

None — standalone fix.

2.2 KiB Raw Blame History