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

Commit

Permalink
fix: #595, refactor ZoneAwareError property copy (#597)
Browse files Browse the repository at this point in the history
  • Loading branch information
@JiaLiPassion @mhevery
JiaLiPassion authored and mhevery committed Jan 17, 2017
1 parent 83dfa97 commit f7330de
Show file tree
Hide file tree
Showing 2 changed files with 199 additions and 25 deletions.
134 changes: 109 additions & 25 deletions lib/zone.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1330,38 +1330,118 @@ const Zone: ZoneType = (function(global: any) {
let frameParserStrategy = null;
const stackRewrite = 'stackRewrite';

const assignAll = function(to, from) {
if (!to) {
return to;
// fix #595, create property descriptor
// for error properties
const createProperty = function(props, key) {
// if property is already defined, skip it.
if (props[key]) {
return;
}
// define a local property
// in case error property is not settable
const name = __symbol__(key);
props[key] = {
configurable: true,
enumerable: true,
get: function() {
// if local property has no value
// use internal error's property value
if (!this[name]) {
const error = this[__symbol__('error')];
if (error) {
this[name] = error[key];
}
}
return this[name];
},
set: function(value) {
// setter will set value to local property value
this[name] = value;
}
};
};

if (from) {
let keys = Object.getOwnPropertyNames(from);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
// Avoid bugs when hasOwnProperty is shadowed
if (Object.prototype.hasOwnProperty.call(from, key)) {
to[key] = from[key];
// fix #595, create property descriptor
// for error method properties
const createMethodProperty = function(props, key) {
if (props[key]) {
return;
}
props[key] = {
configurable: true,
enumerable: true,
writable: true,
value: function() {
const error = this[__symbol__('error')];
let errorMethod = (error && error[key]) || this[key];
if (errorMethod) {
return errorMethod.apply(error, arguments);
}
}
};
};

// copy all properties from prototype
// in Error, property such as name/message is in Error's prototype
// but not enumerable, so we copy those properties through
// Error's prototype
const proto = Object.getPrototypeOf(from);
if (proto) {
let pKeys = Object.getOwnPropertyNames(proto);
for (let i = 0; i < pKeys.length; i++) {
const key = pKeys[i];
// skip constructor
if (key !== 'constructor') {
to[key] = from[key];
}
const createErrorProperties = function() {
const props = Object.create(null);

const error = new NativeError();
let keys = Object.getOwnPropertyNames(error);
for (let i = 0; i < keys.length; i++) {
const key = keys[i];
// Avoid bugs when hasOwnProperty is shadowed
if (Object.prototype.hasOwnProperty.call(error, key)) {
createProperty(props, key);
}
}

const proto = NativeError.prototype;
if (proto) {
let pKeys = Object.getOwnPropertyNames(proto);
for (let i = 0; i < pKeys.length; i++) {
const key = pKeys[i];
// skip constructor
if (key !== 'constructor' && key !== 'toString' && key !== 'toSource') {
createProperty(props, key);
}
}
}
return to;

// some other properties are not
// in NativeError
createProperty(props, 'originalStack');
createProperty(props, 'zoneAwareStack');

// define toString, toSource as method property
createMethodProperty(props, 'toString');
createMethodProperty(props, 'toSource');
return props;
};

const errorProperties = createErrorProperties();

// for derived Error class which extends ZoneAwareError
// we should not override the derived class's property
// so we create a new props object only copy the properties
// from errorProperties which not exist in derived Error's prototype
const getErrorPropertiesForPrototype = function(prototype) {
// if the prototype is ZoneAwareError.prototype
// we just return the prebuilt errorProperties.
if (prototype === ZoneAwareError.prototype) {
return errorProperties;
}
const newProps = Object.create(null);
const cKeys = Object.getOwnPropertyNames(errorProperties);
const keys = Object.getOwnPropertyNames(prototype);
cKeys.forEach(cKey => {
if (keys.filter(key => {
return key === cKey;
})
.length === 0) {
newProps[cKey] = errorProperties[cKey];
}
});

return newProps;
};

/**
Expand All @@ -1378,6 +1458,7 @@ const Zone: ZoneType = (function(global: any) {
}
// Create an Error.
let error: Error = NativeError.apply(this, arguments);
this[__symbol__('error')] = error;

// Save original stack trace
error.originalStack = error.stack;
Expand Down Expand Up @@ -1414,7 +1495,10 @@ const Zone: ZoneType = (function(global: any) {
}
error.stack = error.zoneAwareStack = frames.join('\n');
}
return assignAll(this, error);
// use defineProperties here instead of copy property value
// because of issue #595 which will break angular2.
Object.defineProperties(this, getErrorPropertiesForPrototype(Object.getPrototypeOf(this)));
return this;
}

// Copy the prototype so that instanceof operator works as expected
Expand Down
90 changes: 90 additions & 0 deletions test/common/Error.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,75 @@
* found in the LICENSE file at https://angular.io/license
*/

// simulate @angular/facade/src/error.ts
class BaseError extends Error {
/** @internal **/
_nativeError: Error;

constructor(message: string) {
super(message);
const nativeError = new Error(message) as any as Error;
this._nativeError = nativeError;
}

get message() {
return this._nativeError.message;
}
set message(message) {
this._nativeError.message = message;
}
get name() {
return this._nativeError.name;
}
get stack() {
return (this._nativeError as any).stack;
}
set stack(value) {
(this._nativeError as any).stack = value;
}
toString() {
return this._nativeError.toString();
}
}

class WrappedError extends BaseError {
originalError: any;

constructor(message: string, error: any) {
super(`${message} caused by: ${error instanceof Error ? error.message : error}`);
this.originalError = error;
}

get stack() {
return ((this.originalError instanceof Error ? this.originalError : this._nativeError) as any)
.stack;
}
}

class TestError extends WrappedError {
constructor(message: string, error: any) {
super(`${message} caused by: ${error instanceof Error ? error.message : error}`, error);
}

get message() {
return 'test ' + this.originalError.message;
}
}

class TestMessageError extends WrappedError {
constructor(message: string, error: any) {
super(`${message} caused by: ${error instanceof Error ? error.message : error}`, error);
}

get message() {
return 'test ' + this.originalError.message;
}

set message(value) {
this.originalError.message = value;
}
}

describe('ZoneAwareError', () => {
// If the environment does not supports stack rewrites, then these tests will fail
// and there is no point in running them.
Expand Down Expand Up @@ -67,6 +136,27 @@ describe('ZoneAwareError', () => {
}
});

it('should not use child Error class get/set in ZoneAwareError constructor', () => {
const func = () => {
const error = new BaseError('test');
expect(error.message).toEqual('test');
};

expect(func).not.toThrow();
});

it('should behave correctly with wrapped error', () => {
const error = new TestError('originalMessage', new Error('error message'));
expect(error.message).toEqual('test error message');
error.originalError.message = 'new error message';
expect(error.message).toEqual('test new error message');

const error1 = new TestMessageError('originalMessage', new Error('error message'));
expect(error1.message).toEqual('test error message');
error1.message = 'new error message';
expect(error1.message).toEqual('test new error message');
});

it('should show zone names in stack frames and remove extra frames', () => {
const rootZone = getRootZone();
const innerZone = rootZone.fork({name: 'InnerZone'});
Expand Down

0 comments on commit f7330de

Please sign in to comment.