Skip to content

Commit 0d99779

Browse files
committed
fix(exc[CommandTimeoutError]) Render duration instead of "code None"
why: ``CommandTimeoutError`` inherited the bare ``CommandError`` template ``"Command failed with code {returncode}: {cmd}"``. When the SIGKILL bail fires, ``proc.returncode`` is still ``None`` and users saw the misleading ``"Command failed with code None: <cmd>"`` rather than learning the deadline was exceeded or by how much. what: - Add a ``timeout`` keyword to ``CommandTimeoutError`` carrying the wall-clock deadline that was breached. - Override ``__str__`` to render ``"Command timed out after Xs: <cmd>"`` (or ``"Command timed out: <cmd>"`` when ``timeout`` is unset), with partial output appended on a new line. - Plumb ``timeout`` through ``_wait_with_deadline`` so the exception always carries the duration that fired. - Add a doctest on ``CommandTimeoutError`` and run-level tests covering rendered duration and the ``timeout`` attribute.
1 parent f8700dc commit 0d99779

3 files changed

Lines changed: 82 additions & 1 deletion

File tree

src/libvcs/_internal/run.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ def progress_cb(output: t.AnyStr, timestamp: datetime.datetime) -> None:
265265
code, timeout_stdout, timeout_stderr = _wait_with_deadline(
266266
proc,
267267
deadline=time.monotonic() + timeout,
268+
timeout=timeout,
268269
callback=callback,
269270
cmd=_stringify_command(normalized_args),
270271
)
@@ -316,6 +317,7 @@ def _wait_with_deadline(
316317
proc: subprocess.Popen[bytes],
317318
*,
318319
deadline: float,
320+
timeout: float,
319321
callback: ProgressCallbackProtocol | None,
320322
cmd: str | list[str],
321323
) -> tuple[int, bytes | None, bytes | None]:
@@ -398,6 +400,7 @@ def _wait_with_deadline(
398400
output=message,
399401
returncode=proc.returncode,
400402
cmd=cmd,
403+
timeout=timeout,
401404
)
402405

403406
wait = min(_TIMEOUT_POLL_INTERVAL_SECONDS, remaining)

src/libvcs/exc.py

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,47 @@ def __str__(self) -> str:
3434

3535

3636
class CommandTimeoutError(CommandError):
37-
"""CommandError which gets raised when a subprocess exceeds its timeout."""
37+
r"""CommandError raised when a subprocess exceeds its wall-clock timeout.
38+
39+
Carries the deadline used when the timeout fired so callers and human
40+
readers see the duration in the rendered message rather than the
41+
misleading ``"Command failed with code None: ..."`` produced by the
42+
bare :class:`CommandError` template (the child is not necessarily reaped
43+
before the exception is raised, leaving ``returncode`` ``None``).
44+
45+
Examples
46+
--------
47+
>>> err = CommandTimeoutError(
48+
... output='partial output',
49+
... cmd='git fetch',
50+
... timeout=2.5,
51+
... )
52+
>>> print(str(err))
53+
Command timed out after 2.5s: git fetch
54+
partial output
55+
"""
56+
57+
def __init__(
58+
self,
59+
output: str,
60+
returncode: int | None = None,
61+
cmd: str | list[str] | None = None,
62+
timeout: float | None = None,
63+
) -> None:
64+
super().__init__(output=output, returncode=returncode, cmd=cmd)
65+
#: Wall-clock deadline (seconds) the subprocess exceeded, if known.
66+
self.timeout = timeout
67+
68+
def __str__(self) -> str:
69+
"""Render the timeout duration alongside the command and partial output."""
70+
cmd = getattr(self, "cmd", "")
71+
if self.timeout is not None:
72+
message = f"Command timed out after {self.timeout:g}s: {cmd}"
73+
else:
74+
message = f"Command timed out: {cmd}"
75+
if self.output.strip():
76+
message += f"\n{self.output}"
77+
return message
3878

3979

4080
class InvalidVCS(LibVCSException):

tests/_internal/test_run.py

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -125,6 +125,44 @@ def test_run_timeout_none_is_the_default() -> None:
125125
assert "default" in output
126126

127127

128+
def test_command_timeout_error_renders_duration() -> None:
129+
"""Direct ``CommandTimeoutError`` construction surfaces the duration."""
130+
err = exc.CommandTimeoutError(
131+
output="",
132+
returncode=None,
133+
cmd="git fetch origin",
134+
timeout=2.5,
135+
)
136+
137+
rendered = str(err)
138+
assert "timed out after 2.5s" in rendered
139+
assert "git fetch origin" in rendered
140+
# Regression guard: the bare ``CommandError`` template would have produced
141+
# ``"Command failed with code None: ..."`` when ``returncode`` is ``None``.
142+
assert "code None" not in rendered
143+
144+
145+
def test_command_timeout_error_without_timeout_falls_back() -> None:
146+
"""Omitting ``timeout=`` still produces a readable timeout message."""
147+
err = exc.CommandTimeoutError(output="partial", cmd="git fetch")
148+
149+
rendered = str(err)
150+
assert "timed out" in rendered
151+
assert "git fetch" in rendered
152+
assert "partial" in rendered
153+
154+
155+
def test_run_timeout_message_includes_duration() -> None:
156+
"""``run(..., timeout=X)`` raises an exception whose ``str()`` shows X."""
157+
with pytest.raises(exc.CommandTimeoutError) as excinfo:
158+
run([sys.executable, "-c", "import time; time.sleep(10)"], timeout=0.3)
159+
160+
assert excinfo.value.timeout == 0.3
161+
rendered = str(excinfo.value)
162+
assert "timed out after" in rendered
163+
assert "0.3" in rendered
164+
165+
128166
def test_run_timeout_does_not_deadlock_on_chatty_stdout() -> None:
129167
"""A child filling its stdout pipe must not deadlock the deadline loop.
130168

0 commit comments

Comments
 (0)