fix(errors): strip ffmpeg's banner so the failure is the message (#1311)

* fix(errors): strip ffmpeg's banner so the failure is the message

ffmpeg and ffprobe print a version + configuration banner to stderr on every
invocation, before doing any work. When a command fails we capture that stderr
and it becomes the error, so #1309's reporter was shown several hundred
characters of build flags — "ffmpeg version N-125781-gacf6b520c1-20260727 …
--pkg-config-flags=--static --enable-gpl …" — and not one word about why the
extract failed. The diagnosis is always AFTER the banner.

Stripped centrally in build_failure() rather than at the extract call site:
every stage that shells out to ffmpeg (dub prep, export, retime, media probe)
captures the same stderr and had the same problem.

Done before classify() runs, too — matching docs topics against a build
configuration string is how a real topic gets missed.

A message that is ONLY a banner keeps the banner: unhelpful beats empty.

Closes #1309

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

* test(errors): assert the banner is GONE, not just that the error is present

CodeRabbit: the classification test only checked that the post-banner text
appeared in `reason` — which was true before the fix too, since the banner was
simply prepended to it. It passed against the code it was written to catch.

Now asserts the banner markers are absent and that `reason` STARTS with the
real error, since burying the diagnosis after 300 characters of build flags is
the actual user complaint. Fails without strip_ffmpeg_banner().

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

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
Palash Debnath
2026-07-29 15:02:02 -07:00
committed by GitHub
co-authored by Claude Opus 5
parent b4fc74fa65
commit 42d017ef9b
2 changed files with 141 additions and 0 deletions
+44
View File
@@ -430,6 +430,46 @@ def classify(reason: str) -> str:
return ""
# ffmpeg/ffprobe write a version banner to STDERR on every invocation, before
# doing any work. When a command fails we capture that stderr and it becomes
# the error message — so the user is shown the build configuration of their
# ffmpeg instead of what went wrong (#1309: a dub extract failure reported as
# "extract: ffmpeg version N-125781-gacf6b520c1-20260727 Copyright (c) …").
#
# The real diagnosis is always AFTER the banner. Strip the boilerplate so the
# message starts at the first line that is actually about this run.
_FFMPEG_BANNER_START = re.compile(r"^\s*(ffmpeg|ffprobe)\s+version\s", re.IGNORECASE)
_FFMPEG_BANNER_CONT = re.compile(
r"^\s+(built with|configuration:|lib[a-z]+\s+\d)", re.IGNORECASE
)
def strip_ffmpeg_banner(text: Optional[str]) -> str:
"""Drop ffmpeg's version/configuration preamble from a captured stderr.
Returns the text unchanged when no banner is present, and — importantly —
when stripping would leave nothing: a message that is *only* a banner is
unhelpful, but an empty one is worse.
"""
if not text:
return text or ""
lines = str(text).splitlines()
kept, in_banner = [], False
for line in lines:
if _FFMPEG_BANNER_START.match(line):
in_banner = True
continue
if in_banner:
# The banner is the version line plus its indented continuation
# block; the first line that is not part of that ends it.
if not line.strip() or _FFMPEG_BANNER_CONT.match(line):
continue
in_banner = False
kept.append(line)
out = "\n".join(kept).strip()
return out or str(text).strip()
def sanitize(text: Optional[str]) -> str:
"""Redact secrets and strip the home path from a string.
@@ -673,6 +713,10 @@ def build_failure(
error_class = "Error"
raw = str(exc_or_msg).strip() or "Unknown failure"
# Strip ffmpeg's stderr banner before anything reads the text — both the
# user-facing reason and classify() below, which would otherwise be
# matching against a build configuration string (#1309).
raw = strip_ffmpeg_banner(raw)
reason = sanitize(raw) or error_class
docs_topic = classify(raw)
# HF_MIRROR_UNREACHABLE's hint is dynamic (it names the configured mirror)
+97
View File
@@ -0,0 +1,97 @@
"""#1309: an ffmpeg failure reported its build configuration, not the fault.
ffmpeg and ffprobe print a version + configuration banner to STDERR on every
invocation, before doing any work. When a command fails we capture that stderr
and it becomes the error message, so the reporter saw:
extract: ffmpeg version N-125781-gacf6b520c1-20260727 Copyright (c) 2000-2026 …
built with gcc 15.2.0 (crosstool-NG …)
configuration: --prefix=/ffbuild/prefix --pkg-config-flags=--static …
— several hundred characters of build flags, and not one word about why the
extract failed. The diagnosis is always *after* the banner.
"""
from __future__ import annotations
import pytest
BANNER = """ffmpeg version N-125781-gacf6b520c1-20260727 Copyright (c) 2000-2026 the FFmpeg developers
built with gcc 15.2.0 (crosstool-NG 1.28.0.23_185f348)
configuration: --prefix=/ffbuild/prefix --pkg-config-flags=--static --enable-gpl
libavutil 59. 39.100 / 59. 39.100
libavcodec 61. 19.100 / 61. 19.100
"""
def _strip(text):
from core.failure import strip_ffmpeg_banner
return strip_ffmpeg_banner(text)
def test_real_error_survives_and_banner_does_not():
real = "Output file #0 does not contain any stream"
out = _strip(BANNER + real)
assert out == real
assert "configuration:" not in out
assert "N-125781" not in out
def test_multi_line_error_is_kept_whole():
real = "Input #0, mov,mp4:\n Stream #0:0: Video: h264\nNo audio stream found"
out = _strip(BANNER + real)
assert out == real
def test_ffprobe_banner_too():
out = _strip(BANNER.replace("ffmpeg version", "ffprobe version") + "boom")
assert out == "boom"
def test_banner_only_message_is_left_alone():
"""A message that is *nothing but* banner is unhelpful — an empty one is
worse. Fail loud with something rather than silently with nothing."""
out = _strip(BANNER)
assert out.strip()
assert "ffmpeg version" in out
@pytest.mark.parametrize("value", ["", None, "plain failure, no ffmpeg here"])
def test_non_ffmpeg_text_is_untouched(value):
assert _strip(value) == (value or "")
def test_build_failure_reports_the_error_not_the_build_flags():
"""End-to-end through the function the dub pipeline actually calls."""
from core.failure import build_failure
fields = build_failure(
RuntimeError(BANNER + "Invalid data found when processing input"),
stage="extract",
)
assert "Invalid data found" in fields["reason"]
assert "configuration:" not in fields["reason"]
assert "crosstool-NG" not in fields["reason"]
def test_classification_sees_the_error_not_the_banner():
"""classify() runs on the same text; matching against a build string is how
a real topic gets missed.
Asserts the BANNER IS GONE, not merely that the error is present. The first
version of this test only checked that the post-banner text appeared in
`reason` — which was true before the fix too, since the banner was simply
prepended to it. A test that passes against the code it is meant to catch
is worse than no test (CodeRabbit).
"""
from core.failure import build_failure
fields = build_failure(
RuntimeError(BANNER + "No space left on device"), stage="extract"
)
assert "No space left on device" in fields["reason"]
assert "ffmpeg version" not in fields["reason"]
assert "libavcodec" not in fields["reason"]
# And the reason should START with the real error, not bury it after 300
# characters of build flags — that ordering is the whole user complaint.
assert fields["reason"].strip().startswith("No space left on device")