From 83a3b1726a8518356b03bb5f61da8751288a83e5 Mon Sep 17 00:00:00 2001 From: chrisadubois Date: Thu, 3 Oct 2024 01:40:48 -0700 Subject: [PATCH] fix(mercury): fix logout mercury delete (#3878) --- .../internal-plugin-mercury/src/config.js | 6 + .../internal-plugin-mercury/src/index.js | 2 +- .../internal-plugin-mercury/src/mercury.js | 18 ++- .../src/socket/socket-base.js | 23 +++- .../test/unit/spec/mercury.js | 50 ++++++++ .../test/unit/spec/socket.js | 108 +++++++++++++++++- 6 files changed, 196 insertions(+), 11 deletions(-) diff --git a/packages/@webex/internal-plugin-mercury/src/config.js b/packages/@webex/internal-plugin-mercury/src/config.js index 33d5db52868..a685b21ed44 100644 --- a/packages/@webex/internal-plugin-mercury/src/config.js +++ b/packages/@webex/internal-plugin-mercury/src/config.js @@ -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)', }, }; diff --git a/packages/@webex/internal-plugin-mercury/src/index.js b/packages/@webex/internal-plugin-mercury/src/index.js index 73aff3d62e4..693bd3b8fad 100644 --- a/packages/@webex/internal-plugin-mercury/src/index.js +++ b/packages/@webex/internal-plugin-mercury/src/index.js @@ -14,7 +14,7 @@ import config from './config'; registerInternalPlugin('mercury', Mercury, { config, onBeforeLogout() { - return this.disconnect(); + return this.logout(); }, }); diff --git a/packages/@webex/internal-plugin-mercury/src/mercury.js b/packages/@webex/internal-plugin-mercury/src/mercury.js index c74615ba33a..ed7b706130c 100644 --- a/packages/@webex/internal-plugin-mercury/src/mercury.js +++ b/packages/@webex/internal-plugin-mercury/src/mercury.js @@ -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`); @@ -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(); @@ -446,6 +455,7 @@ 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); @@ -453,7 +463,9 @@ const Mercury = WebexPlugin.extend({ // 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; diff --git a/packages/@webex/internal-plugin-mercury/src/socket/socket-base.js b/packages/@webex/internal-plugin-mercury/src/socket/socket-base.js index aaf953d4ac1..02bbe25d5a1 100644 --- a/packages/@webex/internal-plugin-mercury/src/socket/socket-base.js +++ b/packages/@webex/internal-plugin-mercury/src/socket/socket-base.js @@ -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; } @@ -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); diff --git a/packages/@webex/internal-plugin-mercury/test/unit/spec/mercury.js b/packages/@webex/internal-plugin-mercury/test/unit/spec/mercury.js index adf4f527a61..6e772c301b9 100644 --- a/packages/@webex/internal-plugin-mercury/test/unit/spec/mercury.js +++ b/packages/@webex/internal-plugin-mercury/test/unit/spec/mercury.js @@ -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 @@ -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(); diff --git a/packages/@webex/internal-plugin-mercury/test/unit/spec/socket.js b/packages/@webex/internal-plugin-mercury/test/unit/spec/socket.js index b34f09bdc82..39b4f9f3da1 100644 --- a/packages/@webex/internal-plugin-mercury/test/unit/spec/socket.js +++ b/packages/@webex/internal-plugin-mercury/test/unit/spec/socket.js @@ -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}), ])); @@ -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(); @@ -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(); @@ -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()', () => {