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

doc: call out http(s).globalAgent defaults #52392

Merged

Conversation

fahrradflucht
Copy link
Contributor

@fahrradflucht fahrradflucht commented Apr 6, 2024

Despite the http.Agent stating:

The default http.globalAgent that is used by http.request() has all of these values set to their respective defaults.

this isn't true anymore since node.js 19. Both, the http and the https globalAgent now set { keepAlive: true, scheduling: 'lifo', timeout: 5000 } as options. 'lifo' is the default anyway, but keepAlive is turned off and no timeout is set on new Agent().

Document the diverging behavior in the globalAgent sections, remove the false statement from http.Agent section, and extend the changelog to call out the timeout change as well.

Closes #48821
Closes #51115

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/http
  • @nodejs/net

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 6, 2024
@lpinca
Copy link
Member

lpinca commented Apr 7, 2024

Closes: #51115

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

Despite the `http.Agent` stating:

> The default `http.globalAgent` that is used by `http.request()` has
> all of these values set to their respective defaults.

this isn't true anymore since node.js 19. Both, the http as well as the
https `globalAgent` now set `{ keepAlive: true, scheduling: 'lifo',
timeout: 5000 }` as options. `'lifo'` is the default anyway, but
`keepAlive` is turned off and no `timeout` is set on `new Agent()`.

Document the diverging behaviour in the `globalAgent` sections, remove
the false statement from `http.Agent` section, and extend the changelog
to call out the timeout change as well.
@fahrradflucht fahrradflucht force-pushed the docs/http-global-agent-defaults branch from f477d6d to 4a008be Compare April 7, 2024 08:45
@fahrradflucht
Copy link
Contributor Author

Fixed the commit message. 🙄 Never make an edit without gqq. 😉

@fahrradflucht
Copy link
Contributor Author

"lint-js-and-md" failure looks like CI flake to me.

@marco-ippolito marco-ippolito added the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 8, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Apr 8, 2024
@nodejs-github-bot nodejs-github-bot merged commit 655b327 into nodejs:main Apr 8, 2024
16 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in 655b327

marco-ippolito pushed a commit that referenced this pull request May 2, 2024
Despite the `http.Agent` stating:

> The default `http.globalAgent` that is used by `http.request()` has
> all of these values set to their respective defaults.

this isn't true anymore since node.js 19. Both, the http as well as the
https `globalAgent` now set `{ keepAlive: true, scheduling: 'lifo',
timeout: 5000 }` as options. `'lifo'` is the default anyway, but
`keepAlive` is turned off and no `timeout` is set on `new Agent()`.

Document the diverging behaviour in the `globalAgent` sections, remove
the false statement from `http.Agent` section, and extend the changelog
to call out the timeout change as well.

PR-URL: #52392
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
marco-ippolito pushed a commit that referenced this pull request May 3, 2024
Despite the `http.Agent` stating:

> The default `http.globalAgent` that is used by `http.request()` has
> all of these values set to their respective defaults.

this isn't true anymore since node.js 19. Both, the http as well as the
https `globalAgent` now set `{ keepAlive: true, scheduling: 'lifo',
timeout: 5000 }` as options. `'lifo'` is the default anyway, but
`keepAlive` is turned off and no `timeout` is set on `new Agent()`.

Document the diverging behaviour in the `globalAgent` sections, remove
the false statement from `http.Agent` section, and extend the changelog
to call out the timeout change as well.

PR-URL: #52392
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Marco Ippolito <marcoippolito54@gmail.com>
Reviewed-By: Ulises Gascón <ulisesgascongonzalez@gmail.com>
Reviewed-By: Moshe Atlow <moshe@atlow.co.il>
@fahrradflucht fahrradflucht deleted the docs/http-global-agent-defaults branch June 6, 2024 08:43
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Docs imply that http globalAgent is created with keepAlive: false
8 participants