feat(calling): add media core ICE event listeners for call media monitoring#4925
feat(calling): add media core ICE event listeners for call media monitoring#4925Kesari3008 wants to merge 8 commits intowebex:nextfrom
Conversation
…toring Adds listeners for ICE_GATHERING_STATE_CHANGED, PEER_CONNECTION_STATE_CHANGED, ICE_CONNECTION_STATE_CHANGED, and ICE_CANDIDATE_ERROR events on the media connection. Events are logged, submitted as media metrics, and emitted as call events for consumer handling. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de61f45473
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| private handleIceGatheringStateChanged = ( | ||
| event: Partial<{state: string; iceGatheringState: string}> | ||
| ) => { | ||
| const iceGatheringState = Call.getPeerConnectionStateFromEvent(event, 'iceGatheringState'); | ||
|
|
There was a problem hiding this comment.
The new ICE listeners in Call only log and submit metrics, but never call this.emit(...) for the newly added CALL_EVENT_KEYS.ICE_* events. Since this commit also exposes these events in CallEventTypes, consumers can register handlers that will never fire at runtime. Any client depending on call.on(...) for ICE state/error monitoring will silently miss all updates, so the feature is incomplete for external callers.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8d052aeabe
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| this.metricManager.submitMediaMetric( | ||
| METRIC_EVENT.MEDIA_ERROR, | ||
| MEDIA_CONNECTION_ACTION.ICE_CANDIDATE_ERROR, | ||
| METRIC_TYPE.BEHAVIORAL, | ||
| this.callId, |
There was a problem hiding this comment.
handleIceCandidateError() submits a METRIC_EVENT.MEDIA_ERROR metric without a CallError argument, but submitMediaMetric() only emits MEDIA_ERROR payloads when callError is present (see packages/calling/src/Metrics/index.ts, case METRIC_EVENT.MEDIA_ERROR). In ICE-candidate-failure scenarios this call becomes a no-op, so the new error telemetry is silently dropped instead of being recorded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 348cbb82e7
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| errorCode: event.errorCode, | ||
| errorText: event.errorText, |
There was a problem hiding this comment.
ICE_CANDIDATE_ERROR from media-core is consumed elsewhere in this repo as a nested payload (event.error.errorCode / event.error.errorText in packages/@webex/plugin-meetings/src/meeting/index.ts), but this handler reads top-level event.errorCode and event.errorText. In that common shape, the new warning message and MEDIA_ERROR metric lose the actual ICE failure details (code/text), so the monitoring signal becomes effectively non-actionable in production.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ded9bed949
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| }, | ||
| preferredKey: 'connectionState' | 'iceConnectionState' | 'iceGatheringState' | ||
| ): string { | ||
| return event[preferredKey] || event.state || Call.UNKNOWN_STATE; |
There was a problem hiding this comment.
The new ICE state listeners assume the callback always receives an object and immediately read event[preferredKey], but media-core state-change callbacks are also used in this repo with no payload (for example, PEER_CONNECTION_STATE_CHANGED listeners are invoked as listener() in packages/@webex/plugin-meetings/test/unit/spec/meeting/index.js:9447). In that case event is undefined, this line throws a TypeError, and the call can crash on routine state-change events instead of just logging/submitting metrics.
Useful? React with 👍 / 👎.
…dia-Core-Event-Listeners
mkesavan13
left a comment
mkesavan13
left a comment
There was a problem hiding this comment.
Looks good to me. Kindly resolve the merge conflicts. Approving
2 participants
COMPLETES #CAI-6517
This pull request addresses
Mid-call media monitoring for calls
by making the following changes
Adds listeners for ICE_GATHERING_STATE_CHANGED, PEER_CONNECTION_STATE_CHANGED, ICE_CONNECTION_STATE_CHANGED, and ICE_CANDIDATE_ERROR events on the media connection. Events are logged, submitted as media metrics, and emitted as call events for consumer handling.
Change Type
The following scenarios were tested
< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >
The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.