From 98ede6e5537d2458ce76e04c6a59ba4d4adfbc5e Mon Sep 17 00:00:00 2001
From: Aaron Bohy <aab@odoo.com>
Date: Thu, 18 Mar 2021 10:21:45 +0000
Subject: [PATCH] [IMP] web: patch/unpatch: better error handling
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this commit, calling unpatch on an object that hasn't been
patched before crashed (before reaching the code that handles the
case by throwing an Error). This scenario is now properly handled.

This commit also defines specific Error classes for the patch and
unpatch faulty cases, so that they can be elegantly catched (in
tests for example).

closes odoo/odoo#68057

Signed-off-by: Géry Debongnie (ged) <ged@openerp.com>
---
 addons/web/static/src/js/core/utils.js      | 21 ++++++++++++++++++---
 addons/web/static/tests/core/patch_tests.js | 15 ++++++++++++---
 2 files changed, 30 insertions(+), 6 deletions(-)

diff --git a/addons/web/static/src/js/core/utils.js b/addons/web/static/src/js/core/utils.js
index d6165d0a841..1c2dbd35ff9 100644
--- a/addons/web/static/src/js/core/utils.js
+++ b/addons/web/static/src/js/core/utils.js
@@ -227,7 +227,22 @@ function _getExtractorFrom(criterion) {
     }
 }
 
+class AlreadyDefinedPatchError extends Error {
+    constructor() {
+        super(...arguments);
+        this.name = 'AlreadyDefinedPatchError';
+    }
+}
+class UnknownPatchError extends Error {
+    constructor() {
+        super(...arguments);
+        this.name = 'UnknownPatchError';
+    }
+}
+
 var utils = {
+    AlreadyDefinedPatchError,
+    UnknownPatchError,
 
     /**
      * Throws an error if the given condition is not true
@@ -627,7 +642,7 @@ var utils = {
         }
         const objDesc = patchMap.get(obj);
         if (objDesc.patches.some(p => p.name === patchName)) {
-            throw new Error(`Class ${obj.name} already has a patch ${patchName}`);
+            throw new AlreadyDefinedPatchError(`Patch ${patchName} is already defined`);
         }
         objDesc.patches.push({
             name: patchName,
@@ -901,8 +916,8 @@ var utils = {
      */
     unpatch: function (obj, patchName) {
         const objDesc = patchMap.get(obj);
-        if (!objDesc.patches.some(p => p.name === patchName)) {
-            throw new Error(`Class ${obj.name} does not have any patch ${patchName}`);
+        if (!objDesc || !objDesc.patches.some(p => p.name === patchName)) {
+            throw new UnknownPatchError(`Could not find patch ${patchName}`);
         }
         patchMap.delete(obj);
 
diff --git a/addons/web/static/tests/core/patch_tests.js b/addons/web/static/tests/core/patch_tests.js
index 4668bdf217c..ee6c579cbc5 100644
--- a/addons/web/static/tests/core/patch_tests.js
+++ b/addons/web/static/tests/core/patch_tests.js
@@ -1,7 +1,7 @@
 odoo.define("web.patch_tests", function (require) {
 "use strict";
 
-const { patch, unpatch } = require('web.utils');
+const { AlreadyDefinedPatchError, patch, unpatch, UnknownPatchError } = require('web.utils');
 
 function makeBaseClass(assert, assertInSetup) {
     class BaseClass {
@@ -122,7 +122,7 @@ QUnit.module('core', {}, function () {
             // keys should be unique
             assert.throws(() => {
                 patch(A.prototype, 'patch');
-            });
+            }, AlreadyDefinedPatchError);
         });
 
         QUnit.test('unpatch', async function (assert) {
@@ -278,6 +278,15 @@ QUnit.module('core', {}, function () {
             ]);
         });
 
+        QUnit.test('unpatch a class that has not been patched', async function (assert) {
+            assert.expect(1);
+
+            const A = class {};
+            assert.throws(() => {
+                unpatch(A.prototype, 'patch');
+            }, UnknownPatchError);
+        });
+
         QUnit.test('unpatch twice the same patch name', async function (assert) {
             assert.expect(1);
 
@@ -287,7 +296,7 @@ QUnit.module('core', {}, function () {
             unpatch(A.prototype, 'patch');
             assert.throws(() => {
                 unpatch(A.prototype, 'patch');
-            });
+            }, UnknownPatchError);
         });
 
         QUnit.test('unpatch and re-patch', async function (assert) {