fix(showError): fix ignoreConsoleErrorUncaughtError may change during drain microtask (#763)

JiaLiPassion · mhevery · commit 4baeb5c1758b · 2017-05-02T11:09:47.000-07:00
* fix(showError): fix ignoreConsoleErrorUncaughtError may change during drainMicroTask

* refactor(private): change showError from parameter to function
diff --git a/lib/common/promise.ts b/lib/common/promise.ts
@@ -18,8 +18,8 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
   const symbolPromise = __symbol__('Promise');
   const symbolThen = __symbol__('then');
 
-  api.onUnhandledError = (showError: boolean, e: any) => {
-    if (showError) {
+  api.onUnhandledError = (e: any) => {
+    if (api.showUncaughtError()) {
       const rejection = e && e.rejection;
       if (rejection) {
         console.error(
@@ -32,7 +32,7 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
     }
   };
 
-  api.microtaskDrainDone = (showError: boolean) => {
+  api.microtaskDrainDone = () => {
     while (_uncaughtPromiseErrors.length) {
       while (_uncaughtPromiseErrors.length) {
         const uncaughtPromiseError: UncaughtPromiseError = _uncaughtPromiseErrors.shift();
@@ -41,14 +41,14 @@ Zone.__load_patch('ZoneAwarePromise', (global: any, Zone: ZoneType, api: _ZonePr
             throw uncaughtPromiseError;
           });
         } catch (error) {
-          handleUnhandledRejection(showError, error);
+          handleUnhandledRejection(error);
         }
       }
     }
   };
 
-  function handleUnhandledRejection(ignoreError: boolean, e: any) {
-    api.onUnhandledError(ignoreError, e);
+  function handleUnhandledRejection(e: any) {
+    api.onUnhandledError(e);
     try {
       const handler = (Zone as any)[__symbol__('unhandledPromiseRejectionHandler')];
       if (handler && typeof handler === 'function') {
diff --git a/lib/zone.ts b/lib/zone.ts
@@ -317,8 +317,9 @@ interface _ZonePrivate {
   currentZoneFrame(): _ZoneFrame;
   symbol(name: string): string;
   scheduleMicroTask(task?: MicroTask): void;
-  onUnhandledError: (showError: boolean, error: Error) => void;
-  microtaskDrainDone: (showError: boolean) => void;
+  onUnhandledError: (error: Error) => void;
+  microtaskDrainDone: () => void;
+  showUncaughtError: () => boolean;
 }
 
 /** @internal */
@@ -1235,7 +1236,6 @@ const Zone: ZoneType = (function(global: any) {
   }
 
   function drainMicroTaskQueue() {
-    const showError: boolean = !(Zone as any)[__symbol__('ignoreConsoleErrorUncaughtError')];
     if (!_isDrainingMicrotaskQueue) {
       _isDrainingMicrotaskQueue = true;
       while (_microTaskQueue.length) {
@@ -1246,14 +1246,12 @@ const Zone: ZoneType = (function(global: any) {
           try {
             task.zone.runTask(task, null, null);
           } catch (error) {
-            if ((Zone as any)[__symbol__('ignoreConsoleErrorUncaughtError')]) {
-              return;
-            }
-            _api.onUnhandledError(showError, error);
+            _api.onUnhandledError(error);
           }
         }
       }
-      _api.microtaskDrainDone(showError);
+      const showError: boolean = !(Zone as any)[__symbol__('ignoreConsoleErrorUncaughtError')];
+      _api.microtaskDrainDone();
       _isDrainingMicrotaskQueue = false;
     }
   }
@@ -1278,7 +1276,8 @@ const Zone: ZoneType = (function(global: any) {
     currentZoneFrame: () => _currentZoneFrame,
     onUnhandledError: noop,
     microtaskDrainDone: noop,
-    scheduleMicroTask: scheduleMicroTask
+    scheduleMicroTask: scheduleMicroTask,
+    showUncaughtError: () => !(Zone as any)[__symbol__('ignoreConsoleErrorUncaughtError')]
   };
   let _currentZoneFrame: _ZoneFrame = {parent: null, zone: new Zone(null, null)};
   let _currentTask: Task = null;
diff --git a/test/common/Promise.spec.ts b/test/common/Promise.spec.ts
@@ -244,7 +244,42 @@ describe(
             });
           });
 
-          it('should notify Zone.onError if no one catches promise', (done) => {
+          it('should output error to console if ignoreConsoleErrorUncaughtError is false',
+             (done) => {
+               Zone.current.fork({name: 'promise-error'}).run(() => {
+                 (Zone as any)[Zone.__symbol__('ignoreConsoleErrorUncaughtError')] = false;
+                 const originalConsoleError = console.error;
+                 console.error = jasmine.createSpy('consoleErr');
+                 const p = new Promise((resolve, reject) => {
+                   throw new Error('promise error');
+                 });
+                 setTimeout(() => {
+                   expect(console.error).toHaveBeenCalled();
+                   console.error = originalConsoleError;
+                   done();
+                 }, 10);
+               });
+             });
+
+          it('should not output error to console if ignoreConsoleErrorUncaughtError is true',
+             (done) => {
+               Zone.current.fork({name: 'promise-error'}).run(() => {
+                 (Zone as any)[Zone.__symbol__('ignoreConsoleErrorUncaughtError')] = true;
+                 const originalConsoleError = console.error;
+                 console.error = jasmine.createSpy('consoleErr');
+                 const p = new Promise((resolve, reject) => {
+                   throw new Error('promise error');
+                 });
+                 setTimeout(() => {
+                   expect(console.error).not.toHaveBeenCalled();
+                   console.error = originalConsoleError;
+                   (Zone as any)[Zone.__symbol__('ignoreConsoleErrorUncaughtError')] = false;
+                   done();
+                 }, 10);
+               });
+             });
+
+          it('should notify Zone.onHandleError if no one catches promise', (done) => {
             let promiseError: Error = null;
             let zone: Zone = null;
             let task: Task = null;
