From 6aa91b5666e49cfb5ca2c259fac1abe224ecc4bb Mon Sep 17 00:00:00 2001 From: tsm-odoo Date: Mon, 3 Oct 2022 14:09:05 +0000 Subject: [PATCH] [FIX] bus: reconnect websocket after user log in/out MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this PR, a websocket would wait for the server to close it with a `SessionExpired` close code to be refreshed after the user logged in/out. Indeed, the server is responsible to check for outdated sessions on incoming/outgoing messages. This is problematic: a user logging in with more than one tab opened, would have to wait to receive its messages. This PR solves this issue by refreshing the connection immediately in this scenario. closes odoo/odoo#102420 X-original-commit: 225f80882a130ae63af193adef412e6b993f2c11 Signed-off-by: Sébastien Theys (seb) Signed-off-by: Stockbauer Matthieu (tsm) --- addons/bus/static/src/services/bus_service.js | 30 ++++++- .../static/src/workers/websocket_worker.js | 26 ++++++- addons/bus/static/tests/bus_tests.js | 78 ++++++++++++++++++- 3 files changed, 126 insertions(+), 8 deletions(-) diff --git a/addons/bus/static/src/services/bus_service.js b/addons/bus/static/src/services/bus_service.js index 4354bdd6c41..1bc207b33d3 100644 --- a/addons/bus/static/src/services/bus_service.js +++ b/addons/bus/static/src/services/bus_service.js @@ -64,16 +64,38 @@ export const busService = { bus.trigger(type, data); } + /** + * Initialize the connection to the worker by sending it usefull + * initial informations (last notification id, debug mode, + * ...). + */ + function initializeConnection() { + // User_id has different values according to its origin: + // - frontend: number or false, + // - backend: array with only one number + // - guest page: array containing null or number + // - public pages: undefined + // Let's format it in order to ease its usage: + // - number if user is logged, false otherwise, keep + // undefined to indicate session_info is not available. + let uid = Array.isArray(session.user_id) ? session.user_id[0] : session.user_id; + if (!uid && uid !== undefined) { + uid = false; + } + send('initialize_connection', { + debug: odoo.debug, + lastNotificationId: multiTab.getSharedValue('last_notification_id', 0), + uid, + }); + } + if ('SharedWorker' in window) { worker.port.start(); worker.port.addEventListener('message', handleMessage); } else { worker.addEventListener('message', handleMessage); } - send('initialize_connection', { - debug: odoo.debug, - lastNotificationId: multiTab.getSharedValue('last_notification_id', 0), - }); + initializeConnection(); browser.addEventListener('pagehide', ({ persisted }) => { if (!persisted) { // Page is gonna be unloaded, disconnect this client diff --git a/addons/bus/static/src/workers/websocket_worker.js b/addons/bus/static/src/workers/websocket_worker.js index fa91b205983..31c05b015c3 100644 --- a/addons/bus/static/src/workers/websocket_worker.js +++ b/addons/bus/static/src/workers/websocket_worker.js @@ -11,7 +11,7 @@ import { debounce } from '@bus/workers/websocket_worker_utils'; /** * Type of action that can be sent from the client to the worker. * - * @typedef {'add_channel' | 'delete_channel' | 'force_update_channels' | 'initialize_connection', 'send' | 'leave' } WorkerAction + * @typedef {'add_channel' | 'delete_channel' | 'force_update_channels' | 'initialize_connection' | 'send' | 'leave' } WorkerAction */ export const WEBSOCKET_CLOSE_CODES = Object.freeze({ @@ -30,6 +30,7 @@ export const WEBSOCKET_CLOSE_CODES = Object.freeze({ BAD_GATEWAY: 1014, SESSION_EXPIRED: 4001, KEEP_ALIVE_TIMEOUT: 4002, + RECONNECTING: 4003, }); /** @@ -42,6 +43,8 @@ export const WEBSOCKET_CLOSE_CODES = Object.freeze({ export class WebsocketWorker { constructor(websocketURL) { this.websocketURL = websocketURL; + this.currentUID = null; + this.isWaitingForNewUID = true; this.channelsByClient = new Map(); this.connectRetryDelay = 1000; this.connectTimeout = null; @@ -193,12 +196,26 @@ export class WebsocketWorker { * given client. * @param {Number} [param0.lastNotificationId] Last notification id * known by the client. + * @param {Number|false|undefined} [param0.uid] Current user id + * - Number: user is logged whether on the frontend/backend. + * - false: user is not logged. + * - undefined: not available (e.g. livechat support page) */ - _initializeConnection(client, { debug, lastNotificationId }) { + _initializeConnection(client, { debug, lastNotificationId, uid }) { this.lastNotificationId = lastNotificationId; this.debugModeByClient[client] = debug; this.isDebug = Object.values(this.debugModeByClient).some(debugValue => debugValue !== ''); - this._updateChannels(); + const isCurrentUserKnown = uid !== undefined; + if (this.isWaitingForNewUID && isCurrentUserKnown) { + this.isWaitingForNewUID = false; + this.currentUID = uid; + } + if (this.currentUID === uid || !isCurrentUserKnown) { + this._updateChannels(); + } else if (this._isWebsocketConnected()) { + this.currentUID = uid; + this.websocket.close(WEBSOCKET_CLOSE_CODES.RECONNECTING); + } } /** @@ -244,6 +261,9 @@ export class WebsocketWorker { // Don't wait to reconnect on keep alive timeout. this.connectRetryDelay = 0; } + if (code === WEBSOCKET_CLOSE_CODES.SESSION_EXPIRED) { + this.isWaitingForNewUID = true; + } this._retryConnectionWithDelay(); } diff --git a/addons/bus/static/tests/bus_tests.js b/addons/bus/static/tests/bus_tests.js index ca13a8bcf2d..8e1ea41c100 100644 --- a/addons/bus/static/tests/bus_tests.js +++ b/addons/bus/static/tests/bus_tests.js @@ -370,6 +370,82 @@ QUnit.module('Bus', { await updateLastNotificationDeferred; assert.verifySteps([`initialize_connection - 0`]); }); + + QUnit.test('Websocket reconnects upon user log out', async function (assert) { + // first tab connects to the worker with user logged. + patchWithCleanup(session, { + user_id: 1, + }); + const connectionInitializedDeferred = makeDeferred(); + const connectionRefreshedDeferred = makeDeferred(); + patchWebsocketWorkerWithCleanup({ + _initializeConnection(client, data) { + this._super(client, data); + connectionInitializedDeferred.resolve(); + }, + }); + + const firstTabEnv = await makeTestEnv(); + firstTabEnv.services['bus_service'].addEventListener('reconnect', () => { + assert.step('reconnect'); + connectionRefreshedDeferred.resolve(); + }); + firstTabEnv.services['bus_service'].addEventListener('disconnect', () => { + assert.step('disconnect'); + }); + await connectionInitializedDeferred; + + // second tab connects to the worker after disconnection: user_id + // is now false. + patchWithCleanup(session, { + user_id: false, + }); + await makeTestEnv(); + await connectionRefreshedDeferred; + + assert.verifySteps([ + 'disconnect', + 'reconnect', + ]); + }); + + QUnit.test('Websocket reconnects upon user log in', async function (assert) { + // first tab connects to the worker with no user logged. + patchWithCleanup(session, { + user_id: false, + }); + const connectionInitializedDeferred = makeDeferred(); + const connectionRefreshedDeferred = makeDeferred(); + patchWebsocketWorkerWithCleanup({ + _initializeConnection(client, data) { + this._super(client, data); + connectionInitializedDeferred.resolve(); + }, + }); + + const firstTabEnv = await makeTestEnv(); + firstTabEnv.services['bus_service'].addEventListener('reconnect', () => { + assert.step('reconnect'); + connectionRefreshedDeferred.resolve(); + }); + firstTabEnv.services['bus_service'].addEventListener('disconnect', () => { + assert.step('disconnect'); + }); + await connectionInitializedDeferred; + + // second tab connects to the worker after connection: user_id + // is now set. + patchWithCleanup(session, { + user_id: 1, + }); + await makeTestEnv(); + await connectionRefreshedDeferred; + + assert.verifySteps([ + 'disconnect', + 'reconnect', + ]); + }); +}); }); -});