Conversation
💡 Codex Reviewhttps://github.com/webex/webex-js-sdk/blob/fa258a6c2de92c05be68a6a59ee0fcfa700875ca/docs/samples/meetings.min.js#L2 The updated ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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 pull request is automatically being deployed by Amplify Hosting (learn more). |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1e807ba3c8
ℹ️ 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".
| mercury._onmessage(mercury.defaultSessionId, event); | ||
|
|
||
| // Should NOT early-return — _emit should be called via the normal flow (not the guard) | ||
| // The first call would be from the .then() block, not the early return | ||
| assert.neverCalledWith(mercury._emit, mercury.defaultSessionId, 'event', event.data); |
There was a problem hiding this comment.
Await _onmessage before asserting valid eventType behavior
_onmessage() processes normal events asynchronously (the event emit happens in a .then(...)), but this test calls it and immediately runs assert.neverCalledWith(...). That means the assertion executes before async emits occur, so the test can pass even if valid eventType handling regresses (including the exact call this assertion is trying to reason about). This leaves the new guard path effectively unverified for the non-early-return case.
Useful? React with 👍 / 👎.
COMPLETES #< INSERT LINK TO ISSUE >
This pull request addresses
A
TypeErrorin the Mercury WebSocket message handler (_onmessage) that occurs when processing messages without aneventTypeproperty in theirdatapayload. In large webinar scenarios (5k+ participants), Mercury can receive WebSocket messages (e.g., subscription responses, control messages) that lack the standarddata.eventTypefield. When this happens,_getEventHandlers(data.eventType)is called withundefined, which then throws aTypeErroroneventType.split('.'). This crashes the message processing pipeline and causes a hard JS error at runtime.by making the following changes
_getEventHandlers(eventType)(internal-plugin-mercury/src/mercury.js): Added an early return of[]wheneventTypeis falsy (undefined,null, or empty string), preventing aTypeErroron.split()._onmessage()(internal-plugin-mercury/src/mercury.js): Added a null check before calling_getEventHandlers()— ifdatais missing ordata.eventTypeis not present, the message is emitted as a generic'event'and processing returns early instead of crashing.internal-plugin-mercury/test/unit/spec/mercury.js): Added test coverage for:_onmessage()withundefineddata, missingeventType, and subscription-style responses_onmessage()still correctly processes messages with a valideventType_getEventHandlers()withundefined,null, empty string, and unregistered namespace inputsdocs/samples/meetings.min.js: Rolled back an accidental rebuild of the sample bundle that introduced a broken version header (vundefined) and content changes unrelated to this fix.Change Type
The following scenarios were tested
_onmessage()with missingdata/eventTypeand_getEventHandlers()with falsy inputs — all pass.eventType(e.g.,conversation.activity) still route through the normal event handler pipeline.docs/samples/meetings.min.jsparses cleanly after rollback (node --checkpasses).The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.