[FIX] bus: do not handle events of outdated websockets

Before this commit, late close events were not handled properly. This
could have led to non-reconnecting websockets. The problematic scheme
is the following:
- Close the socket (eg. upon the reception of an `offline` event), let's assume that
the other end will not perform the closing handshake, the connection will be
closed once the browser presumes it is dead.
- Create a new socket (eg. upon the reception of an `online` event)
- The browser assumes the connection is dead and dispatches a `close` event,
the worker switches to the `reconnecting` state and expects an `open` event  to
update its state to connected. Since there is already a running socket, the worker
won't open a new one and will never receive the `open` event.
- Server closes the connection  (eg. `KEEP_ALIVE_TIMEOUT`)
- The close handler is called but since it is in the `reconnecting` state, it assumes
it shouldn't do anything thus, no reconnect attempt is made.

This PR fixed this issue by ignoring events linked to outdated sockets.

closes odoo/odoo#112456

X-original-commit: a2454739156d8742f7606f090126f32735fb15df
Signed-off-by: Sébastien Theys (seb) <seb@odoo.com>

addons/bus/static/src/workers/websocket_worker.js

@@ -34,7 +34,7 @@ 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.3';
+export const WORKER_VERSION = '1.0.4';
 const INITIAL_RECONNECT_DELAY = 1000;
 const MAXIMUM_RECONNECT_DELAY = 60000;
 
@@ -60,6 +60,11 @@ export class WebsocketWorker {
         this.lastNotificationId = 0;
         this.messageWaitQueue = [];
         this._forceUpdateChannels = debounce(this._forceUpdateChannels, 300, true);
+
+        this._onWebsocketClose = this._onWebsocketClose.bind(this);
+        this._onWebsocketError = this._onWebsocketError.bind(this);
+        this._onWebsocketMessage = this._onWebsocketMessage.bind(this);
+        this._onWebsocketOpen = this._onWebsocketOpen.bind(this);
     }
 
     //--------------------------------------------------------------------------
@@ -250,6 +255,16 @@ export class WebsocketWorker {
         return this.websocket && this.websocket.readyState === 0;
     }
 
+    /**
+     * Determine whether or not the websocket associated to this worker
+     * is in the closing state.
+     *
+     * @returns {boolean}
+     */
+    _isWebsocketClosing() {
+        return this.websocket && this.websocket.readyState === 2;
+    }
+
     /**
      * Triggered when a connection is closed. If closure was not clean ,
      * try to reconnect after indicating to the clients that the
@@ -363,11 +378,23 @@ export class WebsocketWorker {
         if (this._isWebsocketConnected() || this._isWebsocketConnecting()) {
             return;
         }
+        if (this.websocket) {
+            this.websocket.removeEventListener('open', this._onWebsocketOpen);
+            this.websocket.removeEventListener('message', this._onWebsocketMessage);
+            this.websocket.removeEventListener('error', this._onWebsocketError);
+            this.websocket.removeEventListener('close', this._onWebsocketClose);
+        }
+        if (this._isWebsocketClosing()) {
+            // close event was not triggered and will never be, broadcast the
+            // disconnect event for consistency sake.
+            this.lastChannelSubscription = null;
+            this.broadcast("disconnect", { code: WEBSOCKET_CLOSE_CODES.ABNORMAL_CLOSURE });
+        }
         this.websocket = new WebSocket(this.websocketURL);
-        this.websocket.addEventListener('open', this._onWebsocketOpen.bind(this));
-        this.websocket.addEventListener('error', this._onWebsocketError.bind(this));
-        this.websocket.addEventListener('message', this._onWebsocketMessage.bind(this));
-        this.websocket.addEventListener('close', this._onWebsocketClose.bind(this));
+        this.websocket.addEventListener('open', this._onWebsocketOpen);
+        this.websocket.addEventListener('error', this._onWebsocketError);
+        this.websocket.addEventListener('message', this._onWebsocketMessage);
+        this.websocket.addEventListener('close', this._onWebsocketClose);
     }
 
     /**

addons/bus/static/tests/bus_tests.js

@@ -480,6 +480,73 @@ QUnit.module('Bus', {
         await nextTick();
         assert.verifySteps([]);
     });
+
+    QUnit.test("Can reconnect after late close event", async function (assert) {
+        let subscribeSent = 0;
+        const closeDeferred = makeDeferred();
+        let openDeferred = makeDeferred();
+        const worker = patchWebsocketWorkerWithCleanup({
+            _onWebsocketOpen() {
+                this._super();
+                openDeferred.resolve();
+            },
+            _sendToServer({ event_name }) {
+                if (event_name === "subscribe") {
+                    subscribeSent++;
+                }
+            },
+        });
+        const pyEnv = await startServer();
+        const env = await makeTestEnv();
+        env.services["bus_service"].start();
+        await openDeferred;
+        patchWithCleanup(worker.websocket, {
+            close(code = WEBSOCKET_CLOSE_CODES.CLEAN, reason) {
+                this.readyState = 2;
+                const _super = this._super;
+                if (code === WEBSOCKET_CLOSE_CODES.CLEAN) {
+                    closeDeferred.then(() => {
+                        // Simulate that the connection could not be closed cleanly.
+                        _super(WEBSOCKET_CLOSE_CODES.ABNORMAL_CLOSURE, reason);
+                    });
+                } else {
+                    _super(code, reason);
+                }
+            },
+        });
+        env.services["bus_service"].addEventListener("connect", () => assert.step("connect"));
+        env.services["bus_service"].addEventListener("disconnect", () => assert.step("disconnect"));
+        env.services["bus_service"].addEventListener("reconnecting", () => assert.step("reconnecting"));
+        env.services["bus_service"].addEventListener("reconnect", () => assert.step("reconnect"));
+        // Connection will be closed when passing offline. But the close event
+        // will be delayed to come after the next open event. The connection
+        // will thus be in the closing state in the meantime.
+        window.dispatchEvent(new Event("offline"));
+        await nextTick();
+        openDeferred = makeDeferred();
+        // Worker reconnects upon the reception of the online event.
+        window.dispatchEvent(new Event("online"));
+        await openDeferred;
+        closeDeferred.resolve();
+        // Trigger the close event, it shouldn't have any effect since it is
+        // related to an old connection that is no longer in use.
+        await nextTick();
+        openDeferred = makeDeferred();
+        // Server closes the connection, the worker should reconnect.
+        pyEnv.simulateConnectionLost(WEBSOCKET_CLOSE_CODES.KEEP_ALIVE_TIMEOUT);
+        await openDeferred;
+        await nextTick();
+        // 3 connections were opened, so 3 subscriptions are expected.
+        assert.strictEqual(subscribeSent, 3);
+        assert.verifySteps([
+            "connect",
+            "disconnect",
+            "connect",
+            "disconnect",
+            "reconnecting",
+            "reconnect",
+        ]);
+    });
 });
 });
 

addons/bus/static/tests/helpers/mock_server.js

@@ -74,8 +74,6 @@ patch(MockServer.prototype, 'bus', {
      * @param {number} clodeCode the code to close the connection with.
      */
     _simulateConnectionLost(closeCode) {
-        this.websocketWorker.websocket.dispatchEvent(new CloseEvent('close', {
-            code: closeCode,
-        }));
+        this.websocketWorker.websocket.close(closeCode);
     },
 });