Skip to content

Removed clients with unrecoverable errors from the Pool#4088

Merged
mcollina merged 5 commits intomainfrom
fix-3895
Mar 12, 2025
Merged

Removed clients with unrecoverable errors from the Pool#4088
mcollina merged 5 commits intomainfrom
fix-3895

Conversation

@mcollina
Copy link
Member

Fixes #3895

Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina requested a review from ronag March 11, 2025 23:43
for (const target of targets) {
// Do not use kRemoveClient here, as it will close the client,
// but the client cannot be closed in this state.
this[kClients].splice(this[kClients].indexOf(target), 1)
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible for a target not to be in the list?

Copy link
Member Author

Choose a reason for hiding this comment

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

no, not really, but better be safe.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
Comment on lines +71 to +75
for (const target of targets) {
// Do not use kRemoveClient here, as it will close the client,
// but the client cannot be closed in this state.
this[kClients].splice(this[kClients].indexOf(target), 1)
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
for (const target of targets) {
// Do not use kRemoveClient here, as it will close the client,
// but the client cannot be closed in this state.
this[kClients].splice(this[kClients].indexOf(target), 1)
}
this[kClients] = this[kClients].filter(target => !targets.includes(target))

Here's an alternative implementation that can handle the case when target isn't in the array.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks! I handled it without a filter.

…rrectly Signed-off-by: Matteo Collina <hello@matteocollina.com>
Signed-off-by: Matteo Collina <hello@matteocollina.com>
@mcollina mcollina marked this pull request as ready for review March 12, 2025 08:33
Copy link
Member

@styfle styfle left a comment

Choose a reason for hiding this comment

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

Great work, thanks! 🎉

I confirmed this fixes the memory leak on my repo:

https://github.com/uncurated-tests/fetch-memory-leak/tree/err-tls-cert-altname-invalid

@mcollina mcollina merged commit f317618 into main Mar 12, 2025
34 of 37 checks passed
This was referenced Mar 12, 2025
@github-actions github-actions bot mentioned this pull request Mar 12, 2025
mcollina added a commit that referenced this pull request Mar 12, 2025
Signed-off-by: Matteo Collina <hello@matteocollina.com>
chance-coleman pushed a commit to defenseunicorns/uds-core that referenced this pull request Mar 13, 2025
This PR contains the following updates: | Package | Type | Update | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---|---|---| | aws | required_provider | minor | `~> 5.90.0` -> `~> 5.91.0` | [![age](https://developer.mend.io/api/mc/badges/age/terraform-provider/hashicorp%2faws/5.91.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/terraform-provider/hashicorp%2faws/5.91.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/terraform-provider/hashicorp%2faws/5.90.1/5.91.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/terraform-provider/hashicorp%2faws/5.90.1/5.91.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [kubernetes-fluent-client](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client) | devDependencies | patch | [`3.4.2` -> `3.4.5`](https://renovatebot.com/diffs/npm/kubernetes-fluent-client/3.4.2/3.4.5) | [![age](https://developer.mend.io/api/mc/badges/age/npm/kubernetes-fluent-client/3.4.5?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/kubernetes-fluent-client/3.4.5?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/kubernetes-fluent-client/3.4.2/3.4.5?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/kubernetes-fluent-client/3.4.2/3.4.5?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>defenseunicorns/kubernetes-fluent-client (kubernetes-fluent-client)</summary> ### [`v3.4.5`](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/releases/tag/v3.4.5) [Compare Source](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/compare/v3.4.4...v3.4.5) ##### Bug Fixes - undici bump with new features ([#&#8203;586](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/586)) ([38d9159](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/commit/38d9159bf34029ac4e2cff238857b4609a454281)), closes [nodejs/undici#4044](https://redirect.github.com/nodejs/undici/issues/4044) [nodejs/undici#4029](https://redirect.github.com/nodejs/undici/issues/4029) [nodejs/undici#4084](https://redirect.github.com/nodejs/undici/issues/4084) [nodejs/undici#4027](https://redirect.github.com/nodejs/undici/issues/4027) [nodejs/undici#4070](https://redirect.github.com/nodejs/undici/issues/4070) [nodejs/undici#4088](https://redirect.github.com/nodejs/undici/issues/4088) [nodejs/undici#4029](https://redirect.github.com/nodejs/undici/issues/4029) [nodejs/undici#4084](https://redirect.github.com/nodejs/undici/issues/4084) [nodejs/undici#4070](https://redirect.github.com/nodejs/undici/issues/4070) [#&#8203;4091](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4091) [#&#8203;4088](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4088) [#&#8203;4070](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4070) [#&#8203;4027](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4027) [#&#8203;4084](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4084) [#&#8203;4029](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4029) [#&#8203;4044](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4044) ### [`v3.4.4`](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/releases/tag/v3.4.4) [Compare Source](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/compare/v3.4.2...v3.4.4) ##### Bug Fixes - update README.md ([#&#8203;585](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/585)) ([a16f08e](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/commit/a16f08e0f2b08d9b465ee01576d4ef55380d034e)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://redirect.github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/defenseunicorns/uds-core). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xOTQuMSIsInVwZGF0ZWRJblZlciI6IjM5LjIwMC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
ronag added a commit to nxtedition/undici that referenced this pull request Mar 14, 2025
* nodejs/main: (23 commits) Bumped v7.5.0 (nodejs#4091) Removed clients with unrecoverable errors from the Pool (nodejs#4088) feat: Allow disabling autoSelectFamily in an Agent (nodejs#4070) chore: update cache tests (nodejs#4027) fix: Fix retry-handler.js when retry-after header is a Date (nodejs#4084) feat: add mock call history to access request configuration in test (nodejs#4029) feat(docs): button to switch dark and light mode (nodejs#4044) Bumped v7.4.0 (nodejs#4071) fix: fix EnvHttpProxyAgent for the Node.js bundle (nodejs#4064) chore: update WPT (nodejs#4062) chore: update WPT (nodejs#4028) fix: handle missing vary header values (nodejs#4031) fix: do not throw unhandled exception when data is undefined in interceptor.reply (nodejs#4036) test: fix windows wpt (nodejs#4050) feat: mark `EnvHttpProxyAgent` as stable (nodejs#4049) don't check AbortSignal maxListeners on some node versions (nodejs#4045) feat(docs): copy to clipboard button (nodejs#4037) docs: fix incorrect method signature of `onResponseError` (nodejs#4030) docs: document about global dispatcher and errors (nodejs#3987) (nodejs#4014) chore: update WPT (nodejs#4011) ...
@Uzlopak Uzlopak deleted the fix-3895 branch June 5, 2025 08:22
@kapouer
Copy link

kapouer commented Oct 8, 2025

Is it possible that the measurements done in https://github.com/nodejs/undici/blob/main/test/tls-cert-leak.js fail on i386 ? Because it seems to be flaky on debian CI, but only for i386.

@metcoder95
Copy link
Member

Feel free to open an issue about it

mjnagel pushed a commit to BagelLab/uds-core that referenced this pull request Nov 14, 2025
This PR contains the following updates: | Package | Type | Update | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---|---|---| | aws | required_provider | minor | `~> 5.90.0` -> `~> 5.91.0` | [![age](https://developer.mend.io/api/mc/badges/age/terraform-provider/hashicorp%2faws/5.91.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/terraform-provider/hashicorp%2faws/5.91.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/terraform-provider/hashicorp%2faws/5.90.1/5.91.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/terraform-provider/hashicorp%2faws/5.90.1/5.91.0?slim=true)](https://docs.renovatebot.com/merge-confidence/) | | [kubernetes-fluent-client](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client) | devDependencies | patch | [`3.4.2` -> `3.4.5`](https://renovatebot.com/diffs/npm/kubernetes-fluent-client/3.4.2/3.4.5) | [![age](https://developer.mend.io/api/mc/badges/age/npm/kubernetes-fluent-client/3.4.5?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/kubernetes-fluent-client/3.4.5?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/kubernetes-fluent-client/3.4.2/3.4.5?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/kubernetes-fluent-client/3.4.2/3.4.5?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- ### Release Notes <details> <summary>defenseunicorns/kubernetes-fluent-client (kubernetes-fluent-client)</summary> ### [`v3.4.5`](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/releases/tag/v3.4.5) [Compare Source](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/compare/v3.4.4...v3.4.5) ##### Bug Fixes - undici bump with new features ([#&#8203;586](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/586)) ([38d9159](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/commit/38d9159bf34029ac4e2cff238857b4609a454281)), closes [nodejs/undici#4044](https://redirect.github.com/nodejs/undici/issues/4044) [nodejs/undici#4029](https://redirect.github.com/nodejs/undici/issues/4029) [nodejs/undici#4084](https://redirect.github.com/nodejs/undici/issues/4084) [nodejs/undici#4027](https://redirect.github.com/nodejs/undici/issues/4027) [nodejs/undici#4070](https://redirect.github.com/nodejs/undici/issues/4070) [nodejs/undici#4088](https://redirect.github.com/nodejs/undici/issues/4088) [nodejs/undici#4029](https://redirect.github.com/nodejs/undici/issues/4029) [nodejs/undici#4084](https://redirect.github.com/nodejs/undici/issues/4084) [nodejs/undici#4070](https://redirect.github.com/nodejs/undici/issues/4070) [#&#8203;4091](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4091) [#&#8203;4088](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4088) [#&#8203;4070](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4070) [#&#8203;4027](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4027) [#&#8203;4084](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4084) [#&#8203;4029](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4029) [#&#8203;4044](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4044) ### [`v3.4.4`](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/releases/tag/v3.4.4) [Compare Source](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/compare/v3.4.2...v3.4.4) ##### Bug Fixes - update README.md ([#&#8203;585](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/585)) ([a16f08e](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/commit/a16f08e0f2b08d9b465ee01576d4ef55380d034e)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://redirect.github.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/defenseunicorns/uds-core). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOS4xOTQuMSIsInVwZGF0ZWRJblZlciI6IjM5LjIwMC4yIiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119--> Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

7 participants