http: make globalAgent of http and https as overridable properties
#9386
Conversation
The `http.globalAgent` and `https.globalAgent` previously cannot be
overridden directly by reassigning new a value.
By exposing the `globalAgent`s of `http` and `https`, now it can be
reassigned by:
http.globalAgent = new MyAgent();
https.globalAgent = new MySecureAgent();
nodejs-github-bot
added
http
|
I would include docs in the PR, because the docs will show how the API changes, and let you document the usefulness of it. Also, does this really work? What happens to any requests that are in process and using the old global agent? And to its cached connections? Do they just leak, or do they timeout and go away? I'm also pretty surprised to see that https.request seems to be modifying its What about just making the globalAgent propert read-only, so nobody can try to assign to it by mistake? |
|
Can you add a test? See https://github.com/nodejs/node/blob/master/doc/guides/writing_tests.md and maybe try to find a similar test in |
|
@Trott I suspect that @diorahman deliberately didn't test or doc, because it is not sure people are 👍 or 👎 on the entire idea. Which is reasonable, IMO. I suggested docs because I think its hard to evaluate the reasonableness of API changes that are doc-less. That may just be me. |
| @@ -23,6 +23,7 @@ const client = require('_http_client'); | |||
| const ClientRequest = exports.ClientRequest = client.ClientRequest; | |||
|
|
|||
| exports.request = function request(options, cb) { | |||
| options._defaultAgent = options._defaultAgent || exports.globalAgent || agent.globalAgent; | |||
There was a problem hiding this comment.
Long line. make test would have caught this, it runs make lint.
|
@sam-github @Trott yes, as Sam said (and it is outlined in the #9057) the idea is still unclear. Will try to provide tests and experiment with what suggested by @sam-github in #9386 (comment) |
jasnell
added
the
semver-minor
|
@nodejs/http ... thoughts? @diorahman ... would you mind adding a test? |
|
This seems to have been dropped. Is there any reason to push it forward or can it be closed along with #9057? |
|
I'll go ahead and close it. Can reopen if @diorahman wants to follow through. |
Under ECMAScript modules when you do "import * as https from 'https'" you get a new object with properties copied from https module exports. So if this is a regular data property, you will just override a copy, but if this would be a accessor property, we can still access the actual https.globalAgent. Refs: nodejs#25170, nodejs#9386
Under ECMAScript modules when you do "import * as https from 'https'" you get a new object with properties copied from https module exports. So if this is a regular data property, you will just override a copy, but if this would be a accessor property, we can still access the actual https.globalAgent. Refs: nodejs#25170, nodejs#9386
Under ECMAScript modules when you do "import * as https from 'https'" you get a new object with properties copied from https module exports. So if this is a regular data property, you will just override a copy, but if this would be a accessor property, we can still access the actual https.globalAgent. Refs: nodejs#25170, nodejs#9386
Under ECMAScript modules when you do "import * as https from 'https'" you get a new object with properties copied from https module exports. So if this is a regular data property, you will just override a copy, but if this would be a accessor property, we can still access the actual https.globalAgent. Refs: nodejs#25170, nodejs#9386
Under ECMAScript modules when you do "import * as https from 'https'" you get a new object with properties copied from https module exports. So if this is a regular data property, you will just override a copy, but if this would be a accessor property, we can still access the actual https.globalAgent. Refs: nodejs#25170, nodejs#9386
Under ECMAScript modules when you do "import * as https from 'https'" you get a new object with properties copied from https module exports. So if this is a regular data property, you will just override a copy, but if this would be a accessor property, we can still access the actual https.globalAgent. Refs: nodejs#25170, nodejs#9386
Checklist
make -j8 test(UNIX), orvcbuild test nosign(Windows) passesAffected core subsystem(s)
http, https
Description of change
The
http.globalAgentandhttps.globalAgentpreviously cannot be overridden directly by reassigning new a value.By exposing the
globalAgents ofhttpandhttps, now it can be reassigned by:Fix #9057