Skip to content

fix(audio): reject ffplay spawn errors#2012

Open
tylergibbs1 wants to merge 1 commit into
openai:mainfrom
tylergibbs1:agent/play-audio-spawn-error
Open

fix(audio): reject ffplay spawn errors#2012
tylergibbs1 wants to merge 1 commit into
openai:mainfrom
tylergibbs1:agent/play-audio-spawn-error

Conversation

@tylergibbs1

Copy link
Copy Markdown
  • I understand that this repository is auto-generated and my pull request may not be merged

Changes being requested

Handle the asynchronous error event from the ffplay child process in playAudio().

When ffplay cannot be started—for example, when it is not installed—spawn() returns a child process and reports the failure asynchronously. Without an error listener, Node.js throws an uncaught EventEmitter error and the promise returned by playAudio() does not reject normally.

This change attaches the listener immediately after spawning and adds a regression test confirming that callers receive the original spawn error as a rejected promise.

Additional context & links

Validation:

  • pnpm test — 120 suites passed, 1,463 tests passed
  • pnpm lint — formatting, ESLint, type checks, package checks, and JSR publish dry run passed
  • pnpm build — Node.js and Deno builds passed

Some portions of this contribution were developed with assistance from OpenAI Codex (GPT-5). The final diff and test results were reviewed locally.

Listen for asynchronous child-process errors so playAudio() rejects when ffplay cannot be started instead of causing an uncaught EventEmitter error.

Assisted-by: OpenAI Codex (GPT-5)
@tylergibbs1
tylergibbs1 requested a review from a team as a code owner July 23, 2026 21:53
Copilot AI review requested due to automatic review settings July 23, 2026 21:53

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR ensures playAudio() properly rejects when ffplay fails to spawn (e.g., not installed), by handling the child process’ asynchronous error event instead of allowing an uncaught EventEmitter error.

Changes:

  • Attach an error event listener to the spawned ffplay process to reject the playAudio() promise on spawn failure.
  • Add a regression test that simulates a spawn failure and asserts the promise rejects with the original error.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
src/helpers/audio.ts Adds ffplay child-process error handling so playAudio() rejects cleanly on spawn failures.
tests/helpers/audio.test.ts Adds a test covering async spawn failure (error event) and verifies promise rejection behavior.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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