Skip to content
Merged
Show file tree
Hide file tree
Changes from 5 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 @@ -882,6 +883,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, {
Comment thread
rsarika marked this conversation as resolved.
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 @@ -938,9 +962,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') {
Comment thread
rsarika marked this conversation as resolved.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nitpick: The comparison reason === 'replaced' is case-sensitive. If there's any chance the server could send 'Replaced' or 'REPLACED', a safer check would be:
if (reason?.toLowerCase() === 'replaced')

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

yes, its already there at declaration const reason = event.reason && event.reason.toLowerCase();

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
Original file line number Diff line number Diff line change
Expand Up @@ -267,8 +267,14 @@ describe('plugin-mercury', () => {
},
{
code: 4000,
reason: 'Replaced',
action: 'replace',
},
{
code: 4000,
reason: 'Unexpected close',
action: 'reconnect',
},
{
action: 'close',
},
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 @@ -1334,6 +1334,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
);
Comment thread
rsarika marked this conversation as resolved.
});

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