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 — lib: support overriding http\s.globalAgent by illBeRoy · Pull Request #25170 · nodejs/node Conversation
Contributor
Overriding In order to achieve that, the following changes were made:
Fixes: #23281 On a side note, this is my first contribution to the project. Would appreciate feedback :) Checklist
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page. All reactions
Overriding `require('http[s]').globalAgent` is now respected by
consequent requests.
In order to achieve that, the following changes were made:
1. Implmentation in `http`: `module.exports.globalAgent` is now defined
through `Object.defineProperty`. Its getter and setter return \ set
`require('_http_agent').globalAgent`.
2. Implementation in `https`: the https `globalAgent` is not the same
as `_http_agent`, and is defined in `https` module itself. Therefore,
the fix here was to simply use `module.exports.globalAgent` to support
mutation.
3. According tests were added for both `http` and `https`, where in
both we create a server, set the default agent to a newly created
instance and make a request to that server. We then assert that the
given instance was actually used by inspecting its sockets property.
Fixes: nodejs#23281
illBeRoy
force-pushed
the
fix/overridable-http-https-global-agent
branch
from
23a222e to
633e0fe
Compare
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
Member
All reactions
Sorry, something went wrong. Uh oh!
There was an error while loading. Please reload this page.
|
There was an error while loading. Please reload this page.
|
Hey @Trott, I believe that the way it worked up until now was a bug, because one could expect reassigning to That's why I don't think a change to the docs is required - as far as I can see, they simply state that the Then again - if you deem it required, I'll be more than happy update my PR :) |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Resume Build CI: https://ci.nodejs.org/job/node-test-pull-request/19764/ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
Resume Build CI again: https://ci.nodejs.org/job/node-test-pull-request/19768/ ✔️ |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
@nodejs/http |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
|
Hey again :) I'm not quite sure about the process, but since it's approved, is there anything else for me to do? how do I know if it was merged? Thanks! |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
|
@illBeRoy Pinging us is the exact right thing to do. :) Landed in 5774688, and congratulations to your first contribution! 🎉 |
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Overriding `require('http[s]').globalAgent` is now respected by
consequent requests.
In order to achieve that, the following changes were made:
1. Implmentation in `http`: `module.exports.globalAgent` is now defined
through `Object.defineProperty`. Its getter and setter return \ set
`require('_http_agent').globalAgent`.
2. Implementation in `https`: the https `globalAgent` is not the same
as `_http_agent`, and is defined in `https` module itself. Therefore,
the fix here was to simply use `module.exports.globalAgent` to support
mutation.
3. According tests were added for both `http` and `https`, where in
both we create a server, set the default agent to a newly created
instance and make a request to that server. We then assert that the
given instance was actually used by inspecting its sockets property.
Fixes: #23281
PR-URL: #25170
Reviewed-By: James M Snell <[email protected]>
Overriding `require('http[s]').globalAgent` is now respected by
consequent requests.
In order to achieve that, the following changes were made:
1. Implmentation in `http`: `module.exports.globalAgent` is now defined
through `Object.defineProperty`. Its getter and setter return \ set
`require('_http_agent').globalAgent`.
2. Implementation in `https`: the https `globalAgent` is not the same
as `_http_agent`, and is defined in `https` module itself. Therefore,
the fix here was to simply use `module.exports.globalAgent` to support
mutation.
3. According tests were added for both `http` and `https`, where in
both we create a server, set the default agent to a newly created
instance and make a request to that server. We then assert that the
given instance was actually used by inspecting its sockets property.
Fixes: #23281
PR-URL: #25170
Reviewed-By: James M Snell <[email protected]>
Overriding `require('http[s]').globalAgent` is now respected by
consequent requests.
In order to achieve that, the following changes were made:
1. Implmentation in `http`: `module.exports.globalAgent` is now defined
through `Object.defineProperty`. Its getter and setter return \ set
`require('_http_agent').globalAgent`.
2. Implementation in `https`: the https `globalAgent` is not the same
as `_http_agent`, and is defined in `https` module itself. Therefore,
the fix here was to simply use `module.exports.globalAgent` to support
mutation.
3. According tests were added for both `http` and `https`, where in
both we create a server, set the default agent to a newly created
instance and make a request to that server. We then assert that the
given instance was actually used by inspecting its sockets property.
Fixes: nodejs#23281
PR-URL: nodejs#25170
Reviewed-By: James M Snell <[email protected]>
Overriding `require('http[s]').globalAgent` is now respected by
consequent requests.
In order to achieve that, the following changes were made:
1. Implmentation in `http`: `module.exports.globalAgent` is now defined
through `Object.defineProperty`. Its getter and setter return \ set
`require('_http_agent').globalAgent`.
2. Implementation in `https`: the https `globalAgent` is not the same
as `_http_agent`, and is defined in `https` module itself. Therefore,
the fix here was to simply use `module.exports.globalAgent` to support
mutation.
3. According tests were added for both `http` and `https`, where in
both we create a server, set the default agent to a newly created
instance and make a request to that server. We then assert that the
given instance was actually used by inspecting its sockets property.
Fixes: nodejs#23281
PR-URL: nodejs#25170
Reviewed-By: James M Snell <[email protected]>
Notable Changes
* compression / zlib:
* Added brotli support (Anna Henningsen and Zach Vacura)
nodejs#24938
* console:
* Added `inspectOptions` option (Ruben Bridgewater)
nodejs#24978
* crypto:
* Always accept private keys as public keys (Tobias Nießen)
nodejs#25217
* deps:
* Upgrade npm to v6.5.0 (Jordan Harband)
nodejs#25234
* fs:
* Use internalBinding('fs') internally instead of
process.binding('fs') (Masashi Hirano)
nodejs#22478
* http(s):
* Support overriding http\\s.globalAgent (Roy Sommer)
nodejs#25170
* util:
* Inspect ArrayBuffers contents closely (Ruben Bridgewater)
nodejs#25006
* worker:
* Expose workers by default and remove `--experimental-worker` flag
(Anna Henningsen) nodejs#25361
PR-URL: nodejs#25537
Notable Changes
* compression / zlib:
* Added brotli support (Anna Henningsen and Zach Vacura)
#24938
* console:
* Added `inspectOptions` option (Ruben Bridgewater)
#24978
* crypto:
* Always accept private keys as public keys (Tobias Nießen)
#25217
* deps:
* Upgrade npm to v6.5.0 (Jordan Harband)
#25234
* fs:
* Use internalBinding('fs') internally instead of
process.binding('fs') (Masashi Hirano)
#22478
* http(s):
* Support overriding http\\s.globalAgent (Roy Sommer)
#25170
* util:
* Inspect ArrayBuffers contents closely (Ruben Bridgewater)
#25006
* worker:
* Expose workers by default and remove `--experimental-worker` flag
(Anna Henningsen) #25361
PR-URL: #25537
Overriding `require('http[s]').globalAgent` is now respected by
consequent requests.
In order to achieve that, the following changes were made:
1. Implmentation in `http`: `module.exports.globalAgent` is now defined
through `Object.defineProperty`. Its getter and setter return \ set
`require('_http_agent').globalAgent`.
2. Implementation in `https`: the https `globalAgent` is not the same
as `_http_agent`, and is defined in `https` module itself. Therefore,
the fix here was to simply use `module.exports.globalAgent` to support
mutation.
3. According tests were added for both `http` and `https`, where in
both we create a server, set the default agent to a newly created
instance and make a request to that server. We then assert that the
given instance was actually used by inspecting its sockets property.
Fixes: #23281
PR-URL: #25170
Reviewed-By: James M Snell <[email protected]>
Overriding `require('http[s]').globalAgent` is now respected by
consequent requests.
In order to achieve that, the following changes were made:
1. Implmentation in `http`: `module.exports.globalAgent` is now defined
through `Object.defineProperty`. Its getter and setter return \ set
`require('_http_agent').globalAgent`.
2. Implementation in `https`: the https `globalAgent` is not the same
as `_http_agent`, and is defined in `https` module itself. Therefore,
the fix here was to simply use `module.exports.globalAgent` to support
mutation.
3. According tests were added for both `http` and `https`, where in
both we create a server, set the default agent to a newly created
instance and make a request to that server. We then assert that the
given instance was actually used by inspecting its sockets property.
Fixes: #23281
PR-URL: #25170
Reviewed-By: James M Snell <[email protected]>
Overriding `require('http[s]').globalAgent` is now respected by
consequent requests.
In order to achieve that, the following changes were made:
1. Implmentation in `http`: `module.exports.globalAgent` is now defined
through `Object.defineProperty`. Its getter and setter return \ set
`require('_http_agent').globalAgent`.
2. Implementation in `https`: the https `globalAgent` is not the same
as `_http_agent`, and is defined in `https` module itself. Therefore,
the fix here was to simply use `module.exports.globalAgent` to support
mutation.
3. According tests were added for both `http` and `https`, where in
both we create a server, set the default agent to a newly created
instance and make a request to that server. We then assert that the
given instance was actually used by inspecting its sockets property.
Fixes: #23281
PR-URL: #25170
Reviewed-By: James M Snell <[email protected]>
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
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
Can we restart the work on overridable globalAgent?
There was an error while loading. Please reload this page.