From 5ffdd92034a9d62bfacc27ae6d664ff01d3e699c Mon Sep 17 00:00:00 2001 From: Samuel Degueldre Date: Mon, 2 Aug 2021 06:56:58 +0000 Subject: [PATCH] [FIX] web: make polling restart correctly when server is killed Previously, when killing the server, the longpolling bus stopped but didn't start polling again after a few seconds, this was caused by the fact that ConnectionLostError wasn't treated like a legacy error and remapped to an object with a message, meaning it didn't trigger guardedCatch callbacks. Since ConnectionLostErrors should be handled much the same way as RPCError when interacting with legacy code this commit simply adds ConnectionLostError in the same places we already have RPCError during error handling of legacy errors, but delegates behaviour to the new lostConnectionHandler when appropriate. closes odoo/odoo#74530 Signed-off-by: Lucas Perais (lpe) --- addons/bus/static/tests/bus_tests.js | 48 +++++++++++++++++++ .../static/src/core/errors/error_handlers.js | 2 +- .../src/legacy/legacy_rpc_error_handler.js | 11 +++-- addons/web/static/src/legacy/utils.js | 3 +- 4 files changed, 59 insertions(+), 5 deletions(-) diff --git a/addons/bus/static/tests/bus_tests.js b/addons/bus/static/tests/bus_tests.js index 90bc31dcc61..58208f293c6 100644 --- a/addons/bus/static/tests/bus_tests.js +++ b/addons/bus/static/tests/bus_tests.js @@ -7,6 +7,10 @@ var AbstractStorageService = require('web.AbstractStorageService'); var RamStorage = require('web.RamStorage'); var testUtils = require('web.test_utils'); var Widget = require('web.Widget'); +const LegacyRegistry = require("web.Registry"); +const { ConnectionLostError } = require("@web/core/network/rpc_service"); +const { patchWithCleanup, nextTick } = require("@web/../tests/helpers/utils"); +const { createWebClient } = require('@web/../tests/webclient/helpers'); var LocalStorageServiceMock; @@ -81,6 +85,50 @@ QUnit.module('Bus', { parent.destroy(); }); + QUnit.test('longpolling restarts when connection is lost', async function (assert) { + assert.expect(4); + const legacyRegistry = new LegacyRegistry(); + legacyRegistry.add("bus_service", BusService); + legacyRegistry.add("local_storage", LocalStorageServiceMock); + + const oldSetTimeout = window.setTimeout; + patchWithCleanup( + window, + { + setTimeout: callback => oldSetTimeout(callback, 0) + }, + { pure: true }, + ) + + let busService; + let rpcCount = 0; + // Using createWebclient to get the compatibility layer between the old services and the new + await createWebClient({ + mockRPC(route) { + if (route === '/longpolling/poll') { + rpcCount++; + assert.step(`polling ${rpcCount}`); + if (rpcCount == 1) { + return Promise.reject(new ConnectionLostError()); + } + assert.equal(rpcCount, 2, "Should not be called after stopPolling"); + busService.stopPolling(); + return Promise.reject(new ConnectionLostError()); + } + }, + legacyParams: { serviceRegistry: legacyRegistry }, + }); + busService = owl.Component.env.services.bus_service; + busService.startPolling(); + // Give longpolling bus a tick to try to restart polling + await nextTick(); + + assert.verifySteps([ + "polling 1", + "polling 2", + ]); + }); + QUnit.test('provide notification ID of 0 by default', async function (assert) { // This test is important in order to ensure that we provide the correct // sentinel value 0 when we are not aware of the last notification ID diff --git a/addons/web/static/src/core/errors/error_handlers.js b/addons/web/static/src/core/errors/error_handlers.js index 1de0938a911..b761d8eff74 100644 --- a/addons/web/static/src/core/errors/error_handlers.js +++ b/addons/web/static/src/core/errors/error_handlers.js @@ -116,7 +116,7 @@ let connectionLostNotifRemove = null; * @param {Error} originalError * @returns {boolean} */ -function lostConnectionHandler(env, error, originalError) { +export function lostConnectionHandler(env, error, originalError) { if (!(error instanceof UncaughtPromiseError)) { return false; } diff --git a/addons/web/static/src/legacy/legacy_rpc_error_handler.js b/addons/web/static/src/legacy/legacy_rpc_error_handler.js index fa00c3f51fd..178d1ccc2e9 100644 --- a/addons/web/static/src/legacy/legacy_rpc_error_handler.js +++ b/addons/web/static/src/legacy/legacy_rpc_error_handler.js @@ -2,7 +2,8 @@ import { registry } from "@web/core/registry"; import { RPCErrorDialog } from "../core/errors/error_dialogs"; -import { RPCError } from "../core/network/rpc_service"; +import { ConnectionLostError, RPCError } from "../core/network/rpc_service"; +import { lostConnectionHandler } from "@web/core/errors/error_handlers"; const errorNotificationRegistry = registry.category("error_notifications"); const errorDialogRegistry = registry.category("error_dialogs"); @@ -28,8 +29,12 @@ function legacyRPCErrorHandler(env, error, originalError) { originalError && originalError.legacy && originalError.message && - originalError.message instanceof RPCError + (originalError.message instanceof RPCError || + originalError.message instanceof ConnectionLostError) ) { + if (originalError.message instanceof ConnectionLostError) { + return lostConnectionHandler(env, error, originalError.message); + } const event = originalError.event; originalError = originalError.message; error.unhandledRejectionEvent.preventDefault(); @@ -59,7 +64,7 @@ function legacyRPCErrorHandler(env, error, originalError) { data: originalError.data, subType: originalError.subType, code: originalError.code, - type: originalError.type + type: originalError.type, }); return true; } diff --git a/addons/web/static/src/legacy/utils.js b/addons/web/static/src/legacy/utils.js index a9ae9470e51..4cb9d7d3811 100644 --- a/addons/web/static/src/legacy/utils.js +++ b/addons/web/static/src/legacy/utils.js @@ -6,6 +6,7 @@ import { ConnectionAbortedError, RPCError, makeErrorFromResponse, + ConnectionLostError, } from "../core/network/rpc_service"; import { ErrorDialog } from "../core/errors/error_dialogs"; @@ -151,7 +152,7 @@ export function mapLegacyEnvToWowlEnv(legacyEnv, wowlEnv) { jsonrpc.abort(); }; jsonrpc.then(resolve).catch((reason) => { - if (reason instanceof RPCError) { + if (reason instanceof RPCError || reason instanceof ConnectionLostError) { // we do not reject an error here because we want to pass through // the legacy guardedCatch code reject({ message: reason, event: $.Event(), legacy: true });