Skip to content

Commit

Permalink
fix(mercury): fix logout mercury delete (webex#3878)
Browse files Browse the repository at this point in the history
  • Loading branch information
chrisadubois authored Oct 3, 2024
1 parent b4e565c commit 83a3b17
Show file tree
Hide file tree
Showing 6 changed files with 196 additions and 11 deletions.
6 changes: 6 additions & 0 deletions packages/@webex/internal-plugin-mercury/src/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,11 @@ export default {
* @type {[type]}
*/
forceCloseDelay: process.env.MERCURY_FORCE_CLOSE_DELAY || 2000,
/**
* When logging out, use default reason which can trigger a reconnect,
* or set to something else, like `done (permanent)` to prevent reconnect
* @type {String}
*/
beforeLogoutOptionsCloseReason: process.env.MERCURY_LOGOUT_REASON || 'done (forced)',
},
};
2 changes: 1 addition & 1 deletion packages/@webex/internal-plugin-mercury/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import config from './config';
registerInternalPlugin('mercury', Mercury, {
config,
onBeforeLogout() {
return this.disconnect();
return this.logout();
},
});

Expand Down
18 changes: 15 additions & 3 deletions packages/@webex/internal-plugin-mercury/src/mercury.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,8 +93,17 @@ const Mercury = WebexPlugin.extend({
});
},

logout() {
return this.disconnect(
this.config.beforeLogoutOptionsCloseReason &&
!normalReconnectReasons.includes(this.config.beforeLogoutOptionsCloseReason)
? {code: 1050, reason: this.config.beforeLogoutOptionsCloseReason}
: undefined
);
},

@oneFlight
disconnect() {
disconnect(options) {
return new Promise((resolve) => {
if (this.backoffCall) {
this.logger.info(`${this.namespace}: aborting connection`);
Expand All @@ -104,7 +113,7 @@ const Mercury = WebexPlugin.extend({
if (this.socket) {
this.socket.removeAllListeners('message');
this.once('offline', resolve);
resolve(this.socket.close());
resolve(this.socket.close(options || undefined));
}

resolve();
Expand Down Expand Up @@ -446,14 +455,17 @@ const Mercury = WebexPlugin.extend({
// if (code == 1011 && reason !== ping error) metric: unexpected disconnect
break;
case 1000:
case 1050: // 1050 indicates logout form of closure, default to old behavior, use config reason defined by consumer to proceed with the permanent block
if (normalReconnectReasons.includes(reason)) {
this.logger.info(`${this.namespace}: socket disconnected; reconnecting`);
this._emit('offline.transient', event);
this._reconnect(socketUrl);
// metric: disconnect
// if (reason === done forced) metric: force closure
} else {
this.logger.info(`${this.namespace}: socket disconnected; will not reconnect`);
this.logger.info(
`${this.namespace}: socket disconnected; will not reconnect: ${event.reason}`
);
this._emit('offline.permanent', event);
}
break;
Expand Down
23 changes: 17 additions & 6 deletions packages/@webex/internal-plugin-mercury/src/socket/socket-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,8 +122,15 @@ export default class Socket extends EventEmitter {
}

options = options || {};
if (options.code && options.code !== 1000 && (options.code < 3000 || options.code > 4999)) {
reject(new Error('`options.code` must be 1000 or between 3000 and 4999 (inclusive)'));
if (
options.code &&
options.code !== 1000 &&
options.code !== 1050 &&
(options.code < 3000 || options.code > 4999)
) {
reject(
new Error('`options.code` must be 1000 or 1050 or between 3000 and 4999 (inclusive)')
);

return;
}
Expand All @@ -137,10 +144,14 @@ export default class Socket extends EventEmitter {
try {
this.logger.info(`socket,${this._domain}: no close event received, forcing closure`);
resolve(
this.onclose({
code: 1000,
reason: 'Done (forced)',
})
this.onclose(
options.code === 1050
? {code: 1050, reason: options.reason}
: {
code: 1000,
reason: 'Done (forced)',
}
)
);
} catch (error) {
this.logger.warn(`socket,${this._domain}: force-close failed`, error);
Expand Down
50 changes: 50 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 @@ -503,6 +503,35 @@ describe('plugin-mercury', () => {
});
});

describe('#logout()', () => {
it('calls disconnect', () => {
sinon.stub(mercury, 'disconnect');
mercury.logout();
assert.called(mercury.disconnect);
});

it('uses the config.beforeLogoutOptionsCloseReason to disconnect and will send code 1050 for logout', () => {
sinon.stub(mercury, 'disconnect');
mercury.config.beforeLogoutOptionsCloseReason = 'done (permanent)';
mercury.logout();
assert.calledWith(mercury.disconnect, {code: 1050, reason: 'done (permanent)'});
});

it('uses the config.beforeLogoutOptionsCloseReason to disconnect and will send code 1050 for logout if the reason is different than standard', () => {
sinon.stub(mercury, 'disconnect');
mercury.config.beforeLogoutOptionsCloseReason = 'test';
mercury.logout();
assert.calledWith(mercury.disconnect, {code: 1050, reason: 'test'});
});

it('uses the config.beforeLogoutOptionsCloseReason to disconnect and will send undefined for logout if the reason is same as standard', () => {
sinon.stub(mercury, 'disconnect');
mercury.config.beforeLogoutOptionsCloseReason = 'done (forced)';
mercury.logout();
assert.calledWith(mercury.disconnect, undefined);
});
});

describe('#disconnect()', () => {
it('disconnects the WebSocket', () =>
mercury
Expand All @@ -525,6 +554,27 @@ describe('plugin-mercury', () => {
assert.isUndefined(mercury.mockWebSocket, 'Mercury does not have a mockWebSocket');
}));

it('disconnects the WebSocket with code 1050', () =>
mercury
.connect()
.then(() => {
assert.isTrue(mercury.connected, 'Mercury is connected');
assert.isFalse(mercury.connecting, 'Mercury is not connecting');
const promise = mercury.disconnect();

mockWebSocket.emit('close', {
code: 1050,
reason: 'done (permanent)',
});

return promise;
})
.then(() => {
assert.isFalse(mercury.connected, 'Mercury is not connected');
assert.isFalse(mercury.connecting, 'Mercury is not connecting');
assert.isUndefined(mercury.mockWebSocket, 'Mercury does not have a mockWebSocket');
}));

it('stops emitting message events', () => {
const spy = sinon.spy();

Expand Down
108 changes: 107 additions & 1 deletion packages/@webex/internal-plugin-mercury/test/unit/spec/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -452,7 +452,7 @@ describe('plugin-mercury', () => {
Promise.all([
assert.isRejected(
socket.close({code: 1001}),
/`options.code` must be 1000 or between 3000 and 4999 \(inclusive\)/
/`options.code` must be 1000 or 1050 or between 3000 and 4999 \(inclusive\)/
),
socket.close({code: 1000}),
]));
Expand All @@ -465,6 +465,14 @@ describe('plugin-mercury', () => {
})
.then(() => assert.calledWith(mockWebSocket.close, 3001, 'Custom Normal')));

it('accepts the logout reason', () =>
socket
.close({
code: 1050,
reason: 'done (permanent)',
})
.then(() => assert.calledWith(mockWebSocket.close, 1050, 'done (permanent)')));

it('can safely be called called multiple times', () => {
const p1 = socket.close();

Expand Down Expand Up @@ -513,6 +521,84 @@ describe('plugin-mercury', () => {
});
});

it('signals closure if no close frame is received within the specified window, but uses the initial options as 1050 if specified by options call', () => {
const socket = new Socket();
const promise = socket.open('ws://example.com', mockoptions);

mockWebSocket.readyState = 1;
mockWebSocket.emit('open');
mockWebSocket.emit('message', {
data: JSON.stringify({
id: uuid.v4(),
data: {
eventType: 'mercury.buffer_state',
},
}),
});

return promise.then(() => {
const spy = sinon.spy();

socket.on('close', spy);
mockWebSocket.close = () =>
new Promise(() => {
/* eslint no-inline-comments: [0] */
});
mockWebSocket.removeAllListeners('close');

const promise = socket.close({code: 1050, reason: 'done (permanent)'});

clock.tick(mockoptions.forceCloseDelay);

return promise.then(() => {
assert.called(spy);
assert.calledWith(spy, {
code: 1050,
reason: 'done (permanent)',
});
});
});
});

it('signals closure if no close frame is received within the specified window, and uses default options as 1000 if the code is not 1050', () => {
const socket = new Socket();
const promise = socket.open('ws://example.com', mockoptions);

mockWebSocket.readyState = 1;
mockWebSocket.emit('open');
mockWebSocket.emit('message', {
data: JSON.stringify({
id: uuid.v4(),
data: {
eventType: 'mercury.buffer_state',
},
}),
});

return promise.then(() => {
const spy = sinon.spy();

socket.on('close', spy);
mockWebSocket.close = () =>
new Promise(() => {
/* eslint no-inline-comments: [0] */
});
mockWebSocket.removeAllListeners('close');

const promise = socket.close({code: 1000, reason: 'test'});

clock.tick(mockoptions.forceCloseDelay);

return promise.then(() => {
assert.called(spy);
assert.calledWith(spy, {
code: 1000,
reason: 'Done (forced)',
});
});
});
});

it('cancels any outstanding ping/pong timers', () => {
mockWebSocket.send = sinon.stub();
socket._ping.resetHistory();
Expand Down Expand Up @@ -618,6 +704,26 @@ describe('plugin-mercury', () => {
}
);
});

describe('when it receives close code 1050', () => {
it(`emits code 1050 for code 1050`, () => {
const code = 1050;
const reason = 'done (permanent)';
const spy = sinon.spy();

socket.on('close', spy);

mockWebSocket.emit('close', {
code,
reason,
});
assert.called(spy);
assert.calledWith(spy, {
code,
reason,
});
});
});
});

describe('#onmessage()', () => {
Expand Down

0 comments on commit 83a3b17

Please sign in to comment.