-
-
Notifications
You must be signed in to change notification settings - Fork 675
Removed clients with unrecoverable errors from the Pool #4088
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
Conversation
lib/dispatcher/pool-base.js
Outdated
| 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) |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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>
lib/dispatcher/pool-base.js
Outdated
| 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) | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| 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.
There was a problem hiding this comment.
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>
styfle
left a comment
There was a problem hiding this 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
Signed-off-by: Matteo Collina <hello@matteocollina.com>
This PR contains the following updates: | Package | Type | Update | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---|---|---| | aws | required_provider | minor | `~> 5.90.0` -> `~> 5.91.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](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) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](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 ([#​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) [#​4091](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4091) [#​4088](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4088) [#​4070](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4070) [#​4027](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4027) [#​4084](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4084) [#​4029](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4029) [#​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 ([#​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>
* 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) ...
|
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. |
|
Feel free to open an issue about it |
This PR contains the following updates: | Package | Type | Update | Change | Age | Adoption | Passing | Confidence | |---|---|---|---|---|---|---|---| | aws | required_provider | minor | `~> 5.90.0` -> `~> 5.91.0` | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](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) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](https://docs.renovatebot.com/merge-confidence/) | [](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 ([#​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) [#​4091](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4091) [#​4088](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4088) [#​4070](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4070) [#​4027](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4027) [#​4084](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4084) [#​4029](https://redirect.github.com/defenseunicorns/kubernetes-fluent-client/issues/4029) [#​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 ([#​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>
Fixes #3895