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.

Sign up for GitHub

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

Jump to bottom

fix(ext/node): support createConnection option in node:http.request() #25470

Draft
wants to merge 114 commits into
base: main
Choose a base branch
from
Draft

fix(ext/node): support createConnection option in node:http.request() #25470

wants to merge 114 commits into from
+414 −229

Conversation

satyarohith
Copy link
Member

@satyarohith satyarohith commented Sep 5, 2024 •
edited
Loading

Work in progress. Feature works but breaks existing tests.

Closes #19507

@kt3k kt3k added the ci-draft Run the CI on draft PRs. label Sep 26, 2024
@kt3k kt3k changed the title WIP: support createConnection option in node:http.request() fix(ext/node): WIP support createConnection option in node:http.request() Sep 26, 2024
@kt3k
Copy link
Member

kt3k commented Oct 29, 2024 •
edited
Loading

I guess this workaround (stopping startTls in afterConnect in tls.connect) is still necessary. ref: 8e55dab

I'll trying restore that workaround and see if it fixes node-gyp issue. nevermind

@kt3k kt3k mentioned this pull request Oct 30, 2024
Closed
@bartlomieju bartlomieju mentioned this pull request Nov 5, 2024
Open
@kt3k kt3k added the ci-draft Run the CI on draft PRs. label Nov 14, 2024
@kt3k kt3k marked this pull request as draft November 14, 2024 06:42
@kt3k
restore cares workaround for windows
ce620ce
@kt3k
use default bind address (0.0.0.0) for Deno.serve
86c32ca 
If hostname specified to localhost on Deno.serve, it seems listening on ::1,
which causes problem on windows.
@kt3k
remove unnecessary logging
d5d6d58
@kt3k
try use request:close event
11ce866
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bartlomieju bartlomieju bartlomieju left review comments

@kt3k kt3k kt3k left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
ci-draft Run the CI on draft PRs.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

node: ClientRequest.options.createConnection
5 participants
@satyarohith @kt3k @harvestcore @cobbvanth @bartlomieju