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.

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

Unhandled promise rejections are handled different in zone and zone/node #49930

Closed
alan-agius4 opened this issue Apr 19, 2023 · 1 comment
Closed
Assignees
Labels
area: zones regression Indicates than the issue relates to something that worked in a previous version type: bug/fix
Milestone

Comments

@alan-agius4
Copy link
Contributor

alan-agius4 commented Apr 19, 2023

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

zone.js

Is this a regression?

No

Description

Unhandled promise rejection are not being intercepted correctly on Node.js when using the zone.js/node entrypoint, however when using the browser version of zone.js (On Node) unhandled promise rejections are being intercepted correctly.

Please provide the exception or error you saw

import 'zone.js/fesm2015/zone-node.js';
Promise.reject('Promise thrown error.');

Output

$ node index.mjs     
node:internal/process/promises:279
            triggerUncaughtException(err, true /* fromPromise */);
            ^

[UnhandledPromiseRejection: This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). The promise rejected with the reason "Promise thrown error.".] {
  code: 'ERR_UNHANDLED_REJECTION'
}
import 'zone.js/fesm2015/zone.js';
Promise.reject('Promise thrown error.');

Output

$ node index.mjs
Unhandled Promise rejection: Promise thrown error. ; Zone: <root> ; Task: null ; Value: Promise thrown error. undefined

```true
Angular CLI: 16.0.0-rc.0
Node: 16.20.0
Package Manager: yarn 1.22.19
OS: linux x64

Angular: 16.0.0-rc.1
... animations, common, compiler, compiler-cli, core, forms
... platform-browser, platform-browser-dynamic, platform-server
... router

Package                         Version
---------------------------------------------------------
@angular-devkit/architect       0.1600.0-rc.0
@angular-devkit/build-angular   16.0.0-rc.0
@angular-devkit/core            16.0.0-rc.0
@angular-devkit/schematics      16.0.0-rc.0
@angular/cli                    16.0.0-rc.0
@nguniversal/builders           14.2.3
@nguniversal/express-engine     14.2.3
@schematics/angular             16.0.0-rc.0
rxjs                            7.5.7
typescript                      5.0.4

Anything else?

  • I expect that when using zone.js/node unhandled promises rejections are intercepted.
  • This problem cause the Angular error handler not the be called on Node.js when there is an unhandled promise rejection.
  • While checking this issue I also noticed that queueMicrotask is only patched on the browser version.
@ngbot ngbot bot added this to the needsTriage milestone Apr 19, 2023
@alan-agius4 alan-agius4 changed the title [Zone] - Unhandled promise rejections are handled different in zone and zone/node [Zone.js] - Unhandled promise rejections are handled different in zone and zone/node Apr 19, 2023
@alan-agius4 alan-agius4 changed the title [Zone.js] - Unhandled promise rejections are handled different in zone and zone/node [Zone.js] Unhandled promise rejections are handled different in zone and zone/node Apr 19, 2023
@alan-agius4 alan-agius4 changed the title [Zone.js] Unhandled promise rejections are handled different in zone and zone/node Unhandled promise rejections are handled different in zone and zone/node Apr 19, 2023
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Apr 20, 2023
Currently, when there is an error inside a lazy loaded component contructor an unhandled promise rejection is thrown which causes the error handler not to be invoked on the server due to an issue with Zone.js see angular#49930.

While this is a workaround for that in general we should not cause unhandled promise rejections and always await and catch promises especially going for going zoneless, this is because Node.js does not wait for pending unhandled promises and these will cause the process to exit. See: nodejs/node#22088

Eventually, the framework will need to be updated to better handle  promises in certain listeners for zoneless and avoid unhandled promises

Example on potential problematic code.
````ts
{
    provide: APP_BOOTSTRAP_LISTENER,
    useFactory: () => {
      Promise.resolve().then(() => {})
    },
}
```

This change is also a set closer to address angular#33642
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Apr 20, 2023
Currently, when there is an error inside a lazy loaded component contructor an unhandled promise rejection is thrown which causes the error handler not to be invoked on the server due to an issue with Zone.js see angular#49930.

While this is a workaround for that in general we should not cause unhandled promise rejections and always await and catch promises especially going for going zoneless, this is because Node.js does not wait for pending unhandled promises and these will cause the process to exit. See: nodejs/node#22088

Eventually, the framework will need to be updated to better handle  promises in certain listeners for zoneless and avoid unhandled promises

Example on potential problematic code.
````ts
{
    provide: APP_BOOTSTRAP_LISTENER,
    useFactory: () => {
      Promise.resolve().then(() => {})
    },
}
```

This change is also a step closer to address angular#33642
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Apr 20, 2023
Currently, when there is an error inside a lazy loaded component contructor an unhandled promise rejection is thrown which causes the error handler not to be invoked on the server due to an issue with Zone.js see angular#49930.

While this is a workaround for that in general we should not cause unhandled promise rejections and always await and catch promises especially going for going zoneless, this is because Node.js does not wait for pending unhandled promises and these will cause the process to exit. See: nodejs/node#22088

Eventually, the framework will need to be updated to better handle  promises in certain listeners for zoneless and avoid unhandled promises

Example on potential problematic code.
````ts
{
    provide: APP_BOOTSTRAP_LISTENER,
    useFactory: () => {
      Promise.resolve().then(() => {})
    },
}
```

This change is also a step closer to address angular#33642
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Apr 20, 2023
Currently, when there is an error inside a lazy loaded component constructor an unhandled promise rejection is thrown which causes the error handler not to be invoked on the server due to an issue with Zone.js see angular#49930.

While this is a workaround for that in general we should not cause unhandled promise rejections and always await and catch promises especially going for going zoneless, this is because Node.js does not wait for pending unhandled promises and these will cause the process to exit. See: nodejs/node#22088

Eventually, the framework will need to be updated to better handle  promises in certain listeners for zoneless and avoid unhandled promises

Example on potential problematic code.
```ts
{
    provide: APP_BOOTSTRAP_LISTENER,
    useFactory: () => {
      Promise.resolve().then(() => {})
    },
}
```

This change is also a step closer to address angular#33642
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Apr 20, 2023
Currently, when there is an error inside a lazy loaded component constructor an unhandled promise rejection is thrown which causes the error handler not to be invoked on the server due to an issue with Zone.js see angular#49930.

While this is a workaround for that in general we should not cause unhandled promise rejections and always await and catch promises especially going for going zoneless, this is because Node.js does not wait for pending unhandled promises and these will cause the process to exit. See: nodejs/node#22088

Eventually, the framework will need to be updated to better handle  promises in certain listeners for zoneless and avoid unhandled promises

Example on potential problematic code.
```ts
{
    provide: APP_BOOTSTRAP_LISTENER,
    useFactory: () => {
      Promise.resolve().then(() => {})
    },
}
```

This change is also a step closer to address angular#33642
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Apr 20, 2023
Currently, when there is an error inside a lazy loaded component constructor an unhandled promise rejection is thrown which causes the error handler not to be invoked on the server due to an issue with Zone.js see angular#49930.

While this is a workaround for that in general we should not cause unhandled promise rejections and always await and catch promises especially going for going zoneless, this is because Node.js does not wait for pending unhandled promises and these will cause the process to exit. See: nodejs/node#22088

Eventually, the framework will need to be updated to better handle  promises in certain listeners for zoneless and avoid unhandled promises

Example on potential problematic code.
```ts
{
    provide: APP_BOOTSTRAP_LISTENER,
    useFactory: () => {
      Promise.resolve().then(() => {})
    },
}
```

This change is also a step closer to address angular#33642
@alan-agius4 alan-agius4 added regression Indicates than the issue relates to something that worked in a previous version and removed regression Indicates than the issue relates to something that worked in a previous version labels Apr 26, 2023
@alan-agius4 alan-agius4 added the regression Indicates than the issue relates to something that worked in a previous version label May 25, 2023
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Jun 2, 2023
In angular#49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to path the entire promise on Node.

The original problem `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of nodejs/node#45175

While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later.

Closes angular#50513, closes angular#50457, closes angular#50414 and closes angular#49930
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Jun 2, 2023
In angular#49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to patch the entire promise on Node.

The original `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of nodejs/node#45175.

While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later.

Closes angular#50513, closes angular#50457, closes angular#50414 and closes angular#49930
alan-agius4 added a commit to alan-agius4/angular that referenced this issue Jun 2, 2023
In angular#49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to patch the entire promise on Node.

The original `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of nodejs/node#45175.

While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later.

Closes angular#50513, closes angular#50457, closes angular#50414 and closes angular#49930
@alxhub alxhub closed this as completed in cb31dbc Jun 2, 2023
alxhub pushed a commit that referenced this issue Jun 2, 2023
In #49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to patch the entire promise on Node.

The original `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of nodejs/node#45175.

While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later.

Closes #50513, closes #50457, closes #50414 and closes #49930

PR Close #50552
crapStone pushed a commit to Calciumdibromid/CaBr2 that referenced this issue Jun 15, 2023
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [@angular/animations](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fanimations/16.0.4/16.1.1) |
| [@angular/common](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcommon/16.0.4/16.1.1) |
| [@angular/compiler](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler/16.0.4/16.1.1) |
| [@angular/compiler-cli](https://github.com/angular/angular/tree/main/packages/compiler-cli) ([source](https://github.com/angular/angular)) | devDependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcompiler-cli/16.0.4/16.1.1) |
| [@angular/core](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fcore/16.0.4/16.1.1) |
| [@angular/forms](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fforms/16.0.4/16.1.1) |
| [@angular/platform-browser](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser/16.0.4/16.1.1) |
| [@angular/platform-browser-dynamic](https://github.com/angular/angular) | dependencies | minor | [`16.0.4` -> `16.1.1`](https://renovatebot.com/diffs/npm/@angular%2fplatform-browser-dynamic/16.0.4/16.1.1) |
| [zone.js](https://github.com/angular/angular) ([changelog](https://github.com/angular/angular/blob/master/packages/zone.js/CHANGELOG.md)) | dependencies | patch | [`0.13.0` -> `0.13.1`](https://renovatebot.com/diffs/npm/zone.js/0.13.0/0.13.1) |

---

### Release Notes

<details>
<summary>angular/angular (@&#8203;angular/animations)</summary>

### [`v16.1.1`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1611-2023-06-14)

[Compare Source](angular/angular@16.1.0...16.1.1)

##### compiler-cli

| Commit | Type | Description |
| -- | -- | -- |
| [71360b3a3e](angular/angular@71360b3) | fix | libraries compiled with v16.1+ breaking with Angular framework v16.0.x ([#&#8203;50715](angular/angular#50715)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [d9bed48eb5](angular/angular@d9bed48) | fix | extend toSignal to accept any Subscribable ([#&#8203;50162](angular/angular#50162)) |

##### migrations

| Commit | Type | Description |
| -- | -- | -- |
| [5e1d8444ae](angular/angular@5e1d844) | fix | Prevent a component from importing itself. ([#&#8203;50554](angular/angular#50554)) |

<!-- CHANGELOG SPLIT MARKER -->

### [`v16.1.0`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1610-2023-06-13)

[Compare Source](angular/angular@16.0.6...16.1.0)

##### compiler

| Commit | Type | Description |
| -- | -- | -- |
| [4e663297c5](angular/angular@4e66329) | fix | error when reading compiled input transforms metadata in JIT mode ([#&#8203;50600](angular/angular#50600)) |
| [721bc72649](angular/angular@721bc72) | fix | resolve deprecation warning with TypeScript 5.1 ([#&#8203;50460](angular/angular#50460)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [68017d4e75](angular/angular@68017d4) | feat | add ability to transform input values ([#&#8203;50420](angular/angular#50420)) |
| [69dadd2502](angular/angular@69dadd2) | feat | support TypeScript 5.1 ([#&#8203;50156](angular/angular#50156)) |
| [c0ebe34cbd](angular/angular@c0ebe34) | fix | add additional component metadata to component ID generation ([#&#8203;50336](angular/angular#50336)) |

##### http

| Commit | Type | Description |
| -- | -- | -- |
| [85c5427582](angular/angular@85c5427) | feat | Introduction of the `fetch` Backend for the `HttpClient` ([#&#8203;50247](angular/angular#50247)) |

<!-- CHANGELOG SPLIT MARKER -->

### [`v16.0.6`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1606-2023-06-13)

[Compare Source](angular/angular@16.0.5...16.0.6)

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [05ac0868c9](angular/angular@05ac086) | fix | avoid duplicated content during hydration while processing a component with i18n ([#&#8203;50644](angular/angular#50644)) |

<!-- CHANGELOG SPLIT MARKER -->

### [`v16.0.5`](https://github.com/angular/angular/blob/HEAD/CHANGELOG.md#&#8203;1605-2023-06-08)

[Compare Source](angular/angular@16.0.4...16.0.5)

##### compiler

| Commit | Type | Description |
| -- | -- | -- |
| [703b8fcac1](angular/angular@703b8fc) | fix | do not remove comments in component styles ([#&#8203;50346](angular/angular#50346)) |

##### core

| Commit | Type | Description |
| -- | -- | -- |
| [2b6da93e19](angular/angular@2b6da93) | fix | incorrectly throwing error for self-referencing component ([#&#8203;50559](angular/angular#50559)) |
| [c992109d6c](angular/angular@c992109) | fix | wait for HTTP in `ngOnInit` correctly before server render ([#&#8203;50573](angular/angular#50573)) |

##### platform-server

| Commit | Type | Description |
| -- | -- | -- |
| [c0d4086c6e](angular/angular@c0d4086) | fix | surface errors during rendering ([#&#8203;50587](angular/angular#50587)) |

<!-- CHANGELOG SPLIT MARKER -->

</details>

<details>
<summary>angular/angular (zone.js)</summary>

### [`v0.13.1`](https://github.com/angular/angular/blob/HEAD/packages/zone.js/CHANGELOG.md#v0131-httpsgithubcomangularangularcomparezonejs-0130zonejs-v0131-2023-06-09)

[Compare Source](angular/angular@zone.js-0.13.0...zone.js-0.13.1)

##### Bug Fixes

-   **zone.js:** enable monkey patching of the `queueMicrotask()` API in node.js ([#&#8203;50467](angular/angular#50467)) ([381cb98](angular/angular@381cb98))
-   **zone.js:** enable monkey patching of the `queueMicrotask()` API in node.js ([#&#8203;50530](angular/angular#50530)) ([7837f71](angular/angular@7837f71))
-   **zone.js:** patch entire promise in node ([#&#8203;50552](angular/angular#50552)) ([cb31dbc](angular/angular@cb31dbc)), closes [#&#8203;50513](angular/angular#50513) [#&#8203;50457](angular/angular#50457) [#&#8203;50414](angular/angular#50414) [#&#8203;49930](angular/angular#49930)
-   **zone.js:** revert Mocha it.skip, describe.skip method patch ([#&#8203;49329](angular/angular#49329)) ([5a2b622](angular/angular@5a2b622))

##### Features

-   **zone.js:** jest 29 should ignore uncaught error console log ([#&#8203;49325](angular/angular#49325)) ([bc412fd](angular/angular@bc412fd)), closes [#&#8203;49110](angular/angular#49110)

##### Reverts

-   Revert "fix(zone.js): enable monkey patching of the `queueMicrotask()` API in node.js ([#&#8203;50467](angular/angular#50467))" ([#&#8203;50529](angular/angular#50529)) ([7567348](angular/angular@7567348)), closes [#&#8203;50467](angular/angular#50467) [#&#8203;50529](angular/angular#50529) [#&#8203;50529](angular/angular#50529)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://github.com/renovatebot/renovate/discussions) if that's undesired.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNS4xMTUuMiIsInVwZGF0ZWRJblZlciI6IjM1LjExNy4yIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9-->

Co-authored-by: cabr2-bot <cabr2.help@gmail.com>
Reviewed-on: https://codeberg.org/Calciumdibromid/CaBr2/pulls/1927
Reviewed-by: Epsilon_02 <epsilon_02@noreply.codeberg.org>
Co-authored-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
Co-committed-by: Calciumdibromid Bot <cabr2_bot@noreply.codeberg.org>
@angular-automatic-lock-bot
Copy link

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 Jul 3, 2023
thomasturrell pushed a commit to thomasturrell/angular that referenced this issue Aug 29, 2023
In angular#49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to patch the entire promise on Node.

The original `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of nodejs/node#45175.

While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later.

Closes angular#50513, closes angular#50457, closes angular#50414 and closes angular#49930

PR Close angular#50552
ChellappanRajan pushed a commit to ChellappanRajan/angular that referenced this issue Jan 23, 2024
In angular#49144 we introduced a change to only path `Promise.prototype.then` due to Node.js `SafePromise` complaining about `Promise.prototype.then` called on incompatible receiver. This however introduced a number of regressions. This commit reverts this change and re-introduces the changes to patch the entire promise on Node.

The original `SafePromise` problem is no longer reproducible as of Node.js version 18.13+ as it was addressed as part of nodejs/node#45175.

While the Angular CLI does not yet generate ESM server bundles, users using ESM with dynamic imports will require using Node.js 18.13 or later.

Closes angular#50513, closes angular#50457, closes angular#50414 and closes angular#49930

PR Close angular#50552
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: zones regression Indicates than the issue relates to something that worked in a previous version type: bug/fix
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants