Skip to content

feat(zone.js): add a zone config to allow user disable wrapping uncaught promise rejection#35873

Closed
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
JiaLiPassion:uncaught-promise-unwrap
Closed

feat(zone.js): add a zone config to allow user disable wrapping uncaught promise rejection#35873
JiaLiPassion wants to merge 1 commit intoangular:masterfrom
JiaLiPassion:uncaught-promise-unwrap

Conversation

@JiaLiPassion
Copy link
Copy Markdown
Contributor

Close #27840.

By default, zone.js wrap uncaught promise error and wrap it to a new Error object with some
additional information includes the value of the error and the stack trace.

Consider the following example:

Zone.current
  .fork({
    name: 'promise-error',
    onHandleError: (delegate: ZoneDelegate, current: Zone, target: Zone, error: any): boolean => {
      console.log('caught an error', error);
      delegate.handleError(target, error);
      return false;
    }
}).run(() => {
  const originalError = new Error('testError');
  Promise.reject(originalError);
});

The promise-error zone catches a wrapped Error object whose reject property equals
to the originalError, and the message will be Uncaught (in promise): testError....
You can disable this wrapping behavior by defining a global configuraiton
__zone_symbol__DISABLE_WRAPPING_UNCAUGHT_PROMISE_REJECTION = true; before importing zone.js.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: #28740

This PR was original submitted here #31443, and it was reverted as it is a breaking change, and in this new PR, I changed the logic and keep the original behavior and use a configuration to let user change the behavior.

@pullapprove pullapprove Bot requested a review from mhevery March 5, 2020 15:08
@JiaLiPassion JiaLiPassion added the area: zones Issues related to zone.js label Mar 5, 2020
@ngbot ngbot Bot added this to the needsTriage milestone Mar 5, 2020
@JiaLiPassion JiaLiPassion added the target: major This PR is targeted for the next major release label Mar 5, 2020
@JiaLiPassion JiaLiPassion force-pushed the uncaught-promise-unwrap branch 4 times, most recently from 343c9c5 to 170d334 Compare March 6, 2020 12:50
Comment thread integration/_payload-limits.json Outdated
Comment thread packages/zone.js/lib/common/promise.ts Outdated
Comment thread packages/zone.js/lib/common/promise.ts Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the benefit of throwing and than catching the error? If there is a reason, can you document it please so others are not confused like me.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the reason of the logic here is if the rejected value is not an Error object, we will build an Error object to add more readable message and stack trace.

Comment thread packages/zone.js/lib/zone.configurations.api.ts Outdated
Comment thread packages/zone.js/lib/zone.configurations.api.ts Outdated
Comment thread packages/zone.js/lib/zone.configurations.api.ts Outdated
@JiaLiPassion JiaLiPassion force-pushed the uncaught-promise-unwrap branch 3 times, most recently from eb0778a to c95b5c4 Compare March 9, 2020 09:59
@JiaLiPassion JiaLiPassion requested a review from mhevery March 9, 2020 10:23
…ght promise rejection

Close angular#27840.

By default, `zone.js` wrap uncaught promise error and wrap it to a new Error object with some
additional information includes the value of the error and the stack trace.

Consider the following example:

```
Zone.current
  .fork({
    name: 'promise-error',
    onHandleError: (delegate: ZoneDelegate, current: Zone, target: Zone, error: any): boolean => {
      console.log('caught an error', error);
      delegate.handleError(target, error);
      return false;
    }
}).run(() => {
  const originalError = new Error('testError');
  Promise.reject(originalError);
});
```

The `promise-error` zone catches a wrapped `Error` object whose `rejection` property equals
to the original error, and the message will be `Uncaught (in promise): testError....`,
You can disable this wrapping behavior by defining a global configuraiton
`__zone_symbol__DISABLE_WRAPPING_UNCAUGHT_PROMISE_REJECTION = true;` before importing `zone.js`.
@JiaLiPassion JiaLiPassion force-pushed the uncaught-promise-unwrap branch from c95b5c4 to 96f648c Compare March 9, 2020 11:04
@mhevery mhevery added the action: merge The PR is ready for merge by the caretaker label Mar 9, 2020
@mhevery
Copy link
Copy Markdown
Contributor

mhevery commented Mar 9, 2020

presubmit

@AndrewKushnir AndrewKushnir added the action: presubmit The PR is in need of a google3 presubmit label Mar 11, 2020
@AndrewKushnir AndrewKushnir removed the action: presubmit The PR is in need of a google3 presubmit label Mar 16, 2020
@AndrewKushnir
Copy link
Copy Markdown
Contributor

FYI, changes in this PR were tested against g3 as a part of #36043, using Global TAP run, which was successful. Thank you.

@angular-automatic-lock-bot
Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: zones Issues related to zone.js cla: yes target: major This PR is targeted for the next major release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

new behavior for global ErrorHandler and uncaught errors from promises

4 participants