GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | ||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | ||||||||||||||||
GeezerVison is showing github.com — http: overridable keep-alive behavior of `Agent` by indutny · Pull Request #13005 · nodejs/node Conversation
Member
Checklist
Affected core subsystem(s)http Introduce two overridable
These methods can be overridden by particular Motivation: destroy persisted sockets after some configurable timeout.
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
Author
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Contributor
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
mscdex
reviewed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wrap this in
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
mscdex
reviewed
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ditto, wrap in
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
mscdex
reviewed
|
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Why is req passed in here? It appears to be unused. Is it for agents that override the function and want to use it?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Yes, this is correct.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Couple of doc nits. Otherwise looks fine to me.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
returns a falsy value
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Nit: Dash (-) should be changed to a comma (,).
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ack.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
this should list the arguments... e.g.
* `socket` {net.Socket|tls.TLSSocket} description...
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ack.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
as above... this should list the arguments...
* `socket` {net.Socket|tls.TLSSocket} ...
* `req` ...
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ack.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
+1 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Brought up in CTC meeting today, nothing was added there, no objections and I voiced my support for this. Unless there's an objection brought up here I don't see why this couldn't move forward. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
I do have one outstanding comment that has not been addressed: #13005 (comment) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Can we get a citgm run on this @MylesBorins? This is a historically brittle area of code where modules tend to depend on esoteric and undocumented internal semantics. |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
CITGM CI: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/830/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
CITGM is clean.
Rubber stamp code.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
s/by/by a/
Also, I'm not sure that "Agent class child" is the best terminology? What about 'subclass' or 'implementation'?
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Ack.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Introduce two overridable `Agent` methods: * `keepSocketAlive(socket)` * `reuseSocket(socket, req)` These methods can be overridden by particular `Agent` class child to make keep-alive behavior customizable. Motivation: destroy persisted sockets after some configurable timeout. It is very non-trivial to do it with available primitives. Such program will most likely need to poke with undocumented events and methods of `Agent`. With introduced API such behavior is easy to implement. PR-URL: #13005 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
|
@nodejs/lts was it ever backported to v6.x? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@indutny not yet, it will land in a future semver minor |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
LTS WG agreed that this can be packported |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@indutny this will need to be manually backported. Would you be able to open one? |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Introduce two overridable `Agent` methods: * `keepSocketAlive(socket)` * `reuseSocket(socket, req)` These methods can be overridden by particular `Agent` class child to make keep-alive behavior customizable. Motivation: destroy persisted sockets after some configurable timeout. It is very non-trivial to do it with available primitives. Such program will most likely need to poke with undocumented events and methods of `Agent`. With introduced API such behavior is easy to implement. Backport-PR-URL: #18168 PR-URL: #13005 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
Introduce two overridable `Agent` methods: * `keepSocketAlive(socket)` * `reuseSocket(socket, req)` These methods can be overridden by particular `Agent` class child to make keep-alive behavior customizable. Motivation: destroy persisted sockets after some configurable timeout. It is very non-trivial to do it with available primitives. Such program will most likely need to poke with undocumented events and methods of `Agent`. With introduced API such behavior is easy to implement. Backport-PR-URL: #18168 PR-URL: #13005 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.
Notable Changes:
* console:
- added console.count() and console.clear() (James M Snell)
#12678
* crypto:
- expose ECDH class (Bryan English)
#8188
- added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
#10209
- warn on invalid authentication tag length (Tobias Nießen)
#17566
* deps:
- upgrade libuv to 1.16.1 (cjihrig)
#16835
* dgram:
- added socket.setMulticastInterface() (Will Young)
#7855
* http:
- add agent.keepSocketAlive and agent.reuseSocket as to allow
overridable keep-alive behavior of `Agent` (Fedor Indutny)
#13005
* lib:
- return this from net.Socket.end() (Sam Roberts)
#13481
* module:
- add builtinModules api that provides list of all builtin modules in
Node (Jon Moss)
#16386
* net:
- return this from getConnections() (Sam Roberts)
#13553
* promises:
- more robust stringification for unhandled rejections (Timothy Gu)
#13784
* repl:
- improve require() autocompletion (Alexey Orlenko)
#14409
* src:
- add openssl-system-ca-path configure option (Daniel Bevenius)
#16790
- add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
#12087
- add process.ppid (cjihrig)
#16839
* tls:
- accept `lookup` option for `tls.connect()` (Fedor Indutny)
#12839
* tools, build:
- a new macOS installer! (JP Wesselink)
#15179
* url:
- WHATWG URL api support (James M Snell)
#7448
* util:
- add %i and %f formatting specifiers (Roman Reiss)
#10308
PR-URL: #18342
Introduce two overridable `Agent` methods: * `keepSocketAlive(socket)` * `reuseSocket(socket, req)` These methods can be overridden by particular `Agent` class child to make keep-alive behavior customizable. Motivation: destroy persisted sockets after some configurable timeout. It is very non-trivial to do it with available primitives. Such program will most likely need to poke with undocumented events and methods of `Agent`. With introduced API such behavior is easy to implement. Backport-PR-URL: #18168 PR-URL: #13005 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.
Notable Changes:
* console:
- added console.count() and console.clear() (James M Snell)
#12678
* crypto:
- expose ECDH class (Bryan English)
#8188
- added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
#10209
- warn on invalid authentication tag length (Tobias Nießen)
#17566
* deps:
- upgrade libuv to 1.16.1 (cjihrig)
#16835
* dgram:
- added socket.setMulticastInterface() (Will Young)
#7855
* http:
- add agent.keepSocketAlive and agent.reuseSocket as to allow
overridable keep-alive behavior of `Agent` (Fedor Indutny)
#13005
* lib:
- return this from net.Socket.end() (Sam Roberts)
#13481
* module:
- add builtinModules api that provides list of all builtin modules in
Node (Jon Moss)
#16386
* net:
- return this from getConnections() (Sam Roberts)
#13553
* promises:
- more robust stringification for unhandled rejections (Timothy Gu)
#13784
* repl:
- improve require() autocompletion (Alexey Orlenko)
#14409
* src:
- add openssl-system-ca-path configure option (Daniel Bevenius)
#16790
- add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
#12087
- add process.ppid (cjihrig)
#16839
* tls:
- accept `lookup` option for `tls.connect()` (Fedor Indutny)
#12839
* tools, build:
- a new macOS installer! (JP Wesselink)
#15179
* url:
- WHATWG URL api support (James M Snell)
#7448
* util:
- add %i and %f formatting specifiers (Roman Reiss)
#10308
PR-URL: #18342
Introduce two overridable `Agent` methods: * `keepSocketAlive(socket)` * `reuseSocket(socket, req)` These methods can be overridden by particular `Agent` class child to make keep-alive behavior customizable. Motivation: destroy persisted sockets after some configurable timeout. It is very non-trivial to do it with available primitives. Such program will most likely need to poke with undocumented events and methods of `Agent`. With introduced API such behavior is easy to implement. Backport-PR-URL: #18168 PR-URL: #13005 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
This LTS release comes with 109 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 29 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.
Notable Changes:
* console:
- added console.count() and console.clear() (James M Snell)
#12678
* crypto:
- expose ECDH class (Bryan English)
#8188
- added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
#10209
- warn on invalid authentication tag length (Tobias Nießen)
#17566
* deps:
- upgrade libuv to 1.16.1 (cjihrig)
#16835
* dgram:
- added socket.setMulticastInterface() (Will Young)
#7855
* http:
- add agent.keepSocketAlive and agent.reuseSocket as to allow
overridable keep-alive behavior of `Agent` (Fedor Indutny)
#13005
* lib:
- return this from net.Socket.end() (Sam Roberts)
#13481
* module:
- add builtinModules api that provides list of all builtin modules in
Node (Jon Moss)
#16386
* net:
- return this from getConnections() (Sam Roberts)
#13553
* promises:
- more robust stringification for unhandled rejections (Timothy Gu)
#13784
* repl:
- improve require() autocompletion (Alexey Orlenko)
#14409
* src:
- add openssl-system-ca-path configure option (Daniel Bevenius)
#16790
- add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
#12087
- add process.ppid (cjihrig)
#16839
* tls:
- accept `lookup` option for `tls.connect()` (Fedor Indutny)
#12839
* tools, build:
- a new macOS installer! (JP Wesselink)
#15179
* url:
- WHATWG URL api support (James M Snell)
#7448
* util:
- add %i and %f formatting specifiers (Roman Reiss)
#10308
PR-URL: #18342
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.
Notable Changes:
* console:
- added console.count() and console.clear() (James M Snell)
#12678
* crypto:
- expose ECDH class (Bryan English)
#8188
- added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
#10209
- warn on invalid authentication tag length (Tobias Nießen)
#17566
* deps:
- upgrade libuv to 1.16.1 (cjihrig)
#16835
* dgram:
- added socket.setMulticastInterface() (Will Young)
#7855
* http:
- add agent.keepSocketAlive and agent.reuseSocket as to allow
overridable keep-alive behavior of `Agent` (Fedor Indutny)
#13005
* lib:
- return this from net.Socket.end() (Sam Roberts)
#13481
* module:
- add builtinModules api that provides list of all builtin modules in
Node (Jon Moss)
#16386
* net:
- return this from getConnections() (Sam Roberts)
#13553
* promises:
- more robust stringification for unhandled rejections (Timothy Gu)
#13784
* repl:
- improve require() autocompletion (Alexey Orlenko)
#14409
* src:
- add openssl-system-ca-path configure option (Daniel Bevenius)
#16790
- add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
#12087
- add process.ppid (cjihrig)
#16839
* tls:
- accept `lookup` option for `tls.connect()` (Fedor Indutny)
#12839
* tools, build:
- a new macOS installer! (JP Wesselink)
#15179
* url:
- WHATWG URL api support (James M Snell)
#7448
* util:
- add %i and %f formatting specifiers (Roman Reiss)
#10308
PR-URL: #18342
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.
Notable Changes:
* console:
- added console.count() and console.clear() (James M Snell)
#12678
* crypto:
- expose ECDH class (Bryan English)
#8188
- added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
#10209
- warn on invalid authentication tag length (Tobias Nießen)
#17566
* deps:
- upgrade libuv to 1.16.1 (cjihrig)
#16835
* dgram:
- added socket.setMulticastInterface() (Will Young)
#7855
* http:
- add agent.keepSocketAlive and agent.reuseSocket as to allow
overridable keep-alive behavior of `Agent` (Fedor Indutny)
#13005
* lib:
- return this from net.Socket.end() (Sam Roberts)
#13481
* module:
- add builtinModules api that provides list of all builtin modules in
Node (Jon Moss)
#16386
* net:
- return this from getConnections() (Sam Roberts)
#13553
* promises:
- more robust stringification for unhandled rejections (Timothy Gu)
#13784
* repl:
- improve require() autocompletion (Alexey Orlenko)
#14409
* src:
- add openssl-system-ca-path configure option (Daniel Bevenius)
#16790
- add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
#12087
- add process.ppid (cjihrig)
#16839
* tls:
- accept `lookup` option for `tls.connect()` (Fedor Indutny)
#12839
* tools, build:
- a new macOS installer! (JP Wesselink)
#15179
* url:
- WHATWG URL api support (James M Snell)
#7448
* util:
- add %i and %f formatting specifiers (Roman Reiss)
#10308
PR-URL: #18342
This LTS release comes with 112 commits, 17 of which are considered
Semver-Minor. This includes 32 which are doc related, 30 which are test
related, 8 which are build / tool related and 1 commit which updates
a dependency.
Notable Changes:
* console:
- added console.count() and console.clear() (James M Snell)
nodejs#12678
* crypto:
- expose ECDH class (Bryan English)
nodejs#8188
- added cypto.randomFill() and crypto.randomFillSync() (Evan Lucas)
nodejs#10209
- warn on invalid authentication tag length (Tobias Nießen)
nodejs#17566
* deps:
- upgrade libuv to 1.16.1 (cjihrig)
nodejs#16835
* dgram:
- added socket.setMulticastInterface() (Will Young)
nodejs#7855
* http:
- add agent.keepSocketAlive and agent.reuseSocket as to allow
overridable keep-alive behavior of `Agent` (Fedor Indutny)
nodejs#13005
* lib:
- return this from net.Socket.end() (Sam Roberts)
nodejs#13481
* module:
- add builtinModules api that provides list of all builtin modules in
Node (Jon Moss)
nodejs#16386
* net:
- return this from getConnections() (Sam Roberts)
nodejs#13553
* promises:
- more robust stringification for unhandled rejections (Timothy Gu)
nodejs#13784
* repl:
- improve require() autocompletion (Alexey Orlenko)
nodejs#14409
* src:
- add openssl-system-ca-path configure option (Daniel Bevenius)
nodejs#16790
- add --use-bundled-ca --use-openssl-ca check (Daniel Bevenius)
nodejs#12087
- add process.ppid (cjihrig)
nodejs#16839
* tls:
- accept `lookup` option for `tls.connect()` (Fedor Indutny)
nodejs#12839
* tools, build:
- a new macOS installer! (JP Wesselink)
nodejs#15179
* url:
- WHATWG URL api support (James M Snell)
nodejs#7448
* util:
- add %i and %f formatting specifiers (Roman Reiss)
nodejs#10308
PR-URL: nodejs#18342
Introduce two overridable `Agent` methods: * `keepSocketAlive(socket)` * `reuseSocket(socket, req)` These methods can be overridden by particular `Agent` class child to make keep-alive behavior customizable. Motivation: destroy persisted sockets after some configurable timeout. It is very non-trivial to do it with available primitives. Such program will most likely need to poke with undocumented events and methods of `Agent`. With introduced API such behavior is easy to implement. PR-URL: nodejs/node#13005 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Refael Ackermann <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Brian White <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.