From 6767ff53cd0fac854c0a81d3e0bd8a0ac193be7b Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Fri, 2 Sep 2016 10:11:19 -0700 Subject: [PATCH] fix(long-stack): Safer writing of stack traces. --- gulpfile.js | 4 ++ lib/browser/define-property.ts | 4 +- lib/jasmine/jasmine.ts | 2 +- lib/zone-spec/long-stack-trace.ts | 41 +++++++++++++------- test/zone-spec/long-stack-trace-zone.spec.ts | 31 +++++++++++++-- 5 files changed, 61 insertions(+), 21 deletions(-) diff --git a/gulpfile.js b/gulpfile.js index 78fb0ce0c..2968c4271 100644 --- 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 index 004c5e0e9..2e82252d6 100644 --- 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 index ee166b53d..f5b5ad381 100644 --- 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 index 3c0a336ca..4048c9a5e 100644 --- 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 index 9297a4064..a4488535d 100644 --- 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);