Skip to content

stream: add new when constructing ERR_MULTIPLE_CALLBACK - #52110

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
haze:pr/hb/err-multiple-callback-fix
Mar 18, 2024
Merged

stream: add new when constructing ERR_MULTIPLE_CALLBACK#52110
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
haze:pr/hb/err-multiple-callback-fix

Conversation

@haze

@hazehaze commented Mar 16, 2024

Copy link
Copy Markdown
Contributor

commit c71e548 changed NodeError from a function to a class, and missed a spot where ERR_MULTIPLE_CALLBACK was being instantiated. This commit fixes that by adding the new keyword to that instance.

@nodejs-github-bot

nodejs-github-bot commented Mar 16, 2024

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@nodejs-github-botnodejs-github-bot added the needs-ci PRs that need a full CI run. label Mar 16, 2024
@debadree25debadree25 added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Mar 16, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 16, 2024
@nodejs-github-bot

nodejs-github-bot commented Mar 16, 2024

Copy link
Copy Markdown
Collaborator

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

@vweeversvweevers mentioned this pull request Mar 16, 2024
Open
lpinca
lpinca approved these changes Mar 16, 2024
View reviewed changes
Comment threadtest/parallel/test-stream-err-multiple-callback-construction.js Outdated
Comment threadtest/parallel/test-stream-err-multiple-callback-construction.js Outdated
lpinca
lpinca reviewed Mar 16, 2024
View reviewed changes
Comment threadtest/parallel/test-stream-err-multiple-callback-construction.js Outdated
@haze
hazeforce-pushed the pr/hb/err-multiple-callback-fix branch from e30cf1e to dcdcf76CompareMarch 16, 2024 20:31
lpinca
lpinca approved these changes Mar 16, 2024
View reviewed changes
Comment threadtest/parallel/test-stream-err-multiple-callback-construction.js Outdated
@lpincalpinca added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. request-ci Add this label to start a Jenkins CI on a PR. labels Mar 16, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 16, 2024
@nodejs-github-bot

nodejs-github-bot commented Mar 16, 2024

Copy link
Copy Markdown
Collaborator

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

@haze@lpinca
stream: add new when constructing ERR_MULTIPLE_CALLBACK
a5a3fd7
commit c71e548 changed NodeError
from a function to a class, and missed a spot where
`ERR_MULTIPLE_CALLBACK` was being instantiated. This commit fixes
that by adding the new keyword to that instance.
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
@haze
hazeforce-pushed the pr/hb/err-multiple-callback-fix branch from f3e8260 to a5a3fd7CompareMarch 17, 2024 01:53
@lpincalpinca added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 17, 2024
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 17, 2024
@nodejs-github-bot

nodejs-github-bot commented Mar 17, 2024

Copy link
Copy Markdown
Collaborator

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

@atlowChemiatlowChemi added the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 17, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 18, 2024
@nodejs-github-bot

nodejs-github-bot commented Mar 18, 2024

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/52110
✔ Done loading data for nodejs/node/pull/52110
----------------------------------- PR info ------------------------------------
Title stream: add `new` when constructing `ERR_MULTIPLE_CALLBACK` (#52110)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch haze:pr/hb/err-multiple-callback-fix -> nodejs:main
Labels author ready, needs-ci, commit-queue-squash
Commits 1
- stream: add `new` when constructing `ERR_MULTIPLE_CALLBACK`
Committers 1
- Haze Booth PR-URL: https://github.com/nodejs/node/pull/52110
Reviewed-By: Robert Nagy Reviewed-By: Debadree Chatterjee Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/52110
Reviewed-By: Robert Nagy Reviewed-By: Debadree Chatterjee Reviewed-By: Luigi Pinca --------------------------------------------------------------------------------
⚠ Commits were pushed since the last approving review:
⚠ - stream: add `new` when constructing `ERR_MULTIPLE_CALLBACK`
ℹ This PR was created on Sat, 16 Mar 2024 03:29:48 GMT
✔ Approvals: 3
✔ - Robert Nagy (@ronag) (TSC): https://github.com/nodejs/node/pull/52110#pullrequestreview-1940941832
✔ - Debadree Chatterjee (@debadree25): https://github.com/nodejs/node/pull/52110#pullrequestreview-1940956860
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/52110#pullrequestreview-1941334372
✔ Last GitHub CI successful
ℹ Last Full PR CI on 2024-03-17T06:35:38Z: https://ci.nodejs.org/job/node-test-pull-request/57784/
- Querying data for job/node-test-pull-request/57784/
✔ Last Jenkins CI successful
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/8320924494

@nodejs-github-botnodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Mar 18, 2024
mcollina
mcollina approved these changes Mar 18, 2024
View reviewed changes

@mcollinamcollina 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.

lgtm

@mcollinamcollina removed the needs-ci PRs that need a full CI run. label Mar 18, 2024
@mcollinamcollina added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Mar 18, 2024
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Mar 18, 2024
@nodejs-github-bot
nodejs-github-bot merged commit 63391e7 into nodejs:mainMar 18, 2024
@nodejs-github-bot

nodejs-github-bot commented Mar 18, 2024

Copy link
Copy Markdown
Collaborator

Landed in 63391e7

@mcollina

mcollina commented Mar 18, 2024

Copy link
Copy Markdown
Member

@nodejs/releasers could you include this in the upcoming v18.x release?

@richardlau

richardlau commented Mar 18, 2024

Copy link
Copy Markdown
Member

@nodejs/releasers could you include this in the upcoming v18.x release?

It would break with policy because this hasn't gone out in a current release yet (default policy is that things should be in current for two weeks before going into an LTS line).

I'm also confused -- according to the description, this PR fixes an issue introduced by c71e548 which is #49654 and that isn't on v18.x. 😕

@haze

haze commented Mar 26, 2024

Copy link
Copy Markdown
ContributorAuthor

@richardlau is this a candidate for v20.x?

@richardlau

richardlau commented Mar 26, 2024

Copy link
Copy Markdown
Member

It needs to be released in a Node.js current release (either 21 or 22 when it arrives next month) before it can land in 20.x.

@RafaelGSSRafaelGSS mentioned this pull request Apr 15, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request May 2, 2024
@haze@lpinca@marco-ippolito
stream: add new when constructing ERR_MULTIPLE_CALLBACK
5c30fec
commit c71e548 changed NodeError
from a function to a class, and missed a spot where
`ERR_MULTIPLE_CALLBACK` was being instantiated. This commit fixes
that by adding the new keyword to that instance.
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #52110
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@marco-ippolitomarco-ippolito mentioned this pull request May 2, 2024
Merged
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
@haze@lpinca@marco-ippolito
stream: add new when constructing ERR_MULTIPLE_CALLBACK
f8209ff
commit c71e548 changed NodeError
from a function to a class, and missed a spot where
`ERR_MULTIPLE_CALLBACK` was being instantiated. This commit fixes
that by adding the new keyword to that instance.
Co-authored-by: Luigi Pinca <luigipinca@gmail.com>
PR-URL: #52110
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Debadree Chatterjee <debadree333@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mcollinamcollinamcollina approved these changes
@lpincalpincalpinca approved these changes
@ronagronagronag approved these changes
+1 more reviewer
@debadree25debadree25debadree25 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

8 participants

@haze@nodejs-github-bot@mcollina@richardlau@lpinca@ronag@debadree25@atlowChemi