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

fix(core): run macrotasks and next ticks after polling dynamic imports #17173

Merged

Conversation

bartlomieju
Copy link
Member

This commit fixes handling of rejected promises in dynamic imports evaluation.

Previously we were running callbacks for next ticks and macrotasks before polling
dynamic imports and checked for unhandled rejections immediately after. This is wrong,
as unhandledrejection event is dispatched and its callbacks are run as macrotasks.

This commit changes order of actions performed by the event loop to following:

  • poll async ops
  • poll dynamic imports
  • run next tick callbacks
  • run macrotask callbacks
  • check for unhandled promise rejections

Closes #16909
Closes #15661

Copy link
Member

@crowlKats crowlKats left a comment

Choose a reason for hiding this comment

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

LGTM

@bartlomieju bartlomieju merged commit 208c91b into denoland:main Dec 23, 2022
@bartlomieju bartlomieju deleted the debug_unhandled_rejection_miss3 branch December 23, 2022 18:46
bartlomieju added a commit that referenced this pull request Jan 5, 2023
#17173)

This commit fixes handling of rejected promises in dynamic imports
evaluation.

Previously we were running callbacks for next ticks and macrotasks
_before_ polling
dynamic imports and checked for unhandled rejections immediately after.
This is wrong,
as `unhandledrejection` event is dispatched and its callbacks are run as
macrotasks.

This commit changes order of actions performed by the event loop to
following:
- poll async ops
- poll dynamic imports
- run next tick callbacks
- run macrotask callbacks
- check for unhandled promise rejections
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants