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

Conversation

JiaLiPassion
Copy link
Collaborator

@JiaLiPassion JiaLiPassion commented Jan 13, 2017

fix #595,#427.
In previous PR to fix #546, we copy every property from NativeError to this (ZoneAwareError) with following logic.

this[key] = error[key]

and such logic will have conflict with following angular2 BaseError logic.

  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();
      }
}

BaseError has such hack logic to make sure this is used.
because BaseError's constructor will call super , and super will call copy, copy will then call
BaseError's setter, but this._nativeError is still undefined, so it will cause Exception.

In this PR, instead of copy property values with assignAll, I create propertyDescriptor to delegate to native error.

Object.defineProperty(this, key, {
    configurable: true, enumerable: true,
    get, set
})

And because ZoneAwareError already handle Error this constructor issue, maybe in the future, angular2 BaseError don't need to handle such issue again.
angular/angular#13908

@JiaLiPassion
Copy link
Collaborator Author

JiaLiPassion commented Jan 13, 2017

@mhevery, sorry for breaking changes again, I'll run test on angular check whether the fix is ok.
Please review.

With this PR #597, and PR #594, angular test and angular-cli generated projects test has passed.

@JiaLiPassion JiaLiPassion force-pushed the issue-595 branch 4 times, most recently from 1f3cdb5 to 3b05a99 Compare January 14, 2017 07:35
@JiaLiPassion JiaLiPassion changed the title fix #595, refactor ZoneAwareError property copy logic fix #595, #427, #602, refactor ZoneAwareError property copy logic Jan 16, 2017
@cocaux
Copy link

cocaux commented Jan 16, 2017

Hi guys,
When will this be released?
Thanks
Coco

@JiaLiPassion JiaLiPassion force-pushed the issue-595 branch 2 times, most recently from d5d4f67 to 5c3362d Compare January 16, 2017 23:37
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

zone.js 0.7.5 prevents proper angular message to be displayed v0.7+ - custom Error classes do not work as expected
4 participants