Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

Commit 6767ff5

Browse files
committed
fix(long-stack): Safer writing of stack traces.
1 parent a85fd68 commit 6767ff5

File tree

5 files changed

+61
-21
lines changed

5 files changed

+61
-21
lines changed

Diff for: gulpfile.js

+4
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,10 @@ gulp.task('test/node', ['compile'], function(cb) {
160160
cb();
161161
}
162162
});
163+
jrunner.print = function(value) {
164+
process.stdout.write(value);
165+
}
166+
jrunner.addReporter(new JasmineRunner.ConsoleReporter(jrunner));
163167
jrunner.projectBaseDir = __dirname;
164168
jrunner.specDir = '';
165169
jrunner.addSpecFiles(specFiles);

Diff for: lib/browser/define-property.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ import {zoneSymbol} from "../common/utils";
44
* things like redefining `createdCallback` on an element.
55
*/
66

7-
const _defineProperty = Object.defineProperty;
8-
const _getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
7+
const _defineProperty = Object[zoneSymbol('defineProperty')] = Object.defineProperty;
8+
const _getOwnPropertyDescriptor = Object[zoneSymbol('getOwnPropertyDescriptor')] = Object.getOwnPropertyDescriptor;
99
const _create = Object.create;
1010
const unconfigurablesKey = zoneSymbol('unconfigurables');
1111

Diff for: lib/jasmine/jasmine.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@
4646
});
4747
['beforeEach', 'afterEach'].forEach((methodName) => {
4848
let originalJasmineFn: Function = jasmineEnv[methodName];
49-
jasmineEnv[methodName] = function(specDefinitions: Function) {
49+
jasmineEnv[methodName] = function(specDefinitions: Function, timeout: number) {
5050
arguments[0] = wrapTestInZone(specDefinitions);
5151
return originalJasmineFn.apply(this, arguments);
5252
}

Diff for: lib/zone-spec/long-stack-trace.ts

+27-14
Original file line numberDiff line numberDiff line change
@@ -87,20 +87,33 @@
8787
{
8888
const parentTask = Zone.currentTask || error.task;
8989
if (error instanceof Error && parentTask) {
90-
let descriptor = Object.getOwnPropertyDescriptor(error, 'stack');
91-
if (descriptor) {
92-
const delegateGet = descriptor.get;
93-
const value = descriptor.value;
94-
descriptor = {
95-
get: function() {
96-
return renderLongStackTrace(parentTask.data && parentTask.data[creationTrace],
97-
delegateGet ? delegateGet.apply(this): value);
98-
}
99-
};
100-
Object.defineProperty(error, 'stack', descriptor);
101-
} else {
102-
error.stack = renderLongStackTrace(parentTask.data && parentTask.data[creationTrace],
103-
error.stack);
90+
var stackSetSucceded: string|boolean = null;
91+
try {
92+
let descriptor = Object.getOwnPropertyDescriptor(error, 'stack');
93+
if (descriptor && descriptor.configurable) {
94+
const delegateGet = descriptor.get;
95+
const value = descriptor.value;
96+
descriptor = {
97+
get: function () {
98+
return renderLongStackTrace(parentTask.data && parentTask.data[creationTrace],
99+
delegateGet ? delegateGet.apply(this) : value);
100+
}
101+
};
102+
Object.defineProperty(error, 'stack', descriptor);
103+
stackSetSucceded = true;
104+
}
105+
} catch (e) { }
106+
var longStack: string = stackSetSucceded ? null : renderLongStackTrace(
107+
parentTask.data && parentTask.data[creationTrace], error.stack);
108+
if (!stackSetSucceded) {
109+
try {
110+
stackSetSucceded = error.stack = longStack;
111+
} catch (e) { }
112+
}
113+
if (!stackSetSucceded) {
114+
try {
115+
stackSetSucceded = (error as any).longStack = longStack;
116+
} catch (e) { }
104117
}
105118
}
106119
return parentZoneDelegate.handleError(targetZone, error);

Diff for: test/zone-spec/long-stack-trace-zone.spec.ts

+27-4
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
1+
import {zoneSymbol} from '../../lib/common/utils';
2+
var defineProperty = Object[zoneSymbol('defineProperty')] || Object.defineProperty;
3+
14
describe('longStackTraceZone', function () {
2-
let log;
5+
let log: Error[];
36
let lstz: Zone;
47

58
beforeEach(function () {
@@ -8,7 +11,7 @@ describe('longStackTraceZone', function () {
811
onHandleError: (parentZoneDelegate: ZoneDelegate, currentZone: Zone, targetZone: Zone,
912
error: any): boolean => {
1013
parentZoneDelegate.handleError(targetZone, error);
11-
log.push(error.stack);
14+
log.push(error);
1215
return false;
1316
}
1417
});
@@ -22,7 +25,7 @@ describe('longStackTraceZone', function () {
2225
setTimeout(function () {
2326
setTimeout(function () {
2427
try {
25-
expect(log[0].split('Elapsed: ').length).toBe(3);
28+
expect(log[0].stack.split('Elapsed: ').length).toBe(3);
2629
done();
2730
} catch (e) {
2831
expect(e).toBe(null);
@@ -34,6 +37,26 @@ describe('longStackTraceZone', function () {
3437
});
3538
});
3639

40+
it('should produce a long stack trace even if stack setter throws', (done) => {
41+
let wasStackAssigne = false;
42+
let error = new Error('Expected error');
43+
defineProperty(error, 'stack', {
44+
configurable: false,
45+
get: () => 'someStackTrace',
46+
set: (v) => {
47+
throw new Error('no writes');
48+
}
49+
});
50+
lstz.run(() => {
51+
setTimeout(() => { throw error; });
52+
});
53+
setTimeout(() => {
54+
var e = log[0];
55+
expect((e as any).longStack).toBeTruthy();
56+
done();
57+
});
58+
});
59+
3760
it('should produce long stack traces when reject in promise', function(done) {
3861
lstz.runGuarded(function () {
3962
setTimeout(function () {
@@ -48,7 +71,7 @@ describe('longStackTraceZone', function () {
4871
});
4972
setTimeout(function () {
5073
try {
51-
expect(log[0].split('Elapsed: ').length).toBe(5);
74+
expect(log[0].stack.split('Elapsed: ').length).toBe(5);
5275
done();
5376
} catch (e) {
5477
expect(e).toBe(null);

0 commit comments

Comments
 (0)