Skip to content

fix(audio): support Response streams safely#1960

Closed
HAYDEN-OAI wants to merge 4 commits into
mainfrom
dev/hayden/pr-1952-review
Closed

fix(audio): support Response streams safely#1960
HAYDEN-OAI wants to merge 4 commits into
mainfrom
dev/hayden/pr-1952-review

Conversation

@HAYDEN-OAI

Copy link
Copy Markdown
Contributor

Summary

  • carry forward @fallintoplace's two original commits from Fix playAudio for Response streams #1952 verbatim
  • convert native Response.body Web streams with Readable.fromWeb() while preserving Node-readable bodies from custom fetch implementations
  • use stream.pipeline() so playback rejects cleanly if the response stream fails and terminates ffplay instead of leaving the child hanging
  • add focused coverage for native Web streams, Node-backed response streams, and mid-stream failures

Why

playAudio(response) currently assumes every response body has Node's .pipe() method. Native fetch responses expose a Web ReadableStream, so playback fails immediately. The original PR fixed that incompatibility and preserved the existing Node-stream path after review feedback.

During deeper review, we found that an error from the adapted Web stream would be emitted without a listener, potentially terminating the process instead of rejecting playAudio(). Routing the selected source through pipeline() handles that failure and closes the playback pipe safely.

Attribution

This integration branch contains the original #1952 commits fb2dc044 and c9e3996a unchanged. A direct maintainer push to the external fork was blocked by local repository policy, so the original history was carried into this upstream branch before adding the review fix.

Validation

  • pnpm exec jest tests/helpers/audio.test.ts --runInBand
  • pnpm exec tsc --noEmit
  • pnpm exec eslint src/helpers/audio.ts tests/helpers/audio.test.ts
  • pnpm exec prettier --check src/helpers/audio.ts tests/helpers/audio.test.ts
  • pnpm build

Supersedes #1952.

@HAYDEN-OAI

Copy link
Copy Markdown
Contributor Author

closing this out since the fix landed through the original author’s PR in #1952. thanks!

@HAYDEN-OAI HAYDEN-OAI closed this Jun 25, 2026
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.

2 participants