fix(long-stack): Safer writing of stack traces.

juliemr · Sep 3, 2016 · 6767ff5 · 6767ff5
1 parent a85fd68
commit 6767ff5
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 21 deletions.
diff --git a/gulpfile.js b/gulpfile.js
@@ -160,6 +160,10 @@ gulp.task('test/node', ['compile'], function(cb) {
       cb();
     }
   });
+  jrunner.print = function(value) {
+    process.stdout.write(value);
+  }
+  jrunner.addReporter(new JasmineRunner.ConsoleReporter(jrunner));
   jrunner.projectBaseDir = __dirname;
   jrunner.specDir = '';
   jrunner.addSpecFiles(specFiles);

diff --git a/lib/browser/define-property.ts b/lib/browser/define-property.ts
@@ -4,8 +4,8 @@ import {zoneSymbol} from "../common/utils";
  * things like redefining `createdCallback` on an element.
  */
 
-const _defineProperty = Object.defineProperty;
-const _getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
+const _defineProperty = Object[zoneSymbol('defineProperty')] = Object.defineProperty;
+const _getOwnPropertyDescriptor = Object[zoneSymbol('getOwnPropertyDescriptor')] = Object.getOwnPropertyDescriptor;
 const _create = Object.create;
 const unconfigurablesKey = zoneSymbol('unconfigurables');
 

diff --git a/lib/jasmine/jasmine.ts b/lib/jasmine/jasmine.ts
@@ -46,7 +46,7 @@
   });
   ['beforeEach', 'afterEach'].forEach((methodName) => {
     let originalJasmineFn: Function = jasmineEnv[methodName];
-    jasmineEnv[methodName] = function(specDefinitions: Function) {
+    jasmineEnv[methodName] = function(specDefinitions: Function, timeout: number) {
       arguments[0] = wrapTestInZone(specDefinitions);
       return originalJasmineFn.apply(this, arguments);
     } 

diff --git a/lib/zone-spec/long-stack-trace.ts b/lib/zone-spec/long-stack-trace.ts
@@ -87,20 +87,33 @@
     {
       const parentTask = Zone.currentTask || error.task;
       if (error instanceof Error && parentTask) {
-        let descriptor = Object.getOwnPropertyDescriptor(error, 'stack');
-        if (descriptor) {
-          const delegateGet = descriptor.get;
-          const value = descriptor.value;
-          descriptor = {
-            get: function() {
-              return renderLongStackTrace(parentTask.data && parentTask.data[creationTrace],
-                  delegateGet ? delegateGet.apply(this): value);
-            }
-          };
-          Object.defineProperty(error, 'stack', descriptor);
-        } else {
-          error.stack = renderLongStackTrace(parentTask.data && parentTask.data[creationTrace],
-              error.stack);
+        var stackSetSucceded: string|boolean = null;
+        try {
+          let descriptor = Object.getOwnPropertyDescriptor(error, 'stack');
+          if (descriptor && descriptor.configurable) {
+            const delegateGet = descriptor.get;
+            const value = descriptor.value;
+            descriptor = {
+              get: function () {
+                return renderLongStackTrace(parentTask.data && parentTask.data[creationTrace],
+                    delegateGet ? delegateGet.apply(this) : value);
+              }
+            };
+            Object.defineProperty(error, 'stack', descriptor);
+            stackSetSucceded = true;
+          }
+        } catch (e) { }
+        var longStack: string = stackSetSucceded ? null : renderLongStackTrace(
+                parentTask.data && parentTask.data[creationTrace], error.stack);
+        if (!stackSetSucceded) {
+          try {
+            stackSetSucceded = error.stack = longStack;
+          } catch (e) { }
+        }
+        if (!stackSetSucceded) {
+          try {
+            stackSetSucceded = (error as any).longStack = longStack;
+          } catch (e) { }
         }
       }
       return parentZoneDelegate.handleError(targetZone, error);

diff --git a/test/zone-spec/long-stack-trace-zone.spec.ts b/test/zone-spec/long-stack-trace-zone.spec.ts
@@ -1,5 +1,8 @@
+import {zoneSymbol} from '../../lib/common/utils';
+var defineProperty = Object[zoneSymbol('defineProperty')] || Object.defineProperty;
+
 describe('longStackTraceZone', function () {
-  let log;
+  let log: Error[];
   let lstz: Zone;
 
   beforeEach(function () {
@@ -8,7 +11,7 @@ describe('longStackTraceZone', function () {
       onHandleError: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
                       error: any): boolean => {
         parentZoneDelegate.handleError(targetZone, error);
-        log.push(error.stack);
+        log.push(error);
         return false;
       }
     });
@@ -22,7 +25,7 @@ describe('longStackTraceZone', function () {
         setTimeout(function () {
           setTimeout(function () {
             try {
-              expect(log[0].split('Elapsed: ').length).toBe(3);
+              expect(log[0].stack.split('Elapsed: ').length).toBe(3);
               done();
             } catch (e) {
               expect(e).toBe(null);
@@ -34,6 +37,26 @@ describe('longStackTraceZone', function () {
     });
   });
 
+  it('should produce a long stack trace even if stack setter throws', (done) => {
+    let wasStackAssigne = false;
+    let error = new Error('Expected error');
+    defineProperty(error, 'stack', {
+      configurable: false,
+      get: () => 'someStackTrace',
+      set: (v) => {
+        throw new Error('no writes');
+      }
+    });
+    lstz.run(() => {
+      setTimeout(() => { throw error; });
+    });
+    setTimeout(() => {
+      var e = log[0];
+      expect((e as any).longStack).toBeTruthy();
+      done();
+    });
+  });
+
   it('should produce long stack traces when reject in promise', function(done) {
     lstz.runGuarded(function () {
       setTimeout(function () {
@@ -48,7 +71,7 @@ describe('longStackTraceZone', function () {
           });
           setTimeout(function () {
             try {
-              expect(log[0].split('Elapsed: ').length).toBe(5);
+              expect(log[0].stack.split('Elapsed: ').length).toBe(5);
               done();
             } catch (e) {
               expect(e).toBe(null);