Skip to content

Fix E2EEManager::frameCryptors() sending an empty E2EE request - #300

Open
mdepinet wants to merge 1 commit into
livekit:mainfrom
mdepinet:mike/fix-frame-cryptors-request
Open

mdepinet wants to merge 1 commit into
livekit:mainfrom
mdepinet:mike/fix-frame-cryptors-request

Conversation

@mdepinet

Copy link
Copy Markdown

This PR was written and verified by Claude Opus 5.5 (an AI model by Anthropic), working with @mdepinet.

Fixes #299

What changed

  • E2EEManager::frameCryptors() now selects the manager_get_frame_cryptors request. Without it, the FFI received an E2eeRequest with no message, rejected it (InvalidRequest("message is empty")), and frameCryptors() threw.
  • A new integration test connects with E2EE, publishes an audio track, and checks that frameCryptors() includes the local participant's cryptor.

Testing

Ran on Linux x86_64 against livekit-server --dev:

  • Without the fix, E2EEIntegrationTest.FrameCryptorsIncludePublishedTrack fails (frameCryptors() throws).
  • With the fix, it passes.

🤖 Generated with Claude Code

The request never selected manager_get_frame_cryptors, so the FFI
rejected it and frameCryptors() always threw.

Fixes livekit#299

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@mdepinet
mdepinet requested review from a team and xianshijing-lk as code owners October 27, 2026 03:21
Copilot AI balanced review requested due to automatic review settings October 27, 2026 03:21
@CLAassistant

CLAassistant commented Oct 27, 2026 •

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🔍 Devin Review: 1 flag

Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

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.

🟢 Approval recommended

The narrow fix addresses the reported FFI request defect, includes regression coverage, and leaves no unresolved review findings.

No open issues 🎉

What changed in this PR

Fixes #299 by selecting the required FFI operation in E2EEManager::frameCryptors(), preventing rejection of an empty E2EE request.

Changes:

  • Selects manager_get_frame_cryptors before sending the request.
  • Adds an integration regression test that publishes an encrypted audio track and checks for the local participant’s cryptor.
File Description
src/​tests/​integration/​test_e2ee.cpp Adds regression coverage for retrieving frame cryptors after publishing.
src/​e2ee.cpp Selects the missing frame-cryptor request operation.

Review effort: Balanced


// The track's cryptor may be created after publishing returns.
bool found = false;
const auto deadline = std::chrono::steady_clock::now() + 5s;

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

is 5s an appropriate deadline? since the creation is all local i would expect <1s. Curious if you tried lower number with/out success?

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.

E2EEManager::frameCryptors() always throws: its request never selects manager_get_frame_cryptors

4 participants