fix(zone.js): only one listener should also re-throw an error correctly (#41868)

Close #41867

In the previous commit https://github.com/angular/angular/pull/41562#issuecomment-822696973,
the error thrown in the event listener will be caught and re-thrown, but there is a bug
in the commit, if there is only one listener for the specified event name, the error
will not be re-thrown, so this commit fixes the issue and make sure the error is re-thrown.

PR Close #41868

goldens/size-tracking/integration-payloads.json

@@ -13,7 +13,7 @@
       "uncompressed": {
         "runtime-es2015": 1205,
         "main-es2015": 17597,
-        "polyfills-es2015": 36709
+        "polyfills-es2015": 37127
       }
     }
   },

packages/zone.js/lib/common/events.ts

@@ -104,7 +104,7 @@ export function patchEventTarget(
   const PREPEND_EVENT_LISTENER = 'prependListener';
   const PREPEND_EVENT_LISTENER_SOURCE = '.' + PREPEND_EVENT_LISTENER + ':';
 
-  const invokeTask = function(task: any, target: any, event: Event) {
+  const invokeTask = function(task: any, target: any, event: Event): Error|undefined {
     // for better performance, check isRemoved which is set
     // by removeEventListener
     if (task.isRemoved) {
@@ -149,16 +149,17 @@ export function patchEventTarget(
     const target: any = context || event.target || _global;
     const tasks = target[zoneSymbolEventNames[event.type][isCapture ? TRUE_STR : FALSE_STR]];
     if (tasks) {
+      const errors = [];
       // invoke all tasks which attached to current target with given event.type and capture = false
       // for performance concern, if task.length === 1, just invoke
       if (tasks.length === 1) {
-        invokeTask(tasks[0], target, event);
+        const err = invokeTask(tasks[0], target, event);
+        err && errors.push(err);
       } else {
         // https://github.com/angular/zone.js/issues/836
         // copy the tasks array before invoke, to avoid
         // the callback will remove itself or other listener
         const copyTasks = tasks.slice();
-        const errors = [];
         for (let i = 0; i < copyTasks.length; i++) {
           if (event && (event as any)[IMMEDIATE_PROPAGATION_SYMBOL] === true) {
             break;
@@ -166,6 +167,12 @@ export function patchEventTarget(
           const err = invokeTask(copyTasks[i], target, event);
           err && errors.push(err);
         }
+      }
+      // Since there is only one error, we don't need to schedule microTask
+      // to throw the error.
+      if (errors.length === 1) {
+        throw errors[0];
+      } else {
         for (let i = 0; i < errors.length; i++) {
           const err = errors[i];
           api.nativeScheduleMicroTask(() => {

packages/zone.js/test/browser/XMLHttpRequest.spec.ts

@@ -132,9 +132,11 @@ describe('XMLHttpRequest', function() {
 
   it('should invoke xhr task even onload listener throw error', function(done) {
     const oriWindowError = window.onerror;
-    window.onerror = function() {};
-    try {
     const logs: string[] = [];
+    window.onerror = function(err: any) {
+      logs.push(err);
+    };
+    try {
       const xhrZone = Zone.current.fork({
         name: 'xhr',
         onInvokeTask: (delegate, curr, target, task, applyThis, applyArgs) => {
@@ -156,7 +158,7 @@ describe('XMLHttpRequest', function() {
           throw new Error('test');
         };
         const unhandledRejection = (e: PromiseRejectionEvent) => {
-          logs.push(e.reason.message);
+          fail('should not be here');
         };
         window.addEventListener('unhandledrejection', unhandledRejection);
         req.addEventListener('load', () => {
@@ -165,8 +167,7 @@ describe('XMLHttpRequest', function() {
             expect(logs).toEqual([
               'hasTask true', 'invokeTask XMLHttpRequest.addEventListener:load', 'onload',
               'invokeTask XMLHttpRequest.addEventListener:load', 'onload1',
-              'invokeTask XMLHttpRequest.send', 'hasTask false',
+              'invokeTask XMLHttpRequest.send', 'hasTask false', 'Uncaught Error: test'
-              'invokeTask Window.addEventListener:unhandledrejection', 'test'
             ]);
             window.removeEventListener('unhandledrejection', unhandledRejection);
             window.onerror = oriWindowError;

packages/zone.js/test/browser/browser.spec.ts

@@ -2491,6 +2491,85 @@ describe('Zone', function() {
            expect(logs).toEqual([]);
          }));
 
+      it('should re-throw the error when the only listener throw error', function(done: DoneFn) {
+        // override global.onerror to prevent jasmine report error
+        let oriWindowOnError = window.onerror;
+        let logs: string[] = [];
+        window.onerror = function(err: any) {
+          logs.push(err);
+        };
+        try {
+          const listener1 = function() {
+            throw new Error('test1');
+          };
+          button.addEventListener('click', listener1);
+
+          const mouseEvent = document.createEvent('MouseEvent');
+          mouseEvent.initEvent('click', true, true);
+
+          const unhandledRejection = (e: PromiseRejectionEvent) => {
+            fail('should not be here');
+          };
+          window.addEventListener('unhandledrejection', unhandledRejection);
+
+          button.dispatchEvent(mouseEvent);
+          expect(logs).toEqual(['Uncaught Error: test1']);
+
+          setTimeout(() => {
+            expect(logs).toEqual(['Uncaught Error: test1']);
+            window.removeEventListener('unhandledrejection', unhandledRejection);
+            window.onerror = oriWindowOnError;
+            done()
+          });
+        } catch (e: any) {
+          window.onerror = oriWindowOnError;
+        }
+      });
+
+      it('should not re-throw the error when zone onHandleError handled the error and the only listener throw error',
+         function(done: DoneFn) {
+           // override global.onerror to prevent jasmine report error
+           let oriWindowOnError = window.onerror;
+           window.onerror = function() {};
+           try {
+             let logs: string[] = [];
+             const listener1 = function() {
+               throw new Error('test1');
+             };
+             const zone = Zone.current.fork({
+               name: 'error',
+               onHandleError: (delegate, curr, target, error) => {
+                 logs.push('zone handled ' + target.name + ' ' + error.message);
+                 return false;
+               }
+             });
+
+             zone.runGuarded(() => {
+               button.addEventListener('click', listener1);
+             });
+
+             const mouseEvent = document.createEvent('MouseEvent');
+             mouseEvent.initEvent('click', true, true);
+
+             const unhandledRejection = (e: PromiseRejectionEvent) => {
+               logs.push(e.reason.message);
+             };
+             window.addEventListener('unhandledrejection', unhandledRejection);
+
+             button.dispatchEvent(mouseEvent);
+             expect(logs).toEqual(['zone handled error test1']);
+
+             setTimeout(() => {
+               expect(logs).toEqual(['zone handled error test1']);
+               window.removeEventListener('unhandledrejection', unhandledRejection);
+               window.onerror = oriWindowOnError;
+               done();
+             });
+           } catch (e: any) {
+             window.onerror = oriWindowOnError;
+           }
+         });
+
       it('should be able to continue to invoke remaining listeners even some listener throw error',
          function(done: DoneFn) {
            // override global.onerror to prevent jasmine report error
@@ -2540,13 +2619,77 @@ describe('Zone', function() {
            }
          });
 
-      it('should be able to continue to invoke remaining listeners even some listener throw error in the different zones',
+      it('should be able to continue to invoke remaining listeners even some listener throw error with onHandleError zone',
          function(done: DoneFn) {
            // override global.onerror to prevent jasmine report error
            let oriWindowOnError = window.onerror;
            window.onerror = function() {};
            try {
+             const zone = Zone.current.fork({
+               name: 'error',
+               onHandleError: (delegate, curr, target, error) => {
+                 logs.push('zone handled ' + target.name + ' ' + error.message);
+                 return false;
+               }
+             });
              let logs: string[] = [];
+             const listener1 = function() {
+               logs.push('listener1');
+             };
+             const listener2 = function() {
+               throw new Error('test1');
+             };
+             const listener3 = function() {
+               throw new Error('test2');
+             };
+             const listener4 = {
+               handleEvent: function() {
+                 logs.push('listener2');
+               }
+             };
+
+             zone.runGuarded(() => {
+               button.addEventListener('click', listener1);
+               button.addEventListener('click', listener2);
+               button.addEventListener('click', listener3);
+               button.addEventListener('click', listener4);
+             });
+
+             const mouseEvent = document.createEvent('MouseEvent');
+             mouseEvent.initEvent('click', true, true);
+
+             const unhandledRejection = (e: PromiseRejectionEvent) => {
+               fail('should not be here');
+             };
+             window.addEventListener('unhandledrejection', unhandledRejection);
+
+             button.dispatchEvent(mouseEvent);
+             expect(logs).toEqual([
+               'listener1', 'zone handled error test1', 'zone handled error test2', 'listener2'
+             ]);
+
+             setTimeout(() => {
+               expect(logs).toEqual([
+                 'listener1', 'zone handled error test1', 'zone handled error test2', 'listener2'
+               ]);
+               window.removeEventListener('unhandledrejection', unhandledRejection);
+               window.onerror = oriWindowOnError;
+               done();
+             });
+           } catch (e: any) {
+             window.onerror = oriWindowOnError;
+           }
+         });
+
+      it('should be able to continue to invoke remaining listeners even some listener throw error in the different zones',
+         function(done: DoneFn) {
+           // override global.onerror to prevent jasmine report error
+           let oriWindowOnError = window.onerror;
+           let logs: string[] = [];
+           window.onerror = function(err: any) {
+             logs.push(err);
+           };
+           try {
              const zone1 = Zone.current.fork({
                name: 'zone1',
                onHandleError: (delegate, curr, target, error) => {
@@ -2580,18 +2723,18 @@ describe('Zone', function() {
              mouseEvent.initEvent('click', true, true);
 
              const unhandledRejection = (e: PromiseRejectionEvent) => {
-               logs.push(e.reason.message);
+               fail('should not be here');
              };
              window.addEventListener('unhandledrejection', unhandledRejection);
 
              button.dispatchEvent(mouseEvent);
-             expect(logs).toEqual(['listener1', 'test1', 'listener2']);
+             expect(logs).toEqual(['listener1', 'test1', 'listener2', 'Uncaught Error: test2']);
 
              setTimeout(() => {
-               expect(logs).toEqual(['listener1', 'test1', 'listener2', 'test2']);
+               expect(logs).toEqual(['listener1', 'test1', 'listener2', 'Uncaught Error: test2']);
                window.removeEventListener('unhandledrejection', unhandledRejection);
                window.onerror = oriWindowOnError;
-               done()
+               done();
              });
            } catch (e: any) {
              window.onerror = oriWindowOnError;