Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

zone.js Promise.finally behavior different from native #43206

Open
diyews opened this issue Aug 19, 2021 · 1 comment
Open

zone.js Promise.finally behavior different from native #43206

diyews opened this issue Aug 19, 2021 · 1 comment
Assignees
@JiaLiPassion
Labels
area: zones P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Milestone
Backlog

Comments

@diyews
Copy link
Contributor

diyews commented Aug 19, 2021 •
edited

Which @angular/* package(s) are the source of the bug?

Don't known / other

Is this a regression?

No

Description

image

image

Sample code 
  new Promise(res => {
     throw 'error';
     res(null);
  }).finally(() => console.log('finally'));

Minimal reproduction

zone.js: https://stackblitz.com/edit/zones-js-zdlcnc?file=index.ts
native: https://stackblitz.com/edit/js-iauprh

What happened in zone.js

zone.js .finally() scheduleResolveOrReject then clearRejectedNoCatch, which will remove the uncaught error

Please provide a link to a minimal reproduction of the bug

https://stackblitz.com/edit/zones-js-zdlcnc?file=index.ts

Please provide the exception or error you saw

zone.js `.finally()` not throw the uncaught error while uncaught, but native promise will.

Please provide the environment you discovered this bug in

zone.js    v0.11.4

Anything else?

No response
@ngbot ngbot bot added this to the needsTriage milestone Aug 19, 2021
arturovt added a commit to arturovt/angular that referenced this issue Mar 26, 2022
@arturovt
fix(zone.js): do not silence uncaught promise rejections when the `fi…
53bff0b 
…nally` is called

The native promise implementation logs unhandled promise rejections after `onFinally`
has been called. We may rely on the `scheduledByFinally` argument, which is `false` by
default, to avoid creating a breaking change; this will also allow us to reduce the number of
changes. The `finally` function should not silence unhandled promise rejections, because
they're not silenced in any implementation. If the `scheduleResolveOrReject` is called within
the `finally` we won't clear unhandled rejections. We'll keep the same behaviour if the
`scheduleResolveOrReject` is called within the `then`.

PR Close angular#43206
@arturovt arturovt mentioned this issue Mar 26, 2022
Closed
5 tasks
arturovt added a commit to arturovt/angular that referenced this issue Jun 18, 2022
@arturovt
fix(zone.js): do not silence uncaught promise rejections when the `fi…
bfeb553 
…nally` is called

The native promise implementation logs unhandled promise rejections after `onFinally`
has been called. We may rely on the `scheduledByFinally` argument, which is `false` by
default, to avoid creating a breaking change; this will also allow us to reduce the number of
changes. The `finally` function should not silence unhandled promise rejections, because
they're not silenced in any implementation. If the `scheduleResolveOrReject` is called within
the `finally` we won't clear unhandled rejections. We'll keep the same behaviour if the
`scheduleResolveOrReject` is called within the `then`.

PR Close angular#43206
arturovt added a commit to arturovt/angular that referenced this issue Jun 19, 2022
@arturovt
fix(zone.js): do not silence uncaught promise rejections when the `fi…
e0baa6c 
…nally` is called

The native promise implementation logs unhandled promise rejections after `onFinally`
has been called. We may rely on the `scheduledByFinally` argument, which is `false` by
default, to avoid creating a breaking change; this will also allow us to reduce the number of
changes. The `finally` function should not silence unhandled promise rejections, because
they're not silenced in any implementation. If the `scheduleResolveOrReject` is called within
the `finally` we won't clear unhandled rejections. We'll keep the same behaviour if the
`scheduleResolveOrReject` is called within the `then`.

PR Close angular#43206
arturovt added a commit to arturovt/angular that referenced this issue Jun 19, 2022
@arturovt
fix(zone.js): do not silence uncaught promise rejections when the `fi…
411e235 
…nally` is called

The native promise implementation logs unhandled promise rejections after `onFinally`
has been called. We may rely on the `scheduledByFinally` argument, which is `false` by
default, to avoid creating a breaking change; this will also allow us to reduce the number of
changes. The `finally` function should not silence unhandled promise rejections, because
they're not silenced in any implementation. If the `scheduleResolveOrReject` is called within
the `finally` we won't clear unhandled rejections. We'll keep the same behaviour if the
`scheduleResolveOrReject` is called within the `then`.

PR Close angular#43206
arturovt added a commit to arturovt/angular that referenced this issue Jun 19, 2022
@arturovt
fix(zone.js): do not silence uncaught promise rejections when the `fi…
5bd2487 
…nally` is called

The native promise implementation logs unhandled promise rejections after `onFinally`
has been called. We may rely on the `scheduledByFinally` argument, which is `false` by
default, to avoid creating a breaking change; this will also allow us to reduce the number of
changes. The `finally` function should not silence unhandled promise rejections, because
they're not silenced in any implementation. If the `scheduleResolveOrReject` is called within
the `finally` we won't clear unhandled rejections. We'll keep the same behaviour if the
`scheduleResolveOrReject` is called within the `then`.

PR Close angular#43206
@AndrewKushnir
Copy link
Contributor

Re-opening this issue, since the fix was reverted.

@AndrewKushnir AndrewKushnir reopened this Jun 22, 2022
@AndrewKushnir AndrewKushnir added P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent P4 A relatively minor issue that is not relevant to core functions labels Jun 22, 2022
@ngbot ngbot bot modified the milestones: needsTriage, Backlog Jun 22, 2022
@AndrewKushnir AndrewKushnir removed the P4 A relatively minor issue that is not relevant to core functions label Jun 22, 2022
arturovt added a commit to arturovt/angular that referenced this issue Jun 22, 2022
@arturovt
fix(zone.js): do not silence uncaught promise rejections when the `fi…
a90573a 
…nally` is called

The native promise implementation logs unhandled promise rejections after `onFinally`
has been called. We may rely on the `scheduledByFinally` argument, which is `false` by
default, to avoid creating a breaking change; this will also allow us to reduce the number of
changes. The `finally` function should not silence unhandled promise rejections, because
they're not silenced in any implementation. If the `scheduleResolveOrReject` is called within
the `finally` we won't clear unhandled rejections. We'll keep the same behaviour if the
`scheduleResolveOrReject` is called within the `then`.

PR Close angular#43206
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@JiaLiPassion JiaLiPassion

Labels
area: zones P3 An issue that is relevant to core functions, but does not impede progress. Important, but not urgent
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@petebacondarwin @JiaLiPassion @diyews @AndrewKushnir