Skip to content

Commit

Permalink
fix angular#595, refactor ZoneAwareError property copy
Browse files Browse the repository at this point in the history
  • Loading branch information
JiaLiPassion committed Jan 14, 2017
1 parent e1d3240 commit 1f3cdb5
Show file tree
Hide file tree
Showing 2 changed files with 155 additions and 25 deletions.
111 changes: 86 additions & 25 deletions lib/zone.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1330,38 +1330,95 @@ const Zone: ZoneType = (function(global: any) {
let frameParserStrategy = null;
const stackRewrite = 'stackRewrite';

const assignAll = function(to, from) {
if (!to) {
return to;
const createProperty = function(props, key) {
if (props[key]) {
return;
}
const name = __symbol__(key);
props[key] = {
configurable: true,
enumerable: true,
get: function() {
if (!this[name]) {
if (this instanceof ZoneAwareError) {
this[name] = this[__symbol__('error')][key];
}
}
return this[name];
},
set: function(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];
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[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;

createProperty(props, 'originalStack');
createProperty(props, 'zoneAwareStack');
createMethodProperty(props, 'toString');
createMethodProperty(props, 'toSource');
return props;
};

const errorProperties = createErrorProperties();

const getErrorPropertiesForPrototype = function(prototype) {
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 +1435,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 +1472,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
69 changes: 69 additions & 0 deletions test/common/Error.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,61 @@
* 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 error message';
}
}

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 +122,20 @@ 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');
});

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 1f3cdb5

Please sign in to comment.