FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

timers: fix multipleResolves in promisified timeouts/immediates by lundibundi · Pull Request #33949 · nodejs/node · GitHub

/ node Public

timers: fix multipleResolves in promisified timeouts/immediates - #33949

Closed
lundibundi wants to merge 2 commits into
nodejs:masterfrom
lundibundi:timeout-multiple-resolves-abort
ConversationCommits (2)ChecksFiles changed
Closed

timers: fix multipleResolves in promisified timeouts/immediates#33949
lundibundi wants to merge 2 commits into
nodejs:masterfrom
lundibundi:timeout-multiple-resolves-abort

Conversation

lundibundi commented Jun 18, 2020

Copy link
Copy Markdown
Member

After successful timer finish the abort event callback would still
reject (already resolved promise) upon calling abortController.abort().

Signed-off-by: Denys Otrishko shishugi@gmail.com

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

/cc @jasnell @nodejs/timers

timers: fix multipleResolves in promisified timeouts/immediates
4fa607f 
After successful timer finish the abort event callback would still
reject (already resolved promise) upon calling abortController.abort().

Signed-off-by: Denys Otrishko <shishugi@gmail.com>
nodejs-github-bot added the timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout. label Jun 18, 2020

nodejs-github-bot commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31926/

devsnek reviewed Jun 18, 2020
View reviewed changes
Comment thread test/parallel/test-timers-promisified.js Outdated
jasnell approved these changes Jun 18, 2020
View reviewed changes

jasnell left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Good catch

jasnell mentioned this pull request Jun 18, 2020
Closed
4 tasks
lundibundi added a commit to lundibundi/node that referenced this pull request Jun 18, 2020
test: print arguments passed to mustNotCall function
9dad0ba 
Refs: nodejs#33949 (comment)
Signed-off-by: Denys Otrishko <shishugi@gmail.com>
lundibundi mentioned this pull request Jun 18, 2020
Closed
4 tasks

nodejs-github-bot commented Jun 18, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31928/

github-actions Bot mentioned this pull request Jun 19, 2020
Open
36 tasks

nodejs-github-bot commented Jun 19, 2020 •
edited by addaleax
Loading

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/31931/ (:yellow_heart:)

jasnell added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Jun 19, 2020
This was referenced Jun 20, 2020
Open
Open

ronag commented Jun 21, 2020

Copy link
Copy Markdown
Member

Landed in 64d22c3

ronag closed this Jun 21, 2020
ronag pushed a commit that referenced this pull request Jun 21, 2020
timers: fix multipleResolves in promisified timeouts/immediates
64d22c3 
After successful timer finish the abort event callback would still
reject (already resolved promise) upon calling abortController.abort().

Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: #33949
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
This was referenced Jun 22, 2020
Open
Open
jasnell pushed a commit that referenced this pull request Jun 24, 2020
test: print arguments passed to mustNotCall function
225a03c 
Refs: #33949 (comment)
Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: #33951
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

codebytere commented Jun 27, 2020

Copy link
Copy Markdown
Member

@lundibundi could you please open a manual backport for this to v14.x-staging? it's conflict-heavy.

addaleax commented Jun 27, 2020

Copy link
Copy Markdown
Member

This depends on #33833, which is labelled dont-land-on-v14.x

codebytere pushed a commit that referenced this pull request Jun 27, 2020
test: print arguments passed to mustNotCall function
4a61013 
Refs: #33949 (comment)
Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: #33951
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jun 30, 2020
test: print arguments passed to mustNotCall function
9037ab3 
Refs: #33949 (comment)
Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: #33951
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 10, 2020
test: print arguments passed to mustNotCall function
7744f66 
Refs: #33949 (comment)
Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: #33951
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
codebytere pushed a commit that referenced this pull request Jul 14, 2020
test: print arguments passed to mustNotCall function
f568a48 
Refs: #33949 (comment)
Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: #33951
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos mentioned this pull request Apr 24, 2021
Closed
targos pushed a commit to targos/node that referenced this pull request Apr 30, 2021
timers: fix multipleResolves in promisified timeouts/immediates
290ce9f 
After successful timer finish the abort event callback would still
reject (already resolved promise) upon calling abortController.abort().

Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: nodejs#33949
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos pushed a commit that referenced this pull request Apr 30, 2021
timers: fix multipleResolves in promisified timeouts/immediates
59a8425 
After successful timer finish the abort event callback would still
reject (already resolved promise) upon calling abortController.abort().

Signed-off-by: Denys Otrishko <shishugi@gmail.com>

PR-URL: #33949
Backport-PR-URL: #38386
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
targos added backported-to-v14.x and removed author ready PRs that have at least one approval, no outstanding review comments, and a CI started. backport-open-v14.x labels Apr 30, 2021
danielleadams mentioned this pull request May 3, 2021
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
addaleax addaleax approved these changes
benjamingr benjamingr approved these changes
+1 more reviewer
devsnek devsnek left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

timers Issues and PRs related to the timers subsystem / setImmediate, setInterval, setTimeout.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL