fix(zone): fix #674, handle error.stack readonly case (#675)

JiaLiPassion · mhevery · commit 8322be881c84 · 2017-03-14T14:15:07.000-07:00
diff --git a/lib/zone.ts b/lib/zone.ts
@@ -1747,7 +1747,7 @@ const Zone: ZoneType = (function(global: any) {
     'long-stack-trace'
   ];
 
-  function attachZoneAndRemoveInternalZoneFrames(error: any) {
+  function attachZoneAndRemoveInternalZoneFrames(error: any, zoneAwareError: any) {
     // Save original stack trace
     error.originalStack = error.stack;
     // Process the stack trace and rewrite the frames.
@@ -1789,7 +1789,14 @@ const Zone: ZoneType = (function(global: any) {
           }
         }
       }
-      error.stack = error.zoneAwareStack = frames.join('\n');
+      const finalStack: string = frames.join('\n');
+      try {
+        error.stack = error.zoneAwareStack = finalStack;
+      } catch (nonWritableErr) {
+        // in some browser, the error.stack is readonly such as PhantomJS
+        // so we need to store the stack frames to zoneAwareError directly
+        zoneAwareError.stack = finalStack;
+      }
     }
   }
 
@@ -1820,7 +1827,7 @@ const Zone: ZoneType = (function(global: any) {
     this[__symbol__('error')] = error;
     // 1. attach zone information to stack frame
     // 2. remove zone internal stack frames
-    attachZoneAndRemoveInternalZoneFrames(error);
+    attachZoneAndRemoveInternalZoneFrames(error, this);
 
     // use defineProperties here instead of copy property value
     // because of issue #595 which will break angular2.
diff --git a/test/common/Error.spec.ts b/test/common/Error.spec.ts
@@ -405,4 +405,24 @@ describe('Error stack', () => {
                         }, null, () => null, null);
        task.invoke();
      }));
+
+  it('should be able to generate zone free stack even NativeError stack is readonly', function() {
+    const _global: any =
+        typeof window === 'object' && window || typeof self === 'object' && self || global;
+    const NativeError = _global['__zone_symbol__Error'];
+    const desc = Object.getOwnPropertyDescriptor(NativeError.prototype, 'stack');
+    if (desc) {
+      const originalSet: (value: any) => void = desc.set;
+      // make stack readonly
+      desc.set = null;
+
+      try {
+        const error = new Error('test error');
+        expect(error.stack).toBeTruthy();
+        assertStackDoesNotContainZoneFrames(error);
+      } finally {
+        desc.set = originalSet;
+      }
+    }
+  });
 });

Original file line number	Diff line number	Diff line change
`@@ -1747,7 +1747,7 @@ const Zone: ZoneType = (function(global: any) {`
`1747`	`1747`	`'long-stack-trace'`
`1748`	`1748`	`];`
`1749`	`1749`
`1750`		`- function attachZoneAndRemoveInternalZoneFrames(error: any) {`
	`1750`	`+ function attachZoneAndRemoveInternalZoneFrames(error: any, zoneAwareError: any) {`
`1751`	`1751`	`// Save original stack trace`
`1752`	`1752`	`error.originalStack = error.stack;`
`1753`	`1753`	`// Process the stack trace and rewrite the frames.`
`@@ -1789,7 +1789,14 @@ const Zone: ZoneType = (function(global: any) {`
`1789`	`1789`	`}`
`1790`	`1790`	`}`
`1791`	`1791`	`}`
`1792`		`- error.stack = error.zoneAwareStack = frames.join('\n');`
	`1792`	`+ const finalStack: string = frames.join('\n');`
	`1793`	`+ try {`
	`1794`	`+ error.stack = error.zoneAwareStack = finalStack;`
	`1795`	`+ } catch (nonWritableErr) {`
	`1796`	`+ // in some browser, the error.stack is readonly such as PhantomJS`
	`1797`	`+ // so we need to store the stack frames to zoneAwareError directly`
	`1798`	`+ zoneAwareError.stack = finalStack;`
	`1799`	`+ }`
`1793`	`1800`	`}`
`1794`	`1801`	`}`
`1795`	`1802`
`@@ -1820,7 +1827,7 @@ const Zone: ZoneType = (function(global: any) {`
`1820`	`1827`	`this[__symbol__('error')] = error;`
`1821`	`1828`	`// 1. attach zone information to stack frame`
`1822`	`1829`	`// 2. remove zone internal stack frames`
`1823`		`- attachZoneAndRemoveInternalZoneFrames(error);`
	`1830`	`+ attachZoneAndRemoveInternalZoneFrames(error, this);`
`1824`	`1831`
`1825`	`1832`	`// use defineProperties here instead of copy property value`
`1826`	`1833`	`// because of issue #595 which will break angular2.`