[IMP] web: patch/unpatch: better error handling

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

@@ -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);
 

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) {