Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 46 additions & 3 deletions packages/@webex/internal-plugin-mercury/src/mercury.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import {
} from './errors';

const normalReconnectReasons = ['idle', 'done (forced)', 'pong not received', 'pong mismatch'];
const MERCURY_CLOSE_4000_METRIC = 'JS_SDK_MERCURY_CLOSE_4000';

const Mercury = WebexPlugin.extend({
namespace: 'Mercury',
Expand Down Expand Up @@ -850,6 +851,29 @@ const Mercury = WebexPlugin.extend({
return handlers;
},

_submitMercuryClose4000Metric(sessionId, event, options) {
const {action, isActiveSocket, messageType} = options;

try {
this.webex.internal.metrics.submitClientMetrics(MERCURY_CLOSE_4000_METRIC, {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle the metric submission promise

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 👍 / 👎.

fields: {
action,
close_code: event.code,
close_reason: event.reason || '',
is_active_socket: isActiveSocket,
message_type: messageType,
session_id: sessionId,
},
tags: {
action,
message_type: messageType,
},
});
} catch (error) {
this.logger.warn(`${this.namespace}: failed to submit Mercury 4000 close metric`, error);
}
},

_onclose(sessionId, event, sourceSocket) {
// I don't see any way to avoid the complexity or statement count in here.
/* eslint complexity: [0] */
Expand Down Expand Up @@ -901,9 +925,28 @@ const Mercury = WebexPlugin.extend({
break;
case 4000:
// metric: disconnect
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') {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep bare 4000 closes in the replaced path

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._submitMercuryClose4000Metric(sessionId, event, {
action: 'no_action',
isActiveSocket,
messageType: 'replaced',
});
this.logger.info(`${this.namespace}: socket ${sessionId} replaced; will not reconnect`);
if (isActiveSocket) this._emit(sessionId, 'offline.replaced', event);
} else {
this._submitMercuryClose4000Metric(sessionId, event, {
action: isActiveSocket ? 'reconnect' : 'ignore_non_active',
isActiveSocket,
messageType: 'other',
});
this.logger.info(
`${this.namespace}: socket ${sessionId} disconnected with 4000: ${event.reason}; reconnecting`
);
if (isActiveSocket) {
this._emit(sessionId, 'offline.transient', event);
this._reconnect(socketUrl, sessionId);
}
}
break;
case 4001:
// replaced during shutdown
Expand Down
122 changes: 122 additions & 0 deletions packages/@webex/internal-plugin-mercury/test/unit/spec/mercury.js
Original file line number Diff line number Diff line change
Expand Up @@ -1280,6 +1280,128 @@ describe('plugin-mercury', () => {
});
});

describe('#_onclose() with code 4000', () => {
let mockSocket, anotherSocket;

beforeEach(() => {
mockSocket = {
url: 'ws://active-socket.com',
removeAllListeners: sinon.stub(),
};
anotherSocket = {
url: 'ws://old-socket.com',
removeAllListeners: sinon.stub(),
};
mercury.socket = mockSocket;
mercury.sockets.set(mercury.defaultSessionId, mockSocket);
mercury.connected = true;
sinon.stub(mercury, '_emit');
sinon.stub(mercury, '_reconnect');
sinon.stub(mercury, 'unset');
});

afterEach(() => {
mercury._emit.restore();
mercury._reconnect.restore();
mercury.unset.restore();
});

it('should not reconnect when active socket is replaced', () => {
const closeEvent = {
code: 4000,
reason: 'Replaced',
};

mercury._onclose(mercury.defaultSessionId, closeEvent, mockSocket);

assert.calledOnceWithExactly(
webex.internal.metrics.submitClientMetrics,
'JS_SDK_MERCURY_CLOSE_4000',
{
fields: {
action: 'no_action',
close_code: 4000,
close_reason: 'Replaced',
is_active_socket: true,
message_type: 'replaced',
session_id: mercury.defaultSessionId,
},
tags: {
action: 'no_action',
message_type: 'replaced',
},
}
);
assert.calledWith(mercury._emit, mercury.defaultSessionId, 'offline.replaced', closeEvent);
assert.notCalled(mercury._reconnect);
});

it('should reconnect active socket when 4000 has an unknown reason', () => {
const closeEvent = {
code: 4000,
reason: 'Unexpected close',
};

mercury._onclose(mercury.defaultSessionId, closeEvent, mockSocket);

assert.calledOnceWithExactly(
webex.internal.metrics.submitClientMetrics,
'JS_SDK_MERCURY_CLOSE_4000',
{
fields: {
action: 'reconnect',
close_code: 4000,
close_reason: 'Unexpected close',
is_active_socket: true,
message_type: 'other',
session_id: mercury.defaultSessionId,
},
tags: {
action: 'reconnect',
message_type: 'other',
},
}
);
assert.calledWith(mercury._emit, mercury.defaultSessionId, 'offline.transient', closeEvent);
assert.calledWith(
mercury._reconnect,
mockSocket.url,
mercury.defaultSessionId
);
});

it('should not reconnect a non-active socket with an unknown 4000 reason', () => {
const closeEvent = {
code: 4000,
reason: 'Unexpected close',
};

mercury._onclose(mercury.defaultSessionId, closeEvent, anotherSocket);

assert.calledOnceWithExactly(
webex.internal.metrics.submitClientMetrics,
'JS_SDK_MERCURY_CLOSE_4000',
{
fields: {
action: 'ignore_non_active',
close_code: 4000,
close_reason: 'Unexpected close',
is_active_socket: false,
message_type: 'other',
session_id: mercury.defaultSessionId,
},
tags: {
action: 'ignore_non_active',
message_type: 'other',
},
}
);
assert.notCalled(mercury._emit);
assert.notCalled(mercury._reconnect);
assert.isTrue(mercury.connected);
});
});

describe('#_onclose() with code 4001 (shutdown replacement)', () => {
let mockSocket, anotherSocket;

Expand Down
Loading