Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 12 additions & 3 deletions packages/@webex/internal-plugin-mercury/src/mercury.js
Original file line number Diff line number Diff line change
Expand Up @@ -901,9 +901,18 @@ 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.logger.info(`${this.namespace}: socket ${sessionId} replaced; will not reconnect`);
if (isActiveSocket) this._emit(sessionId, 'offline.replaced', event);
} else {
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 @@ -1280,6 +1280,74 @@ 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.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.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.notCalled(mercury._emit);
assert.notCalled(mercury._reconnect);
assert.isTrue(mercury.connected);
});
});

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

Expand Down
Loading