Bound download subprocess waits and retry transient failures on POST and ad-hoc fetches

[JavaGT]

60-second summary

Downloads can hang forever if the yt-dlp child or N_m3u8DL-RE stalls, and the DRM-critical POST requests (webplayback, license exchange) plus manifest/cover fetches give up on the first 429/5xx — so one transient server hiccup fails a whole track. This PR caps both download subprocess paths at 1800 s per track (kill + clean error on overrun) and adds retry policies: POST allowed on the existing main-client retry (it is the only client whose POSTs hit these two endpoints), and a 3-retry policy on the ad-hoc manifest/cover clients.

Verify in one command: uv run python -c "import asyncio; from gamdl.utils import async_subprocess; asyncio.run(async_subprocess('sleep','10',timeout=2,silent=True))" → the child is killed and a RuntimeError: Timed out after 2 seconds: sleep 10 is raised at ~2 s; with no timeout argument the call completes as before. Rollback: revert the single commit; no config keys or CLI flags change.

What

Bounded waits for the two external download paths and transient-failure retries for the fetch paths that previously had none. Successful downloads behave exactly as before.

Why

• Unbounded waits: the yt-dlp parent loop while process.is_alive(): await asyncio.sleep(0.1) (gamdl/downloader/base.py:282-283 pre-change) has no deadline — its terminate/kill finally can never trigger; async_subprocess awaited proc.communicate() with no timeout (gamdl/utils.py:20 pre-change).
• POSTs never retried: the main AMP client's Retry(total=6, …) has no allowed_methods, and httpx-retries 0.4.6 retries only idempotent methods by default — get_webplayback (apple_music.py:725) and get_license_exchange (apple_music.py:760) are POSTs, so 429/5xx hard-fail the track on the DRM-critical path (mechanism matches Error fetching license exchange data (Status code: 429) glomatico/gamdl#306).
• Ad-hoc clients never retried: get_response (manifest/lyrics fetches, used at interface/song.py:310,416,533 and music_video.py:193,358,385) and get_cover_bytes each built a fresh httpx.AsyncClient with no retry transport — one transient blip fails the track.

The change

• gamdl/utils.py: DOWNLOAD_TIMEOUT_SECONDS = 1800; async_subprocess wraps communicate() in asyncio.wait_for, kills the child on overrun, and raises a RuntimeError naming the command and timeout.
• gamdl/downloader/base.py: the yt-dlp wait loop checks a monotonic deadline and raises on overrun; the existing finally (terminate → grace join → kill) then runs unchanged.
• gamdl/api/apple_music.py: allowed_methods added to the existing Retry — scoped in practice, because the only POSTs on that client are the two above, whose JSON bodies are rewindable.
• gamdl/interface/base.py: shared _HTTP_RETRY (total=3, backoff 1, 429/5xx) applied via RetryTransport to both ad-hoc clients; their existing per-call timeouts (60 s / 30 s) and follow_redirects behavior are preserved.
• Intentional non-goals: connection pooling for the ad-hoc clients (perf, separate theme), making cover failure non-fatal (caller-contract change, separate discussion), and the per-track process-spawn performance item (Perf: a fresh OS process is spawned per track for yt-dlp downloads (spawn + full yt-dlp import per song) #3, measure-first).

Verification

• python3 -m py_compile clean on all four files; package builds via uv run.
• Live check: Retry(..., allowed_methods=…) accepts the new field and reports POST retryable under the pinned httpx-retries 0.4.6 (already a runtime dependency via the main client — this PR adds no new dependency); async_subprocess('sleep','10', timeout=2) kills the child and raises at 2.0 s; a fast-exiting command completes normally.
• Limits and review outcomes, stated honestly: no Apple Music credentials in this environment, so the retry paths were verified against the library's own semantics and the timeout path against synthetic processes, not against a real server or download. The repo has no test suite, so no tests are added. Known boundaries: the cap bounds whole-track wall time, not stalls only — a legitimately slow transfer that exceeds 30 minutes will now fail (make it configurable if that bites anyone); on timeout only the direct child is killed, so a grandchild holding inherited pipes could linger (latent: only affects silent=True runs); allowing POST retries also retries network-exception failures, not just 429/5xx — both POSTs are idempotent read-style fetches, so this is safe; the one-shot setup clients (get_token, get_account_info) are intentionally left unretried — a startup failure is user-visible and cheap to rerun.

Design notes / questions

• 600 s is meant as a generous whole-track cap (long tracks on slow links) while still bounding a wedged child; happy to make it configurable via the config file if you'd prefer.
• Related upstream work: Increase httpx client timeout when fetching apples homepage glomatico/gamdl#332 (increase httpx timeout on homepage fetch) addresses a different timeout; this PR does not touch that path.

Attribution

This PR was produced with AI assistance under the direction of @JavaGT:

• Exploration, evidence verification, implementation: GLM agents (ZCode)
• Planning: GLM agents; findings cross-checked at audit time by OpenAI GPT-5.6 ("Luna") and xAI Grok 4.6
• Diff review this round: DeepSeek v4.1 Flash (hostile round) and DeepSeek v4 Flash (test-honesty lens) via opencode; the GPT-5.6 seat hit its usage limit and the xAI endpoint returned HTTP 403 today — their review rounds will be appended when available.
  The final diff was verified against the described behavior. Happy to adjust or close any part of this — tell me what doesn't fit the project's direction.

[JavaGT]

[Account-2 readiness review — 2026-09-19 · read-only adversarial verification pass; nothing in this PR, repo, or workspace was modified]

Verdict: READY-WITH-NOTES for upstream filing (one stale body paragraph to fix first). The diff delivers all claimed mechanisms: DOWNLOAD_TIMEOUT_SECONDS = 1800 + asyncio.wait_for + kill + descriptive RuntimeError; monotonic deadline in the yt-dlp loop with the pre-existing terminate→grace-join→kill finally untouched; allowed_methods = httpx-retries defaults + POST on the existing Retry(total=6); shared _HTTP_RETRY (total=3) via RetryTransport on both ad-hoc clients, timeouts preserved. httpx-retries>=0.4.6 confirmed pre-existing (pyproject.toml:18 on base) — the rejected "new dependency" review note was indeed wrong. POST census verified: exactly two .post( call sites (apple_music.py:725, :760), none in interface files, matching the body's scoping claim. Scope 4 files +51/−5, one theme; non-goals stated; no upstream overlap (glomatico#332 explicitly disambiguated); no license/secret concerns.

PRE-SHIP — confidence high: the Design-notes paragraph still says "600 s is meant as a generous whole-track cap" — a leftover from before a475bd9 raised the cap to 1800 s (DOWNLOAD_TIMEOUT_SECONDS). Everything else in the body (60-second summary, What section) says 1800 s. Update that paragraph so the 60-second read is self-consistent.

Two evaluate-only observations are ticketed on their canonical issues (#6 — unbounded post-kill cleanup read; #7 — blanket POST retry vs the issue's endpoint-scoped direction). Commit-subject length noted for the upstream squash (~88 chars vs upstream's short imperative style).

Decision question for the owner: fix the stale 600 s paragraph at pickup? (implement-it / evaluate — nothing pre-selected.)