From fbbe32a67666cc1e7d74bc767e0625ecf8d67613 Mon Sep 17 00:00:00 2001 From: 0xDevNinja Date: Wed, 1 Jul 2026 13:56:09 +0530 Subject: [PATCH] fix(video_compose): surface Remotion failures and add render timeout passthrough MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The high-level Remotion render path hid the useful failure reason. run_command runs with check=True + capture_output, so a non-zero exit raised CalledProcessError whose str() is only 'returned non-zero exit status 1' — the actual Remotion diagnostics in stderr were dropped. Catch CalledProcessError and surface the stderr/stdout tail, and TimeoutExpired with an actionable hint. Also add a creator-facing remotion_timeout_ms input, passed through as Remotion's --timeout (headless-browser setup + delayRender). Slow browser startup on restricted networks previously failed opaquely at the default 30s with no way to raise it. The subprocess timeout is widened to match so run_command does not kill Remotion before its own timeout fires. Closes #217 --- tests/tools/test_remotion_diagnostics.py | 95 ++++++++++++++++++++++++ tools/video/video_compose.py | 44 ++++++++++- 2 files changed, 138 insertions(+), 1 deletion(-) create mode 100644 tests/tools/test_remotion_diagnostics.py diff --git a/tests/tools/test_remotion_diagnostics.py b/tests/tools/test_remotion_diagnostics.py new file mode 100644 index 00000000..f86526e7 --- /dev/null +++ b/tests/tools/test_remotion_diagnostics.py @@ -0,0 +1,95 @@ +"""Tests for Remotion render debuggability in video_compose (issue #217). + +Two creator-facing gaps: + 1. A failed `npx remotion render` surfaced only "returned non-zero exit + status 1"; the useful Remotion diagnostics in stderr were dropped. + 2. There was no pass-through for Remotion's `--timeout`, so a slow headless + browser setup failed opaquely with no way to raise the limit. +""" + +import subprocess +import sys +from pathlib import Path + +import pytest + +PROJECT_ROOT = Path(__file__).resolve().parent.parent.parent +sys.path.insert(0, str(PROJECT_ROOT)) + +from tools.video.video_compose import VideoCompose # noqa: E402 + + +@pytest.fixture +def tool(monkeypatch): + monkeypatch.setattr("shutil.which", lambda _: "/usr/bin/npx") + return VideoCompose() + + +def test_render_failure_surfaces_remotion_stderr_tail(tool, tmp_path, monkeypatch): + stderr = "some npm noise\nError: Delayed render timed out\nRemotion actual cause here" + + def fake_run_command(cmd, *a, **k): + raise subprocess.CalledProcessError(returncode=1, cmd=cmd, output="", stderr=stderr) + + monkeypatch.setattr(tool, "run_command", fake_run_command) + result = tool._remotion_render( + {"composition_data": {"cuts": []}, "output_path": str(tmp_path / "out.mp4")} + ) + + assert result.success is False + assert "exit 1" in result.error + assert "Remotion actual cause here" in result.error + + +def test_timeout_expired_gives_actionable_message(tool, tmp_path, monkeypatch): + def fake_run_command(cmd, *a, **k): + raise subprocess.TimeoutExpired(cmd=cmd, timeout=600) + + monkeypatch.setattr(tool, "run_command", fake_run_command) + result = tool._remotion_render( + {"composition_data": {"cuts": []}, "output_path": str(tmp_path / "out.mp4")} + ) + + assert result.success is False + assert "timed out" in result.error.lower() + assert "remotion_timeout_ms" in result.error + + +def test_remotion_timeout_ms_is_passed_through(tool, tmp_path, monkeypatch): + seen = {} + + def fake_run_command(cmd, *a, **k): + seen["cmd"] = cmd + seen["timeout"] = k.get("timeout") + return None # output file intentionally absent + + monkeypatch.setattr(tool, "run_command", fake_run_command) + tool._remotion_render( + { + "composition_data": {"cuts": []}, + "output_path": str(tmp_path / "out.mp4"), + "remotion_timeout_ms": 120000, + } + ) + + assert "--timeout=120000" in seen["cmd"] + # subprocess timeout widened past the 120s render budget so run_command + # does not kill Remotion before its own timeout fires. + assert seen["timeout"] >= 180 + + +def test_no_timeout_flag_when_not_requested(tool, tmp_path, monkeypatch): + seen = {} + + def fake_run_command(cmd, *a, **k): + seen["cmd"] = cmd + seen["timeout"] = k.get("timeout") + return None + + monkeypatch.setattr(tool, "run_command", fake_run_command) + tool._remotion_render( + {"composition_data": {"cuts": []}, "output_path": str(tmp_path / "out.mp4")} + ) + + assert not any(str(c).startswith("--timeout") for c in seen["cmd"]) + assert seen["timeout"] == 600 diff --git a/tools/video/video_compose.py b/tools/video/video_compose.py index c2aa7e38..badfedf3 100644 --- a/tools/video/video_compose.py +++ b/tools/video/video_compose.py @@ -187,6 +187,15 @@ class VideoCompose(BaseTool): "codec": {"type": "string", "default": "libx264"}, "crf": {"type": "integer", "default": 23}, "preset": {"type": "string", "default": "medium"}, + "remotion_timeout_ms": { + "type": "integer", + "description": ( + "Remotion render timeout in milliseconds, passed through as " + "`--timeout` (governs headless-browser setup and delayRender). " + "Raise this when the browser is slow to start (e.g. restricted " + "networks). The subprocess timeout is widened to match." + ), + }, }, } @@ -1738,12 +1747,45 @@ class VideoCompose(BaseTool): except (ImportError, ValueError): pass + # Optional creator-facing render timeout. Remotion's `--timeout` (ms) + # governs headless-browser setup and delayRender(); on slow machines or + # restricted networks the default 30s browser setup times out with an + # opaque failure. Pass it through and give the subprocess enough headroom + # so run_command() does not kill Remotion before its own timeout fires. + remotion_timeout_ms = inputs.get("remotion_timeout_ms") + subprocess_timeout = 600 + if remotion_timeout_ms: + try: + ms = int(remotion_timeout_ms) + cmd.append(f"--timeout={ms}") + subprocess_timeout = max(subprocess_timeout, ms // 1000 + 60) + except (TypeError, ValueError): + pass + try: # Invoke from inside the composer dir so npx can resolve the # local remotion binary via node_modules/.bin. Without this, # Windows npx cannot locate the CLI and returns "could not # determine executable to run". - self.run_command(cmd, timeout=600, cwd=composer_dir) + self.run_command(cmd, timeout=subprocess_timeout, cwd=composer_dir) + except subprocess.CalledProcessError as e: + # run_command uses check=True + capture_output, so the useful + # Remotion diagnostics live in stderr/stdout — surface the tail + # instead of the bare "returned non-zero exit status 1". + detail = (e.stderr or e.stdout or "").strip() + tail = "\n".join(detail.splitlines()[-25:]) if detail else "(no output captured)" + return ToolResult( + success=False, + error=f"Remotion render failed (exit {e.returncode}):\n{tail}", + ) + except subprocess.TimeoutExpired as e: + return ToolResult( + success=False, + error=( + f"Remotion render timed out after {e.timeout}s. If the headless " + "browser is slow to start, raise remotion_timeout_ms (ms)." + ), + ) except Exception as e: return ToolResult(success=False, error=f"Remotion render failed: {e}") finally: