From 2d6dc75a2cb5fa81189920d0f7fdc7b7f12132ab Mon Sep 17 00:00:00 2001 From: fja-odoo Date: Thu, 25 Jul 2019 08:20:59 +0000 Subject: [PATCH 1/3] [FIX] web, *: make crash_manager use services correctly * = website_forum, stock Fix for: https://github.com/odoo/odoo/pull/32132 crash_manager.js was using the services while bypassing ServiceProviderMixin That means all services are duplicated, each having their own state. To fix this issue, CrashManager now extends AbstractService. Also crash_manager is now registered in serviceRegistry instead of being instanciated on the fly. The crash_manager now being a Service raise one issue with the tests: The services are not run for the tests but some tests needs the crash_manager 'unhandledrejection' listener. For that matter the services are reenable in qunit. Also the crash_manager's 'unhandledrejection' listener has to have precedence over the qunit listener as it will handle rejections wich have no reason or are not Errors. Part of: https://github.com/odoo/odoo/pull/35209 --- .../js/stock_traceability_report_backend.js | 3 +- .../src/js/chrome/action_manager_report.js | 5 +- addons/web/static/src/js/core/misc.js | 4 +- .../web/static/src/js/fields/basic_fields.js | 10 ++-- .../static/src/js/services/crash_manager.js | 47 ++++++++++--------- .../src/js/views/pivot/pivot_controller.js | 5 +- .../web/static/src/js/widgets/data_export.js | 3 +- addons/web/views/webclient_templates.xml | 20 +------- .../static/src/js/website_forum.js | 12 +++-- 9 files changed, 46 insertions(+), 63 deletions(-) diff --git a/addons/stock/static/src/js/stock_traceability_report_backend.js b/addons/stock/static/src/js/stock_traceability_report_backend.js index 56673f4e094..31e42767cf5 100644 --- a/addons/stock/static/src/js/stock_traceability_report_backend.js +++ b/addons/stock/static/src/js/stock_traceability_report_backend.js @@ -6,7 +6,6 @@ var core = require('web.core'); var session = require('web.session'); var ReportWidget = require('stock.ReportWidget'); var framework = require('web.framework'); -var crash_manager = require('web.crash_manager'); var QWeb = core.qweb; @@ -104,7 +103,7 @@ var stock_report_generic = AbstractAction.extend({ url: url_data.replace('output_format', 'pdf'), data: {data: JSON.stringify(dict)}, complete: framework.unblockUI, - error: crash_manager.rpc_error.bind(crash_manager), + error: () => this.call('crash_manager', 'rpc_error', ...arguments), }); }); return this.$buttons; diff --git a/addons/web/static/src/js/chrome/action_manager_report.js b/addons/web/static/src/js/chrome/action_manager_report.js index 30bac14006d..416cad6a942 100644 --- a/addons/web/static/src/js/chrome/action_manager_report.js +++ b/addons/web/static/src/js/chrome/action_manager_report.js @@ -8,7 +8,6 @@ odoo.define('web.ReportActionManager', function (require) { var ActionManager = require('web.ActionManager'); var core = require('web.core'); -var crash_manager = require('web.crash_manager'); var framework = require('web.framework'); var session = require('web.session'); @@ -54,8 +53,8 @@ ActionManager.include({ data: JSON.stringify([url, type]), }, success: resolve, - error: function () { - crash_manager.rpc_error.apply(crash_manager, arguments); + error: () => { + this.call('crash_manager', 'rpc_error', ...arguments); reject(); }, complete: framework.unblockUI, diff --git a/addons/web/static/src/js/core/misc.js b/addons/web/static/src/js/core/misc.js index 39fd8021071..6260a4ab583 100644 --- a/addons/web/static/src/js/core/misc.js +++ b/addons/web/static/src/js/core/misc.js @@ -2,9 +2,9 @@ odoo.define('web.framework', function (require) { "use strict"; var core = require('web.core'); -var crash_manager = require('web.crash_manager'); var ajax = require('web.ajax'); var Widget = require('web.Widget'); +var disableCrashManager = require('web.CrashManager').disable; var _t = core._t; @@ -101,7 +101,7 @@ function unblockUI() { */ function redirect (url, wait) { // Dont display a dialog if some xmlhttprequest are in progress - crash_manager.disable(); + disableCrashManager(); var load = function() { var old = "" + window.location; diff --git a/addons/web/static/src/js/fields/basic_fields.js b/addons/web/static/src/js/fields/basic_fields.js index 4e63e5d3613..322168a7241 100644 --- a/addons/web/static/src/js/fields/basic_fields.js +++ b/addons/web/static/src/js/fields/basic_fields.js @@ -10,7 +10,6 @@ odoo.define('web.basic_fields', function (require) { var AbstractField = require('web.AbstractField'); var config = require('web.config'); var core = require('web.core'); -var crash_manager = require('web.crash_manager'); var datepicker = require('web.datepicker'); var dom = require('web.dom'); var Domain = require('web.Domain'); @@ -1928,11 +1927,10 @@ var FieldBinaryFile = AbstractFieldBinary.extend({ ev.stopPropagation(); } else if (this.res_id) { framework.blockUI(); - var c = crash_manager; var filename_fieldname = this.attrs.filename; this.getSession().get_file({ - 'url': '/web/content', - 'data': { + complete: framework.unblockUI, + data: { 'model': this.model, 'id': this.res_id, 'field': this.name, @@ -1941,8 +1939,8 @@ var FieldBinaryFile = AbstractFieldBinary.extend({ 'download': true, 'data': utils.is_bin_size(this.value) ? null : this.value, }, - 'complete': framework.unblockUI, - 'error': c.rpc_error.bind(c), + error: () => this.call('crash_manager', 'rpc_error', ...arguments), + url: '/web/content', }); ev.stopPropagation(); } diff --git a/addons/web/static/src/js/services/crash_manager.js b/addons/web/static/src/js/services/crash_manager.js index 701d97f0a53..4e4d4c9b0ff 100644 --- a/addons/web/static/src/js/services/crash_manager.js +++ b/addons/web/static/src/js/services/crash_manager.js @@ -1,12 +1,10 @@ odoo.define('web.CrashManager', function (require) { "use strict"; +const AbstractService = require('web.AbstractService'); var ajax = require('web.ajax'); var core = require('web.core'); var Dialog = require('web.Dialog'); -var mixins = require('web.mixins'); -var ServicesMixin = require('web.ServicesMixin'); -var ServiceProviderMixin = require('web.ServiceProviderMixin'); var Widget = require('web.Widget'); var _t = core._t; @@ -23,6 +21,14 @@ var map_title ={ karma_error: _lt("Karma Error"), }; +// Register this eventlistener before qunit does. +// Some errors needs to be negated by the crash_manager. +window.addEventListener('unhandledrejection', ev => + core.bus.trigger('crash_manager_unhandledrejection', ev) +); + +let active = true; + /** * An extension of Dialog Widget to render the warnings and errors on the website. * Extend it with your template of choice like ErrorDialog/WarningDialog @@ -78,13 +84,13 @@ var WarningDialog = CrashManagerDialog.extend({ }, }); -var CrashManager = core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, ServiceProviderMixin,{ +var CrashManager = AbstractService.extend({ init: function () { var self = this; - this.active = true; + active = true; this.isConnected = true; - mixins.EventDispatcherMixin.init.call(this); + this._super.apply(this, arguments); // crash manager integration core.bus.on('rpc_error', this, this.rpc_error); @@ -128,7 +134,7 @@ var CrashManager = core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, // listen to unhandled rejected promises, and throw an error when the // promise has been rejected due to a crash - window.addEventListener('unhandledrejection', function (ev) { + core.bus.on('crash_manager_unhandledrejection', this, function (ev) { if (ev.reason && ev.reason instanceof Error) { var traceback = ev.reason.stack; self.show_error({ @@ -144,13 +150,12 @@ var CrashManager = core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, ev.preventDefault(); } }); - ServiceProviderMixin.init.call(this); }, enable: function () { - this.active = true; + active = true; }, disable: function () { - this.active = false; + active = false; }, handleLostConnection: function () { var self = this; @@ -175,7 +180,12 @@ var CrashManager = core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, }, delay); }, rpc_error: function(error) { - if (!this.active) { + // Some qunit tests produces errors before the DOM is set. + // This produces an error loop as the modal/toast has no DOM to attach to. + if (!document.body) { + return; + } + if (!active) { return; } if (this.connection_lost) { @@ -229,7 +239,7 @@ var CrashManager = core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, } }, show_warning: function (error, options) { - if (!this.active) { + if (!active) { return; } var message = error.data ? error.data.message : error.message; @@ -243,7 +253,7 @@ var CrashManager = core.Class.extend(mixins.EventDispatcherMixin, ServicesMixin, }, options)); }, show_error: function (error) { - if (!this.active) { + if (!active) { return; } var dialog = new ErrorDialog(this, { @@ -371,17 +381,12 @@ core.crash_registry.add('504', function (cm) { }; }); +core.serviceRegistry.add('crash_manager', CrashManager); + return { CrashManager: CrashManager, ErrorDialog: ErrorDialog, WarningDialog: WarningDialog, + disable: () => active = false, }; }); - -odoo.define('web.crash_manager', function (require) { -"use strict"; - -var CrashManager = require('web.CrashManager').CrashManager; -return new CrashManager(); - -}); diff --git a/addons/web/static/src/js/views/pivot/pivot_controller.js b/addons/web/static/src/js/views/pivot/pivot_controller.js index 046c2979063..b5257a987c7 100644 --- a/addons/web/static/src/js/views/pivot/pivot_controller.js +++ b/addons/web/static/src/js/views/pivot/pivot_controller.js @@ -14,7 +14,6 @@ odoo.define('web.PivotController', function (require) { var AbstractController = require('web.AbstractController'); var core = require('web.core'); -var crash_manager = require('web.crash_manager'); var framework = require('web.framework'); var session = require('web.session'); @@ -126,7 +125,7 @@ var PivotController = AbstractController.extend({ _downloadTable: function () { var table = this.model.exportData(); if(table.measure_row.length + 1 > 256) { - crash_manager.show_message(_t("For Excel compatibility, data cannot be exported if there are more than 256 columns.\n\nTip: try to flip axis, filter further or reduce the number of measures.")); + this.call('crash_manager', 'show_message', _t("For Excel compatibility, data cannot be exported if there are more than 256 columns.\n\nTip: try to flip axis, filter further or reduce the number of measures.")); framework.unblockUI(); return; } @@ -136,7 +135,7 @@ var PivotController = AbstractController.extend({ url: '/web/pivot/export_xls', data: {data: JSON.stringify(table)}, complete: framework.unblockUI, - error: crash_manager.rpc_error.bind(crash_manager) + error: () => this.call('crash_manager', 'rpc_error', ...arguments), }); }, /** diff --git a/addons/web/static/src/js/widgets/data_export.js b/addons/web/static/src/js/widgets/data_export.js index 6903e7fea33..9031dd43232 100644 --- a/addons/web/static/src/js/widgets/data_export.js +++ b/addons/web/static/src/js/widgets/data_export.js @@ -3,7 +3,6 @@ odoo.define('web.DataExport', function (require) { var config = require('web.config'); var core = require('web.core'); -var crash_manager = require('web.crash_manager'); var Dialog = require('web.Dialog'); var data = require('web.data'); var framework = require('web.framework'); @@ -168,7 +167,7 @@ var DataExport = Dialog.extend({ }) }, complete: framework.unblockUI, - error: crash_manager.rpc_error.bind(crash_manager), + error: () => this.call('crash_manager', 'rpc_error', ...arguments), }); }, /** diff --git a/addons/web/views/webclient_templates.xml b/addons/web/views/webclient_templates.xml index c3fc7d15479..7a96a2c30d7 100644 --- a/addons/web/views/webclient_templates.xml +++ b/addons/web/views/webclient_templates.xml @@ -572,27 +572,9 @@ + @@ -386,6 +387,7 @@