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

lib: support promise reject for http2.connect promisify integration #53475

Closed
wants to merge 6 commits into from

Conversation

ehsankhfr
Copy link
Contributor

This PR enhances the connect function to support Promise-based usage, adding error handling to the Promise wrapper to reject the Promise if an error occurs during the session's connect event.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run. labels Jun 16, 2024
Copy link
Member

@pimterry pimterry left a comment

Choose a reason for hiding this comment

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

Good idea! Two small comments but this definitely seems useful to me 👍

test/parallel/test-http2-client-promisify-connect.js Outdated Show resolved Hide resolved
lib/internal/http2/core.js Show resolved Hide resolved
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2024
@nodejs-github-bot

This comment was marked as outdated.

@ehsankhfr
Copy link
Contributor Author

@pimterry @marco-ippolito thanks for your comments, the requested changes are done

@mcollina mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2024
@nodejs-github-bot
Copy link
Collaborator

@ehsankhfr ehsankhfr requested a review from pimterry June 18, 2024 12:37
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@pimterry pimterry added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jun 18, 2024
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

pimterry pushed a commit that referenced this pull request Jun 20, 2024
PR-URL: #53475
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
@pimterry
Copy link
Member

Landed in 42bce05

@pimterry pimterry closed this Jun 20, 2024
targos pushed a commit that referenced this pull request Jun 20, 2024
PR-URL: #53475
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
sophonieb pushed a commit to sophonieb/node that referenced this pull request Jun 20, 2024
PR-URL: nodejs#53475
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
bmeck pushed a commit to bmeck/node that referenced this pull request Jun 22, 2024
PR-URL: nodejs#53475
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
PR-URL: #53475
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
marco-ippolito pushed a commit that referenced this pull request Jul 19, 2024
PR-URL: #53475
Reviewed-By: Tim Perry <pimterry@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. http2 Issues or PRs related to the http2 subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants