feat(mercury): handle mercury shutdown #4497

Shreyas281299 · 2025-09-24T11:28:52Z

COMPLETES # https://jira-eng-sjc12.cisco.com/jira/browse/CAI-6913

This pull request addresses

The PR adds the support for the shutdown event from the mercury.

by making the following changes

When mercury sends the shutdown event, SDK connects to a new mercury connection using the same mercury url. When the new WS is connected the older one is closed.

Change Type

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Tooling change
Internal code refactor

The following scenarios were tested

Unit Tests
While on a call triggered a shutdown message on the mercury connection and ensure a new connection was setted up without any interruption
While in a meeting triggered a shutdown message on the mercury connection and ensure a new connection was setted up without any interruption
Locally linked SDK with WWC and tested a call and a meeting and triggered a shutdown event.

The GAI Coding Policy And Copyright Annotation Best Practices

I certified that

I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly

Make sure to have followed the contributing guidelines before submitting.

Shreyas281299 · 2025-09-24T11:29:23Z

@CodeRabbit review

aws-amplify-us-east-2 · 2025-09-24T11:38:31Z

This pull request is automatically being deployed by Amplify Hosting (learn more).

Access this pull request here: https://pr-4497.d3m3l2kee0btzx.amplifyapp.com

rarajes2

Are we not adding/updating UTs ?
Please add vidcast

rarajes2 · 2025-10-08T05:49:32Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+
+          if (this.webex.config.defaultMercuryOptions) {
+            this.logger.info(`${this.namespace}: setting custom options for switchover`);
+            options = {...options, ...this.webex.config.defaultMercuryOptions};


Order of spreading the options seems incorrect. The default options should come first so that the later can override it.

Thanks for catching this

rarajes2 · 2025-10-08T05:50:08Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+      }
+      this._shutdownSwitchoverInProgress = true;
+      this._shutdownSwitchoverId = `${Date.now()}`;
+      this.logger.info(


this.logger.log

In this file the deafult has been logger.info for the existint code. I feel we can keep this as info only.

rarajes2 · 2025-10-08T05:50:54Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+          return pendingSocket.open(webSocketUrl, options);
+        })
+        .then(() => {
+          this.logger.info(


Same here. We were keeping info only at the beginning of the function

rarajes2 · 2025-10-08T05:58:47Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+        `${this.namespace}: [shutdown] switchover start, id=${this._shutdownSwitchoverId}`
+      );
+
+      const pendingSocket = new Socket();


Let's rename it newSocket

rarajes2 · 2025-10-08T05:59:22Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+
+      Promise.all([this._prepareUrl(), this.webex.credentials.getUserToken()])
+        .then(([webSocketUrl, token]) => {
+          attemptWSUrl = webSocketUrl;


attemptWSUrl -> newWSUrl

rarajes2 · 2025-10-08T06:17:34Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+      pendingSocket.on('close', (event) => this._onclose(event, pendingSocket));
+      pendingSocket.on('message', (...args) => this._onmessage(...args));
+      pendingSocket.on('pong', (...args) => this._setTimeOffset(...args));
+      pendingSocket.on('sequence-mismatch', (...args) => this._emit('sequence-mismatch', ...args));
+      pendingSocket.on('ping-pong-latency', (...args) => this._emit('ping-pong-latency', ...args));


This seems like repetitive code. We can create a method which accepts the socket param and adds these listeners and call that method in other paces too

Made a new method for adding the event listeners

rarajes2 · 2025-10-08T06:19:35Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+          // Do not force-close oldSocket; server will close it with 4001.
+          if (oldSocket) {
+            this.logger.info(
+              `${this.namespace}: [shutdown] old socket retained; awaiting 4001 close`
+            );
+          }


Q. Is 4001 a status code? We can improve the message

Yes, 4001 is 'replacement of the connection due to shutdown'. Updated the message

rarajes2 · 2025-10-08T06:23:37Z

packages/@webex/internal-plugin-mercury/src/mercury.js

    /* eslint complexity: [0] */

    try {
+      const isActiveSocket = !sourceSocket || (this.socket && sourceSocket === this.socket);


The sourceSocket passed is a new socket created (line number 300) which will never be undefined so the first condition will always return false and proceed with the next condition. Please correct if I am wrong

I think we are already updating the this.socket with the new one or active one. Shouldn't we be able to call the close on it directly ?

So in this condition all we need to know is that

If the closing socket is the current socket (this is a normal closing of mercury connection)

If the closing socket is an older inactive socket (this is the case of mercury shutdown)

Simplified the condition

I think I got confused with the name activeSocket. I thought it means the socket is active but here it means that it is the currently being used socket.

rarajes2 · 2025-10-08T06:33:38Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+      try {
+        this._emit('event:mercury_shutdown_imminent', envelope);
+      } catch (e) {
+        // ignore observer errors
+      }


Same here. Try-catch not needed

rarajes2 · 2025-10-08T06:39:56Z

packages/@webex/internal-plugin-mercury/src/socket/socket-base.js

+        typeof data.sequenceNumber === 'number' ||
+        (typeof data.sequenceNumber === 'string' && data.sequenceNumber.trim() !== '')
+      ) {
+        const sequenceNumber = parseInt(data.sequenceNumber, 10);


Are we getting sequenceNumber in different number system ? We are already checking the type. I think the parsing should have been done before the condition

Kesari3008

Did we test calling flows when the shutdown occurs to see if we have any impact there due to connection replacement ?

packages/@webex/internal-plugin-mercury/src/mercury.js

Kesari3008 · 2025-10-08T14:35:57Z

Please update the description with details of testing done for this change and check the boxes required

Kesari3008

Unit test is failing mercury plugin
Did we test calling flows with mercury shutdown ?

mkesavan13

Did we test this by locally linking the web client once? We need one of WWC member's review as well on this one.

rarajes2 · 2025-10-14T11:28:15Z

packages/@webex/internal-plugin-mercury/src/mercury.js

-      this._emit('offline', event);
-      this.webex.internal.newMetrics.callDiagnosticMetrics.setMercuryConnectedStatus(false);
+      let socketUrl;
+      if (isActiveSocket && this.socket) {


isActiveSocket is already ensuring that the this.socket has some value so we don't need to add both the checks.

Shreyas281299 · 2025-10-15T14:18:25Z

Did we test this by locally linking the web client once? We need one of WWC member's review as well on this one.

Yes, Ive locally linked and tested a call and a meeting in the WWC

Coread · 2025-10-16T12:29:39Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+        .then(([webSocketUrl, token]) => {
+          newWSUrl = webSocketUrl;
+
+          let options = {


these options are a complete duplicate of the code in the open method. This could have been factored out to a private method. In fact, most of what is in this .then could have been factored out. It only really differs by the log strings

Coread · 2025-10-16T12:38:42Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+
+          this.logger.info(`${this.namespace}: [shutdown] switchover url: ${webSocketUrl}`);
+
+          return newSocket.open(webSocketUrl, options);


if the socket fails to open, I think it won't ever have the event listeners removed. You should call removeAllListeners on the socket in the catch

Coread · 2025-10-16T12:44:34Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+      if (isActiveSocket) {
+        // Only tear down state if the currently active socket closed
+        if (this.socket) {
+          this.socket.removeAllListeners();


question: When are the listeners removed from the old socket?

Coread · 2025-10-16T12:48:36Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+
+          const oldSocket = this.socket;
+          // Atomically switch active socket reference without closing the old one.
+          this.socket = newSocket;


There's a potential race condition here. If the old mercury websocket disconnects before it reaches this line, then the plugin will still emit the offline. This code won't emit the online either so the consuming application would not realise that it has reconnected

Coread · 2025-10-16T12:49:42Z

packages/@webex/internal-plugin-mercury/src/mercury.js

+
+          this.logger.info(`${this.namespace}: [shutdown] switchover url: ${webSocketUrl}`);
+
+          return newSocket.open(webSocketUrl, options);


This directly connects the websocket where as the first connection uses the backoff strategy. Is this deliberate? Are we not worried that this reconnection attempt is more fragile than the normal connection?

feat(mercury): handle mercury shutdown

192b226

Shreyas281299 requested review from a team as code owners September 24, 2025 11:28

Merge branch 'next' into mercury-shutdown

75f37e9

Shreyas281299 added the validated If the pull request is validated for automation. label Sep 24, 2025

rarajes2 requested changes Oct 8, 2025

View reviewed changes

Kesari3008 requested changes Oct 8, 2025

View reviewed changes

Shreyas281299 added 3 commits October 9, 2025 19:44

feat(mercury): uts for shutdown protocol changes

0d73a63

fix(mercury): review comments

9b57170

fix(mercury): review-comments

5dc635a

Kesari3008 reviewed Oct 14, 2025

View reviewed changes

mkesavan13 reviewed Oct 14, 2025

View reviewed changes

Kesari3008 approved these changes Oct 14, 2025

View reviewed changes

Shreyas281299 added 2 commits October 14, 2025 14:07

fix(mercury): fix-uts

a253f7a

fix(mercury): revert socket-base changes

727d886

rarajes2 approved these changes Oct 14, 2025

View reviewed changes

Merge remote-tracking branch 'upstream/next' into mercury-shutdown

5c6f47c

fix(mercury): fix tests

e69d1c5

Coread reviewed Oct 16, 2025

View reviewed changes


		this.logger.info(`${this.namespace}: [shutdown] switchover url: ${webSocketUrl}`);

		return newSocket.open(webSocketUrl, options);

feat(mercury): handle mercury shutdown #4497

Are you sure you want to change the base?

feat(mercury): handle mercury shutdown #4497

Uh oh!

Conversation

Shreyas281299 commented Sep 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

COMPLETES # https://jira-eng-sjc12.cisco.com/jira/browse/CAI-6913

This pull request addresses

by making the following changes

Change Type

The following scenarios were tested

The GAI Coding Policy And Copyright Annotation Best Practices

I certified that

Uh oh!

Shreyas281299 commented Sep 24, 2025

Uh oh!

aws-amplify-us-east-2 bot commented Sep 24, 2025

Uh oh!

rarajes2 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kesari3008 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Kesari3008 commented Oct 8, 2025

Uh oh!

Kesari3008 left a comment

Choose a reason for hiding this comment

Uh oh!

mkesavan13 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shreyas281299 commented Oct 15, 2025

Uh oh!

Shreyas281299 commented Sep 24, 2025 •

edited

Loading

Coread Oct 16, 2025 •

edited

Loading