From fa028a18dff8cb24a97747e53382f07fed9a6adb Mon Sep 17 00:00:00 2001 From: tsm-odoo Date: Thu, 26 Jan 2023 16:08:17 +0000 Subject: [PATCH] [FIX] bus: make bus_service recover faster from offline/online switch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Before this PR, recovering from an offline/online switch could take up to 20mn. Indeed, WebSocket are based on TCP that uses FIN packets to close the connection. In case of a sudden loss of internet connection, the client is unaware of the already dead TCP connection. It is up to the browser to assume the connection is dead after some time. This commit solves the issue by closing the connection properly upon the reception of the offline event. When the client is back online, the connection is opened again thus allowing an almost instant recovery. At the same time, this PR increases the default keep_alive_timeout for websocket connections. Indeed, those re-connections lead to cursor usage bursts and are not required so frequently. closes odoo/odoo#111274 X-original-commit: d3a3792e5247cc41a58b47d148d4adf4fb0e4631 Signed-off-by: Alexandre Kühn (aku) Signed-off-by: Sébastien Theys (seb) --- addons/bus/static/src/services/bus_service.js | 18 +++++++++++-- .../static/src/workers/websocket_worker.js | 25 +++++++++++++++---- addons/bus/static/tests/bus_tests.js | 25 +++++++++++++++++++ .../static/tests/helpers/mock_websocket.js | 2 +- odoo/tools/config.py | 2 +- 5 files changed, 63 insertions(+), 9 deletions(-) diff --git a/addons/bus/static/src/services/bus_service.js b/addons/bus/static/src/services/bus_service.js index 74b86935814..71640baa450 100644 --- a/addons/bus/static/src/services/bus_service.js +++ b/addons/bus/static/src/services/bus_service.js @@ -25,6 +25,7 @@ export const busService = { async start(env, { multi_tab: multiTab, "bus.parameters": params }) { const bus = new EventBus(); + let isActive = false; let workerURL = `${params.serverURL}/bus/websocket_worker_bundle?v=${WORKER_VERSION}`; if (params.serverURL !== window.origin) { // Bus service is loaded from a different origin than the bundle @@ -121,6 +122,12 @@ export const busService = { send('leave'); } }); + browser.addEventListener('online', () => { + if (isActive) { + send('start'); + } + }); + browser.addEventListener('offline', () => send('stop')); await connectionInitializedDeferred; return { @@ -128,14 +135,21 @@ export const busService = { addChannel: channel => { send('add_channel', channel); send('start'); + isActive = true; }, deleteChannel: channel => send('delete_channel', channel), forceUpdateChannels: () => send('force_update_channels'), trigger: bus.trigger.bind(bus), removeEventListener: bus.removeEventListener.bind(bus), send: (eventName, data) => send('send', { event_name: eventName, data }), - start: () => send('start'), - stop: () => send('leave'), + start: () => { + send('start'); + isActive = true; + }, + stop: () => { + send('leave'); + isActive = false; + }, }; }, }; diff --git a/addons/bus/static/src/workers/websocket_worker.js b/addons/bus/static/src/workers/websocket_worker.js index a2a3437f545..1a8d23cb9e3 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' | 'stop' | 'start' } WorkerAction */ export const WEBSOCKET_CLOSE_CODES = Object.freeze({ @@ -34,7 +34,8 @@ export const WEBSOCKET_CLOSE_CODES = Object.freeze({ }); // Should be incremented on every worker update in order to force // update of the worker in browser cache. -export const WORKER_VERSION = "1.0.1"; +export const WORKER_VERSION = '1.0.2'; +const INITIAL_RECONNECT_DELAY = 1000; /** * This class regroups the logic necessary in order for the @@ -49,7 +50,7 @@ export class WebsocketWorker { this.currentUID = null; this.isWaitingForNewUID = true; this.channelsByClient = new Map(); - this.connectRetryDelay = 1000; + this.connectRetryDelay = INITIAL_RECONNECT_DELAY; this.connectTimeout = null; this.debugModeByClient = new Map(); this.isDebug = false; @@ -122,6 +123,8 @@ export class WebsocketWorker { return this._sendToServer(data); case 'start': return this._start(); + case 'stop': + return this._stop(); case 'leave': return this._unregisterClient(client); case 'add_channel': @@ -322,7 +325,7 @@ export class WebsocketWorker { this.messageWaitQueue.forEach(msg => this.websocket.send(msg)); this.messageWaitQueue = []; this.broadcast(this.isReconnecting ? 'reconnect' : 'connect'); - this.connectRetryDelay = 0; + this.connectRetryDelay = INITIAL_RECONNECT_DELAY; this.connectTimeout = null; this.isReconnecting = false; } @@ -332,7 +335,7 @@ export class WebsocketWorker { * applied to the reconnect attempts. */ _retryConnectionWithDelay() { - this.connectRetryDelay = this.connectRetryDelay * 1.5 + 500 * Math.random(); + this.connectRetryDelay = this.connectRetryDelay * 1.5 + 1000 * Math.random(); this.connectTimeout = setTimeout(this._start.bind(this), this.connectRetryDelay); } @@ -366,6 +369,18 @@ export class WebsocketWorker { this.websocket.addEventListener('close', this._onWebsocketClose.bind(this)); } + /** + * Stop the worker. + */ + _stop() { + clearTimeout(this.connectTimeout); + this.connectRetryDelay = INITIAL_RECONNECT_DELAY; + this.isReconnecting = false; + if (this.websocket) { + this.websocket.close(); + } + } + /** * Update the channel subscription on the server. Ignore if the channels * did not change since the last subscription. diff --git a/addons/bus/static/tests/bus_tests.js b/addons/bus/static/tests/bus_tests.js index 377688a21b3..47c20494afb 100644 --- a/addons/bus/static/tests/bus_tests.js +++ b/addons/bus/static/tests/bus_tests.js @@ -455,6 +455,31 @@ QUnit.module('Bus', { await websocketCreatedDeferred; assert.verifySteps([`${serverURL.replace("http", "ws")}/websocket`]); }); + + QUnit.test("Disconnect on offline, re-connect on online", async function (assert) { + patchWebsocketWorkerWithCleanup(); + const env = await makeTestEnv(); + env.services["bus_service"].addEventListener("connect", () => assert.step("connect")); + env.services["bus_service"].addEventListener("disconnect", () => assert.step("disconnect")); + env.services["bus_service"].start(); + window.dispatchEvent(new Event("offline")); + await nextTick(); + window.dispatchEvent(new Event("online")); + await nextTick(); + assert.verifySteps(["connect", "disconnect", "connect"]); + }); + + QUnit.test("No disconnect on change offline/online when bus inactive", async function (assert) { + patchWebsocketWorkerWithCleanup(); + const env = await makeTestEnv(); + env.services["bus_service"].addEventListener("connect", () => assert.step("connect")); + env.services["bus_service"].addEventListener("disconnect", () => assert.step("disconnect")); + window.dispatchEvent(new Event("offline")); + await nextTick(); + window.dispatchEvent(new Event("online")); + await nextTick(); + assert.verifySteps([]); + }); }); }); diff --git a/addons/bus/static/tests/helpers/mock_websocket.js b/addons/bus/static/tests/helpers/mock_websocket.js index 23730eed884..3320f6db05e 100644 --- a/addons/bus/static/tests/helpers/mock_websocket.js +++ b/addons/bus/static/tests/helpers/mock_websocket.js @@ -18,7 +18,7 @@ class WebSocketMock extends EventTarget { }); } - close(code, reason) { + close(code = 1000, reason) { this.readyState = 3; const closeEv = new CloseEvent('close', { code, diff --git a/odoo/tools/config.py b/odoo/tools/config.py index 74d2c37a1fe..37731d7c4ec 100644 --- a/odoo/tools/config.py +++ b/odoo/tools/config.py @@ -77,7 +77,7 @@ class configmanager(object): 'publisher_warranty_url': 'http://services.openerp.com/publisher-warranty/', 'reportgz': False, 'root_path': None, - 'websocket_keep_alive_timeout': 600, + 'websocket_keep_alive_timeout': 3600, 'websocket_rate_limit_burst': 10, 'websocket_rate_limit_delay': 0.2, }