fix(mercury): gracefully handle mercury rebalancing - #5106
Conversation
![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: 8e0f835eee
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| this.logger.info(`${this.namespace}: socket ${sessionId} replaced; will not reconnect`); | ||
| if (isActiveSocket) this._emit(sessionId, 'offline.replaced', event); | ||
| // If not active, nothing to do | ||
| if (reason === 'replaced') { |
There was a problem hiding this comment.
When the active socket closes with code 4000 but no reason is provided, this condition now falls into the reconnect branch, emitting offline.transient and calling _reconnect; previously every 4000 close was treated as offline.replaced, and the existing mercury-events unit coverage still exercises {code: 4000} as a replacement. In clients or browsers that do not surface the close reason, a replaced socket will now reconnect and can churn against the intended replacement connection instead of staying closed.
Useful? React with 👍 / 👎.
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
Shreyas281299
added
the
validated
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c0d1145da
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const {action, isActiveSocket, messageType} = options; | ||
|
|
||
| try { | ||
| this.webex.internal.metrics.submitClientMetrics(MERCURY_CLOSE_4000_METRIC, { |
There was a problem hiding this comment.
When the client-metrics request fails asynchronously—such as during the same network disruption that closed Mercury—submitClientMetrics() returns a rejected promise, but this call discards it. The surrounding try/catch only handles synchronous throws, so the failure becomes an unhandled rejection and can terminate Node clients instead of merely logging the metric failure as intended; attach a rejection handler to the returned promise.
Useful? React with 👍 / 👎.
1 participant
COMPLETES #N/A
This pull request addresses
Mercury close code
4000is not always a signal that the current SDK context should reconnect. According to Mercury connection behavior,4000with close reasonReplacedmeans another connection already replaced the current registration, so this older connection should stand down. A4000with any other reason should be treated as an unexpected close and reconnect.by making the following changes
4000.4000with reasonReplacedby emittingoffline.replaced.4000with any other reason by emittingoffline.transientand calling_reconnect().4000/Replaced,4000with an unknown reason, and non-active socket close handling.Change Type
The following scenarios were tested
yarn workspace @webex/internal-plugin-mercury test:unit --targets mercury.jspassed with100 passing.yarn workspace @webex/internal-plugin-mercury test:stylecompleted with no errors and one existing JSDoc warning insrc/index.js.The GAI Coding Policy And Copyright Annotation Best Practices
I certified that
Make sure to have followed the contributing guidelines before submitting.