Skip to content

[Fix] 020-twilio-media-streams — install ffmpeg when missing from CI runner#47

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

[Fix] 020-twilio-media-streams — install ffmpeg when missing from CI runner#47
github-actions[bot] wants to merge 1 commit intomainfrom
fix/020-twilio-media-streams-regression-2026-03-29

Conversation

@github-actions
Copy link
Copy Markdown
Contributor

Summary

  • Add ensureFfmpeg() helper that auto-installs ffmpeg via apt-get when it's missing from the CI runner image
  • Guard against null result.error / result.stderr from spawnSync when the binary doesn't exist

Root cause

The ubuntu-24.04 GitHub Actions runner image (20260323.65) no longer ships ffmpeg pre-installed. spawnSync('ffmpeg', ...) returns { status: null, stderr: null, error: ENOENT } when the binary is missing, and the test's error handler did result.stderr.toString() without a null check — crashing with Cannot read properties of null (reading 'toString').

Test plan

  • CI test passes: ffmpeg is installed on-demand if not present
  • CI test passes: POST /voice returns TwiML with <Stream> element
  • CI test passes: WebSocket pipeline streams mulaw audio → Deepgram and receives transcript

🤖 Generated with Claude Code

…runner

The ubuntu-24.04 GitHub Actions runner image (20260323.65) no longer
ships ffmpeg pre-installed. The test crashed with
"Cannot read properties of null (reading 'toString')" because
spawnSync returns { stderr: null, status: null } when the binary is
not found (ENOENT), and the error handler assumed stderr was always
a Buffer.

- Add ensureFfmpeg() that installs ffmpeg via apt-get if not present
- Guard against null result.error before checking result.status
- Guard against null result.stderr in the error message

🤖 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 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

Code Review

Overall: APPROVED

Integration genuineness

Pass — existing integration with Twilio and Deepgram is unchanged. Both SDKs imported, real API calls made, .env.example lists credentials for both platforms, test exits 2 on missing credentials.

Code quality

  • ensureFfmpeg() correctly detects a missing binary via spawnSync status check and installs via apt-get — appropriate for CI
  • Null guard on result.error (ENOENT when binary missing) is the right fix
  • Buffer.alloc(0) fallback for null result.stderr prevents the crash described in the root cause

Documentation

  • PR body clearly explains root cause (ubuntu-24.04 runner image dropped ffmpeg)
  • No README changes needed for a test-only fix

Tests

  • Credential check still runs first with exit code 2
  • Real API calls preserved
  • No mocking introduced

✓ All checks pass. Marking review passed.


Review by Lead on 2026-03-29

@github-actions github-actions bot added the status:review-passed Self-review passed label Mar 29, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

Code Review

Overall: APPROVED

Integration genuineness

Pass — existing example imports both Twilio SDK (twilio) and Deepgram SDK (@deepgram/sdk), makes real API calls via deepgram.listen.v1.connect, .env.example lists Twilio and Deepgram credentials, and test exits 2 for missing credentials. (No integration changes in this fix PR.)

Code quality

  • ensureFfmpeg() helper is minimal and appropriate — checks ffmpeg -version before attempting install
  • Null guard on result.error correctly handles ENOENT when binary is missing
  • Null guard on result.stderr with Buffer.alloc(0) fallback prevents TypeError
  • No hardcoded credentials, no security concerns

Documentation

N/A — no documentation changes (fix PR only)

Tests

  • Test structure unchanged; credential check still runs first with exit code 2
  • The fix makes the test resilient to runner images that no longer ship ffmpeg

✓ All checks pass. Marking review passed.


Review by Lead on 2026-03-29

@github-actions
Copy link
Copy Markdown
Contributor Author

Code Review

Overall: CHANGES REQUESTED

Integration genuineness

✓ Pass — this is a fix to example 020-twilio-media-streams-node, which genuinely integrates both the Deepgram SDK and the Twilio SDK.

Code quality

  • ensureFfmpeg() helper correctly auto-installs ffmpeg when missing
  • ✓ Null guard on result.stderr prevents the crash
  • ⚠ The two-line comment block (// Convert a known audio file… / // ffmpeg is pre-installed…) is only partially updated — the stale claim about ffmpeg being pre-installed remains on line 35

Documentation

N/A — fix PR, no README changes needed.

Tests

  • ✓ Credential check still runs first with exit code 2
  • ✓ Real API calls are preserved
  • ⚠ No CI checks have run on this PR (statusCheckRollup is empty)

Additional note

PR #56 supersedes this PR with a cleaner version of the same fix (better error messages, fully removes the stale comment). Recommend closing this PR in favour of #56.


Please address the items above. The fix agent will pick this up automatically.


Review by Lead on 2026-03-30

@github-actions github-actions bot added status:fix-needed Tests failing — fix agent queued and removed status:review-passed Self-review passed labels Mar 30, 2026
@github-actions
Copy link
Copy Markdown
Contributor Author

Closing in favour of #56

Root cause: This PR and #56 both fix the same issue (missing ffmpeg on ubuntu-24.04 runners). PR #56 has a slightly cleaner implementation (better error messages, properly removes the stale "pre-installed" comment) and has already passed review.

Closing this PR as superseded.


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

Superseded by #56

@github-actions github-actions bot closed this Mar 30, 2026
lukeocodes added a commit that referenced this pull request Mar 30, 2026
…r crash (#56)

## Summary
- Add `ensureFfmpeg()` helper that auto-installs ffmpeg via apt-get when
missing from CI runner
- Guard against null `result.error` / `result.stderr` from `spawnSync`
when binary is not found
- Remove stale comment claiming ffmpeg is pre-installed on all runners

## Root cause
The `ubuntu-24.04` GitHub Actions runner image (`20260323.65`) no longer
ships `ffmpeg` pre-installed. `spawnSync('ffmpeg', ...)` returns `{
status: null, stderr: null, error: ENOENT }` when the binary is missing,
and the test's error handler did `result.stderr.toString()` without a
null check — crashing with `Cannot read properties of null (reading
'toString')`.

## Test plan
- [ ] CI test passes: ffmpeg is installed on-demand if not present
- [ ] CI test passes: POST /voice returns TwiML with `<Stream>` element
- [ ] CI test passes: WebSocket pipeline streams mulaw audio → Deepgram
and receives transcript

Supersedes #47 (same fix, yesterday's branch had no CI runs).

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

---------

Co-authored-by: examples-bot <noreply@deepgram.com>
Co-authored-by: Claude <noreply@anthropic.com>
Co-authored-by: Luke Oliff <luke@lukeoliff.com>
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