Skip to content

fix: recover from broken S3 audio streams instead of wedging - #32

Merged
andrewchen5678 merged 1 commit into
mainfrom
fix/audio-stream-pool-reset
Aug 3, 2026
Merged

andrewchen5678 merged 1 commit into
mainfrom
fix/audio-stream-pool-reset

Conversation

@andrewchen5678

Copy link
Copy Markdown
Contributor

Problem

GET /api/shows/episodes/{id}/audio returned Cloudflare 520 for every episode. S3 itself was healthy — the presigned URL played fine, and a fresh process in the same pod streamed the full object.

The running process was wedged. Bisecting by Range against the origin socket showed it delivered at most one 64 KB chunk (AUDIO_CHUNK_SIZE):

range    64 KB: delivered=  65536  OK
range    65 KB: delivered=  65536  *** TRUNCATED ***
range  1024 KB: delivered=      0  *** TRUNCATED ***

The first error in the container log was a one-off ResponseStreamingError: Connection broken: BrokenPipeError; every error after it was RuntimeError: Response content shorter than Content-Length. A socket to the S3 endpoint was left in CLOSE_WAIT. The broken connection stayed in the @lru_cached client's pool, so subsequent get_object calls returned correct headers with an empty body — uvicorn aborted those responses, cloudflared logged unexpected EOF, and Cloudflare turned that into 520. It never recovered on its own.

Fix

  • Count bytes streamed and compare against Content-Length; on a short or failed stream, drop the cached client so the next request builds a fresh pool. One request fails instead of all of them, forever.
  • Reset on connection-level BotoCoreError from get_object too, and return 502 instead of an unhandled error.
  • Client disconnects raise GeneratorExit, not Exception, so ordinary seeking does not churn the client — this matches the logs, where months of cancels were harmless and only the read error wedged it.

Tests

Four cases added: full stream keeps the client, short stream resets it, mid-stream error resets it and still closes the body, connection error resets it and returns 502.

ruff check, ruff format --check, basedpyright, and pytest (95 passed) all clean.

🤖 Generated with Claude Code

A single mid-stream failure while proxying audio left a dead connection in
the shared boto3 client's pool. Every later get_object then returned valid
headers with an empty body, so uvicorn aborted the response and Cloudflare
served 520 for all playback until the process was restarted.

Track bytes streamed against Content-Length and drop the cached client when
a stream errors or ends short, so the damage is limited to the one request
that hit the dead connection. Connection-level failures on get_object reset
it too, and now surface as 502 rather than an unhandled error.

Client disconnects (seeking) raise GeneratorExit and are deliberately left
alone -- they are routine and leave the connection usable.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@andrewchen5678
andrewchen5678 merged commit bf3324b into main Aug 3, 2026
2 checks passed
@andrewchen5678
andrewchen5678 deleted the fix/audio-stream-pool-reset branch August 3, 2026 23:15
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