-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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
doc: improve http.request and https.request options #1551
Conversation
If we want to explicitly support the |
Also copied all options from |
preferred over `host`. | ||
- `family`: IP address family to use when resolving `host` and `hostname`. | ||
Valid values are `4` or `6`. When unspecified, both IP v4 and v6 will be | ||
used. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So basically #708..
There was a problem hiding this comment.
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.
@Fishrock123 Any objections to me merging this right now? I think it's a clear improvement. |
LGTM |
This adds a few previously undocumented option to both functions. PR-URL: #1551 Fixes: #1082 Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
Landed in b4ad5d7 |
Fixes #1082. Also made the
hostname
option more clear.R=@cjihrig