fix(transcribe): wrap subprocess launch errors - #621
Closed
alooshxl wants to merge 1 commit into
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
_run()normalizes timeouts and nonzero exits intoTranscribeError, but anOSErrorraised while starting the process escapes unchanged:_cmd_transcribecatchesTranscribeErroronly (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 fromPATHis already reported asMissingDependency. What remains uncovered is the case whereshutil.which()resolves but the launch still fails — a non-executable file or a permission denial (PermissionError), a broken shim orENOEXEC, or the executable disappearing between the check and the launch.The sibling helper in the same module already handles this.
_probe_audio_duration()has anexcept OSError as exc: raise TranscribeError(...) from excbranch (agent_reach/transcribe.py:119-122);_run()was the inconsistent one.Change
agent_reach/transcribe.py: addexcept OSError as exc: raise TranscribeError(f"failed to start {cmd[0]}") from excdirectly after the existing timeout handler.tests/test_transcribe.py: one parameterized regression inTestSubprocessDecodingcoveringFileNotFoundErrorandPermissionError, 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 thatcmdcarries user-supplied URLs and paths, andstr(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 exckeeps the originalOSErroravailable 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):
python -m pytest tests/test_transcribe.py -vpython -m ruff check agent_reach/transcribe.py tests/test_transcribe.pypython -m mypy agent_reach/transcribe.pypython -m ruff check agent_reach testspython -m mypy agent_reachgit diff --checkpython -m pytest tests/ -vBoth parameter cases were confirmed to fail before the change — the raw
FileNotFoundErrorandPermissionErrorpropagated 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.pyandtests/test_reddit_channel.pyraiseOSError: [WinError 1314] A required privilege is not held by the clientbecause this Windows account cannot create symlinks. They fail identically onmainbefore this change (baseline: 4 failed, 554 passed, 28 skipped), and #616 addresses them separately.Scope
_run()exception boundary changes._probe_audio_duration(),_require(), executable discovery, provider fallback, download validation, and timeout values are untouched.cmdvalidation: every existing caller passes a non-empty command.