feat(client): echo negotiationId in subscriber offer answer#2166
feat(client): echo negotiationId in subscriber offer answer#2166
Conversation
Echo the negotiationId from the SubscriberOffer back in the SendAnswerRequest so the SFU can correlate offers with their corresponding answers. Also includes codegen updates from protocol PR #1717 and populates the new webrtcVersion field in ClientDetails. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughUpdates client-side WebRTC handling to expose WebRTC version information in client details for browser and React Native environments, and propagates a negotiation identifier through the subscriber offer/answer exchange to the SFU. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use TruffleHog to scan for secrets in your code with verification capabilities.Add a TruffleHog config file (e.g. trufflehog-config.yml, trufflehog.yml) to your project to customize detectors and scanning behavior. The tool runs only when a config file is present. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/client/src/rtc/Subscriber.ts`:
- Around line 168-172: The call to sfuClient.sendAnswer (sending
PeerType.SUBSCRIBER with negotiationId from subscriberOffer) is awaited but its
returned RPC payload/error is not checked; update the code around sendAnswer to
capture the response, check response.error (or equivalent error field) and
handle non-null errors by throwing or returning a failure before marking the
negotiation successful, ensuring you don't proceed when the SFU reports an error
for this negotiationId and sdp.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c43f4bbc-d046-42b9-8abb-3a3d61d78149
⛔ Files ignored due to path filters (4)
packages/client/src/gen/video/sfu/event/events.tsis excluded by!**/gen/**packages/client/src/gen/video/sfu/models/models.tsis excluded by!**/gen/**packages/client/src/gen/video/sfu/signal_rpc/signal.client.tsis excluded by!**/gen/**packages/client/src/gen/video/sfu/signal_rpc/signal.tsis excluded by!**/gen/**
📒 Files selected for processing (3)
packages/client/src/helpers/client-details.tspackages/client/src/rtc/Subscriber.tspackages/client/src/rtc/__tests__/Subscriber.test.ts
| await this.sfuClient.sendAnswer({ | ||
| peerType: PeerType.SUBSCRIBER, | ||
| sdp: answer.sdp || '', | ||
| negotiationId: subscriberOffer.negotiationId, | ||
| }); |
There was a problem hiding this comment.
Handle sendAnswer RPC errors before treating negotiation as successful.
sendAnswer is awaited but its SFU error payload is not checked, which can mask negotiation failures.
💡 Proposed fix
- await this.sfuClient.sendAnswer({
+ const { response } = await this.sfuClient.sendAnswer({
peerType: PeerType.SUBSCRIBER,
sdp: answer.sdp || '',
negotiationId: subscriberOffer.negotiationId,
});
+ if (response.error) throw new NegotiationError(response.error);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/client/src/rtc/Subscriber.ts` around lines 168 - 172, The call to
sfuClient.sendAnswer (sending PeerType.SUBSCRIBER with negotiationId from
subscriberOffer) is awaited but its returned RPC payload/error is not checked;
update the code around sendAnswer to capture the response, check response.error
(or equivalent error field) and handle non-null errors by throwing or returning
a failure before marking the negotiation successful, ensuring you don't proceed
when the SFU reports an error for this negotiationId and sdp.
1 participant
💡 Overview
Echo the
negotiationIdfrom theSubscriberOfferback in theSendAnswerRequestso the SFU can correlate offers with their corresponding answers.📝 Implementation notes
Subscriber.negotiate()to passsubscriberOffer.negotiationIdthrough tosfuClient.sendAnswer()negotiationIdis echoedwebrtcVersioninClientDetails)webrtcVersionfield inclient-details.ts🎫 Ticket: https://linear.app/stream/issue/REACT-898/echo-negotiationid-param-in-subscriber-offer
🔗 Protocol PR: GetStream/protocol#1717
Summary by CodeRabbit
New Features
Tests