Skip to content

[Fix] 020-twilio-media-streams-node — handle missing ffmpeg in test#63

Closed
github-actions[bot] wants to merge 1 commit intomainfrom
fix/twilio-media-streams-node-regression-2026-03-30
Closed

[Fix] 020-twilio-media-streams-node — handle missing ffmpeg in test#63
github-actions[bot] wants to merge 1 commit intomainfrom
fix/twilio-media-streams-node-regression-2026-03-30

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Summary

  • Test crashed with Cannot read properties of null (reading 'toString') when ffmpeg was not pre-installed on the GitHub Actions runner
  • Added ensureFfmpeg() helper that auto-installs ffmpeg via apt-get when not found
  • Fixed null stderr handling in spawnSync error path (ENOENT returns null for stderr/stdout)

Root cause

GitHub Actions ubuntu runner images no longer guarantee ffmpeg is pre-installed. When spawnSync('ffmpeg', ...) fails with ENOENT, result.stderr is null and result.status is null, causing the existing error handler to crash on result.stderr.toString().

Test plan

  • CI runs the test successfully with ffmpeg auto-installed
  • Test still works on runners that already have ffmpeg

🤖 Generated with Claude Code

ffmpeg is no longer pre-installed on all GitHub Actions ubuntu runner
images. The test now auto-installs ffmpeg when not found and properly
handles null stderr from spawnSync ENOENT errors.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <noreply@anthropic.com>
@github-actions github-actions bot added the type:fix Bug fix label Mar 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

Code Review

Overall: CHANGES REQUESTED

Integration genuineness

✓ Pass — this is a test-infrastructure fix only. The underlying example genuinely integrates both twilio and @deepgram/sdk, .env.example lists Twilio and Deepgram credentials, and the test makes real API calls.

Code quality

ensureFfmpeg() helper with timeout: 60_000 on apt-get is good defensive coding
✓ Null stderr guard with (result.stderr || Buffer.alloc(0)) correctly handles ENOENT case
result.error check added before result.status check
⚠ Minor: uses which ffmpeg which may not exist on all systems; command -v ffmpeg or spawning ffmpeg -version would be more portable

Documentation

✓ PR body clearly explains root cause and fix

Tests

✓ Credential check still runs first with exit code 2
✓ Test still makes real API calls

Duplicate concern

This PR appears to be a duplicate of #56, which fixes the exact same issue (missing ffmpeg + null stderr crash) in the exact same file. PR #56 is already labeled status:review-passed. These two PRs will conflict on merge. One should be closed.


Please address the duplicate concern above — if #56 covers this fix, this PR should be closed. If this PR is intended to replace #56, please note that in the PR body and close #56.


Review by Lead on 2026-03-30

@github-actions github-actions bot added the status:fix-needed Tests failing — fix agent queued label Mar 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

Closing as duplicate

Root cause: This PR fixes the exact same issue (missing ffmpeg + null stderr crash in 020-twilio-media-streams-node/tests/test.js) as PR #56, which is already open with status:review-passed.

Action: Closing this PR in favor of #56 to avoid merge conflicts. Both PRs add ensureFfmpeg(), guard result.error, and handle null stderr — the changes are functionally identical.


Fix triage by Lead on 2026-03-30

@github-actions github-actions bot removed the status:fix-needed Tests failing — fix agent queued label Mar 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

Closing as duplicate of #56.

@github-actions github-actions bot closed this Mar 30, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type:fix Bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

0 participants