Skip to content

fix(transcribe): wrap subprocess launch errors - #621

Closed
alooshxl wants to merge 1 commit into
Panniantong:mainfrom
alooshxl:fix/transcribe-subprocess-launch-errors
Closed

fix(transcribe): wrap subprocess launch errors#621
alooshxl wants to merge 1 commit into
Panniantong:mainfrom
alooshxl:fix/transcribe-subprocess-launch-errors

Conversation

@alooshxl

Copy link
Copy Markdown

Problem

_run() normalizes timeouts and nonzero exits into TranscribeError, but an OSError raised while starting the process escapes unchanged:

    except subprocess.TimeoutExpired:
        raise TranscribeError(f"{cmd[0]} timed out after {timeout}s")
    if proc.returncode != 0:
        ...

_cmd_transcribe catches TranscribeError only (agent_reach/cli.py:1546-1558), so such a failure bypasses the ❌ … / exit-1 path and surfaces as an unhandled exception.

The callers do run _require() first, so a binary that is simply absent from PATH is already reported as MissingDependency. What remains uncovered is the case where shutil.which() resolves but the launch still fails — a non-executable file or a permission denial (PermissionError), a broken shim or ENOEXEC, or the executable disappearing between the check and the launch.

The sibling helper in the same module already handles this. _probe_audio_duration() has an except OSError as exc: raise TranscribeError(...) from exc branch (agent_reach/transcribe.py:119-122); _run() was the inconsistent one.

Change

  • agent_reach/transcribe.py: add except OSError as exc: raise TranscribeError(f"failed to start {cmd[0]}") from exc directly after the existing timeout handler.
  • tests/test_transcribe.py: one parameterized regression in TestSubprocessDecoding covering FileNotFoundError and PermissionError, asserting the exact message, the absence of the command argument and of the OS error text, and that the original exception is preserved as __cause__.

The message deliberately uses only cmd[0]. _run()'s own docstring notes that cmd carries user-supplied URLs and paths, and str(exc) for a launch failure can echo a resolved local path or platform-specific text, so neither is interpolated into the public message. Nothing is lost for debugging: from exc keeps the original OSError available as __cause__ and in the traceback. The exact-message assertion is what stops that detail from creeping back in later.

Timeout behavior, the bounded nonzero-exit stderr message, and successful runs are unchanged.

Verification

Run on this branch (Windows, Python 3.14.5, pytest 9.1.1, ruff 0.16.3, mypy 2.3.0):

Command Result
python -m pytest tests/test_transcribe.py -v 62 passed
python -m ruff check agent_reach/transcribe.py tests/test_transcribe.py All checks passed
python -m mypy agent_reach/transcribe.py Success: no issues found in 1 source file
python -m ruff check agent_reach tests All checks passed
python -m mypy agent_reach Success: no issues found in 35 source files
git diff --check clean
python -m pytest tests/ -v 4 failed, 556 passed, 28 skipped, 16 subtests passed

Both parameter cases were confirmed to fail before the change — the raw FileNotFoundError and PermissionError propagated out of _run() — and to pass after it.

The 4 failures are pre-existing in this local environment and unrelated to this change: the symlink security tests in tests/test_channels.py and tests/test_reddit_channel.py raise OSError: [WinError 1314] A required privilege is not held by the client because this Windows account cannot create symlinks. They fail identically on main before this change (baseline: 4 failed, 554 passed, 28 skipped), and #616 addresses them separately.

Scope

  • Only the _run() exception boundary changes. _probe_audio_duration(), _require(), executable discovery, provider fallback, download validation, and timeout values are untouched.
  • No new exception subclass, no retry or fallback executable, no shell invocation.
  • No empty-cmd validation: every existing caller passes a non-empty command.

@alooshxl alooshxl closed this by deleting the head repository Aug 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant