Skip to content

doc: improve http.request and https.request options - #1551

Closed
silverwind wants to merge 3 commits into
nodejs:masterfrom
silverwind:doc-http-request
Closed

doc: improve http.request and https.request options#1551
silverwind wants to merge 3 commits into
nodejs:masterfrom
silverwind:doc-http-request

Conversation

@silverwind

@silverwindsilverwind commented Apr 28, 2015

Copy link
Copy Markdown
Contributor

Fixes#1082. Also made the hostname option more clear.

R=@cjihrig

@silverwindsilverwind added the doc Issues and PRs related to the documentations. label Apr 28, 2015
@cjihrig

cjihrig commented Apr 28, 2015

Copy link
Copy Markdown
Contributor

If we want to explicitly support the family option here, this LGTM.

@silverwind

silverwind commented Apr 28, 2015

Copy link
Copy Markdown
ContributorAuthor

Also copied all options from http.request to https.request, it was lagging behind on docs.

@silverwindsilverwind changed the title doc: improve http.connect option descriptionsdoc: improve http.connect optionsApr 28, 2015
Fishrock123
Fishrock123 reviewed Apr 29, 2015
View reviewed changes
Comment threaddoc/api/http.markdown

@Fishrock123Fishrock123Apr 29, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@cjihrig didn't your example in the original issue state that this was required for ipv6?

@cjihrigcjihrigApr 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@Fishrock123 TL;DR, I believe @silverwind statement to be correct here.

In that particular example, IPv6 was required, as the server was only listening on '::1'. When dns.lookup() executes, it looks up localhost and returns the results [ '127.0.0.1', '::1', 'fe80::1' ] (on my local machine). The first result is used, and the family is inferred as 4.

@Fishrock123Fishrock123Apr 30, 2015

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So basically #708..

@silverwindsilverwindApr 30, 2015

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

I understood #708 as a request to attempt multiple IPs from the same family.

What exactly does a browser on dual stack do if it encounters both v4 and v6? Attempt both in succession? In parallel? Whatever it does, I think that's what we should do if no family is given.

@silverwindsilverwind changed the title doc: improve http.connect optionsdoc: improve http.request and https.request optionsApr 30, 2015
@silverwind

silverwind commented May 1, 2015

Copy link
Copy Markdown
ContributorAuthor

@Fishrock123 Any objections to me merging this right now? I think it's a clear improvement.

@Fishrock123

Fishrock123 commented May 1, 2015

Copy link
Copy Markdown
Contributor

LGTM

silverwind added a commit that referenced this pull request May 1, 2015
@silverwind
doc: improve http.request and https.request opts
b4ad5d7
This adds a few previously undocumented option to both functions.
PR-URL: #1551Fixes: #1082
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@silverwind

silverwind commented May 1, 2015

Copy link
Copy Markdown
ContributorAuthor

Landed in b4ad5d7

@rvaggrvagg mentioned this pull request May 2, 2015
Closed
@B020239B020239 mentioned this pull request Mar 9, 2024
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

docIssues and PRs related to the documentations.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Dns resolver ipv6

3 participants

@silverwind@cjihrig@Fishrock123