Skip to content

fix: keep stderr written before an async timeout - #6

Merged
jqnatividad merged 1 commit into
mainfrom
fix/async-timeout-stderr
Oct 10, 2026
Merged

jqnatividad merged 1 commit into
mainfrom
fix/async-timeout-stderr

Conversation

@jqnatividad

Copy link
Copy Markdown
Contributor

Problem

On timeout, AsyncQsv raised QsvTimeout with an empty or truncated stderr, while Qsv kept everything qsv wrote before the deadline.

run_async wrapped proc.communicate() in asyncio.wait_for. On timeout, wait_for cancels it, and the inner StreamReader.read(-1) collects chunks in a local list, so whatever it had already read was thrown away. _timeout_cleanup then only saw stderr arriving after the kill.

Fix

run_async now waits on the communicate() task with asyncio.wait(..., timeout=) and doesn't cancel it on timeout. The process tree is killed as before, then the still-running read is given kill_grace to finish and its output is used. If a descendant that left the process group still holds the pipes, the read is cancelled and the output dropped, the same as run_sync. Cancellation and teardown paths are otherwise unchanged.

Tests

  • New test_timeout_keeps_stderr_written_before_it (sync) and test_async_timeout_keeps_stderr_written_before_it. The async one failed before the fix.
  • Full suite passes locally (63 passed, macOS). ruff and mypy are clean (both default and --platform win32).
  • The async tests produce no asyncio "Task was destroyed" or "never retrieved" log messages.
  • The change is platform-neutral but has not been run on Windows locally; CI covers it.

🤖 Generated with Claude Code

run_async wrapped communicate() in wait_for, so a timeout cancelled it
and discarded the chunks it had already read: QsvTimeout.stderr from
AsyncQsv held only what arrived after the kill, while Qsv kept it all.

Let communicate() keep running through the kill and collect its output
within kill_grace; if an escaped descendant still holds the pipes, drop
the output as run_sync does.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@jqnatividad
jqnatividad merged commit b6a6535 into main Oct 10, 2026
16 checks passed
@jqnatividad
jqnatividad deleted the fix/async-timeout-stderr branch October 10, 2026 17:44
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