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

References to an "KeepAlive" flag in the http.Agent constructor, when it is actually "keepAlive" #10567

Closed
papiro opened this issue Jan 1, 2017 · 8 comments
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. http Issues or PRs related to the http subsystem.

Comments

@papiro
Copy link

papiro commented Jan 1, 2017

  • Version:
  • Platform:
  • Subsystem:

In the version 7.3.0 documentation, in the page on http, in the first few paragraphs of the http.Agent section it makes references to a KeepAlive flag in the http.Agent constructor which you can specify as true if you'd like to keep unused sockets pooled. Admittedly this caused me a decent amount of confusion because of the case mismatch :)

@Fishrock123 Fishrock123 added the doc Issues and PRs related to the documentations. label Jan 1, 2017
@mscdex mscdex added the http Issues or PRs related to the http subsystem. label Jan 1, 2017
@evanlucas
Copy link
Contributor

@papiro would you be interested in making a contribution to the documentation that fixes this? It would be greatly appreciated. The actual file that is used to generate that documentation can be found at https://github.com/nodejs/node/blob/master/doc/api/http.md. Feel free to ping me if you have any questions on making this change. Thanks for bringing it to our attention!

@gibfahn gibfahn added the good first issue Issues that are suitable for first-time contributors. label Jan 2, 2017
@gibfahn
Copy link
Member

gibfahn commented Jan 2, 2017

Adding the good first contribution label, if anyone would like to PR this change (including @papiro !) then awesome.

@papiro
Copy link
Author

papiro commented Jan 2, 2017

I'll give it a shot :) Thanks!

@papiro
Copy link
Author

papiro commented Jan 2, 2017

To be honest, I'm trying to grok what the documentation is actually trying to say and I just don't get it. This line specifically:

This means that Node.js's pool has the benefit of keep-alive when under load but still does not require developers to manually close the HTTP clients using KeepAlive.

Is it referring to HTTP clients with an agent that has the keepAlive flag set to true (Nodejs-specific)? I've googled "KeepAlive" and can't find any useful references to it (everything seems to be talking about the Connection header with a value of keep-alive.

Also, this line:

If you opt into using HTTP KeepAlive...

Is it referring again to the option to the http.Agent constructor? It's confusing because it refers to it as "HTTP" KeepAlive, which makes me think it's not Node-specific.

Any clarity on these would be greatly appreciated.

fhalde added a commit to fhalde/node that referenced this issue Jan 4, 2017
@fhalde fhalde mentioned this issue Jan 4, 2017
2 tasks
@martinakraus
Copy link

Is this stilla point?

@papiro
Copy link
Author

papiro commented Jan 9, 2017 via email

@sam-github
Copy link
Contributor

Have you seen #10715, in process of addressing this?

@papiro
Copy link
Author

papiro commented Jan 9, 2017 via email

@lance lance closed this as completed Jan 25, 2017
lance added a commit that referenced this issue Jan 25, 2017
The HTTP Keep-Alive text was inconsistent. These changes follow the
following rules

* When referring to flag used in code, it should always be written using
  backticks and in camel case. E.g. `keepAlive`.
* When referring to the mechanism Keep-Alive functionality as described
  in the HTTP 1.1 RFC, it is written as 'HTTP Keep-Alive', without the
  use of backticks.
* When referring to the request header, it should always use backticks
  and be written as `Connection: keep-alive`.

This commit also includes some changes to how `http.Agent` is
referenced. When `Agent` is used as a reference to an object or
instance, backticks should always be used.

And lastly, the documentation about `Agent` behavior around HTTP
Keep-Alive has been clarified and improved.

Fixes: #10567
PR-URL: #10715

Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this issue Jan 28, 2017
The HTTP Keep-Alive text was inconsistent. These changes follow the
following rules

* When referring to flag used in code, it should always be written using
  backticks and in camel case. E.g. `keepAlive`.
* When referring to the mechanism Keep-Alive functionality as described
  in the HTTP 1.1 RFC, it is written as 'HTTP Keep-Alive', without the
  use of backticks.
* When referring to the request header, it should always use backticks
  and be written as `Connection: keep-alive`.

This commit also includes some changes to how `http.Agent` is
referenced. When `Agent` is used as a reference to an object or
instance, backticks should always be used.

And lastly, the documentation about `Agent` behavior around HTTP
Keep-Alive has been clarified and improved.

Fixes: #10567
PR-URL: #10715

Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 30, 2017
The HTTP Keep-Alive text was inconsistent. These changes follow the
following rules

* When referring to flag used in code, it should always be written using
  backticks and in camel case. E.g. `keepAlive`.
* When referring to the mechanism Keep-Alive functionality as described
  in the HTTP 1.1 RFC, it is written as 'HTTP Keep-Alive', without the
  use of backticks.
* When referring to the request header, it should always use backticks
  and be written as `Connection: keep-alive`.

This commit also includes some changes to how `http.Agent` is
referenced. When `Agent` is used as a reference to an object or
instance, backticks should always be used.

And lastly, the documentation about `Agent` behavior around HTTP
Keep-Alive has been clarified and improved.

Fixes: nodejs#10567
PR-URL: nodejs#10715

Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
italoacasas pushed a commit to italoacasas/node that referenced this issue Jan 30, 2017
The HTTP Keep-Alive text was inconsistent. These changes follow the
following rules

* When referring to flag used in code, it should always be written using
  backticks and in camel case. E.g. `keepAlive`.
* When referring to the mechanism Keep-Alive functionality as described
  in the HTTP 1.1 RFC, it is written as 'HTTP Keep-Alive', without the
  use of backticks.
* When referring to the request header, it should always use backticks
  and be written as `Connection: keep-alive`.

This commit also includes some changes to how `http.Agent` is
referenced. When `Agent` is used as a reference to an object or
instance, backticks should always be used.

And lastly, the documentation about `Agent` behavior around HTTP
Keep-Alive has been clarified and improved.

Fixes: nodejs#10567
PR-URL: nodejs#10715

Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
jasnell pushed a commit that referenced this issue Mar 8, 2017
The HTTP Keep-Alive text was inconsistent. These changes follow the
following rules

* When referring to flag used in code, it should always be written using
  backticks and in camel case. E.g. `keepAlive`.
* When referring to the mechanism Keep-Alive functionality as described
  in the HTTP 1.1 RFC, it is written as 'HTTP Keep-Alive', without the
  use of backticks.
* When referring to the request header, it should always use backticks
  and be written as `Connection: keep-alive`.

This commit also includes some changes to how `http.Agent` is
referenced. When `Agent` is used as a reference to an object or
instance, backticks should always be used.

And lastly, the documentation about `Agent` behavior around HTTP
Keep-Alive has been clarified and improved.

Fixes: #10567
PR-URL: #10715

Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
The HTTP Keep-Alive text was inconsistent. These changes follow the
following rules

* When referring to flag used in code, it should always be written using
  backticks and in camel case. E.g. `keepAlive`.
* When referring to the mechanism Keep-Alive functionality as described
  in the HTTP 1.1 RFC, it is written as 'HTTP Keep-Alive', without the
  use of backticks.
* When referring to the request header, it should always use backticks
  and be written as `Connection: keep-alive`.

This commit also includes some changes to how `http.Agent` is
referenced. When `Agent` is used as a reference to an object or
instance, backticks should always be used.

And lastly, the documentation about `Agent` behavior around HTTP
Keep-Alive has been clarified and improved.

Fixes: #10567
PR-URL: #10715

Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. good first issue Issues that are suitable for first-time contributors. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants