fix: Scope cached http(s) agents more strongly by davidkna-sap · Pull Request #6716 · SAP/cloud-sdk-js

davidkna-sap · 2026-06-29T09:17:03Z

Closes https://github.com/SAP/ai-sdk-js-backlog/issues/604.

InjunPark-sap · 2026-07-08T12:47:09Z

 ): HttpAgentConfig | HttpsAgentConfig {
-  const protocol = getProtocolOrDefault(destination);
-  const cacheKey = hashCacheKey({ protocol, options });
+  const cacheKey = hashCacheKey({ destination, options });


[Nit] destination.certificates and options already overlap — the same cert material ends up in the hash twice. Is there any functional impact on this change?

Including the full destination busts the cache on every token refresh for OAuth2ClientCredentials/OAuth2SAMLBearerFlow destinations — `authTokens.value changes hourly but is never read by createAgent, so keep-alive reuse breaks entirely for those destinations.

I would like to suggest scoping the key to only what affects agent behavior:

const cacheKey = hashCacheKey({ protocol, options, proxyConfiguration: destination.proxyConfiguration, cloudConnectorLocationId: destination.cloudConnectorLocationId });

I think a 1h cache is acceptable for this use-case. In addition to cloudConnectorLocationId for the proxy the agent also needs to be keyed per-user.

InjunPark-sap

nit question and suggest changes on http-agent.ts:309

InjunPark-sap

Two open questions:

Does authTokens actually appear in the destination object passed to getAgentConfig in OAuth2 flows? If yes, every token refresh produces a new cache key and keep-alive reuse is lost.
Is proxyConfiguration sufficient for per-user keying, or are there cases where the user identity affects agent behavior outside of proxyConfiguration.headers?

davidkna-sap · 2026-07-08T14:21:36Z

@InjunPark-sap

I would say that is acceptable.
The user keying appears to work for the specific support cases, but in general other destination types appear to be less picky.

InjunPark-sap

Approval with the understanding that OAuth2 destinations will see reduced agent reuse on token refresh. (Worth revisiting if it shows up as a connection overhead issue in production.)

marikaner · 2026-07-16T09:09:23Z

 ): Promise<HttpAgentConfig | HttpsAgentConfig> {
-  const protocol = getProtocolOrDefault(destination);
-  const cacheKey = await hashCacheKey({ protocol, options });
+  const cacheKey = await hashCacheKey({ destination, options });


[req] I don't think it is a good idea to hash the whole destination. E.g. auth tokens would be part of the key. Consider using specific values from the destination, like name and auth type. Also, this will expire, right? So if the destination changes it will eventually be re-created?

What do you think about special-casing for connectivity only? So far only for that destination there are additional requirements.

Regarding expiring the http agents should also eventually be evicted from the LRU cache.

I did choose the whole destination because connectivity service seems to be very sensitive to connection re-use: they need to be different both per-user and per-connectivity-location. Including the destination token allowed satisfying the per-user requirement. I will re-evaluate the options, and whether to just default to keepAlive: false for the connectivity proxy.

What do you think about special-casing for connectivity only? So far only for that destination there are additional requirements.

That might make it more complex.

I did choose the whole destination because connectivity service seems to be very sensitive to connection re-use: they need to be different both per-user and per-connectivity-location. Including the destination token allowed satisfying the per-user requirement. I will re-evaluate the options, and whether to just default to keepAlive: false for the connectivity proxy.

Would it work to just include user and connectivity information (without secrets) for the key?

I don't have an E2E test setup, but as currently implemented for connectivity this PR extracts user/tenant information and the location Id into the identifier.

Currently it has to be connectivity specific (SAP-Connectivity-Authentication), but I could also try to extend this to extract the data from other JWT locations.

…http-agent-cache * origin/main: (33 commits) fix(openapi): Return `Blob` instead of `Buffer` (matching external types) (#6663) chore(deps): bump fast-xml-parser from 5.9.3 to 5.10.0 (#6780) chore: merge dependabot dev and prod config (#6779) chore: fix dependabot config (#6778) chore: add dev dependency config (#6777) chore(deps-dev): bump turbo from 2.10.0 to 2.10.4 (#6758) chore(deps): bump typescript-eslint from 8.62.1 to 8.63.0 (#6774) chore(deps-dev): bump @sap/cds-dk from 10.0.3 to 10.0.4 (#6772) chore(deps): bump memfs from 4.57.8 to 4.64.0 (#6769) chore(deps): bump @typescript-eslint/parser from 8.62.1 to 8.63.0 (#6776) chore(deps-dev): bump @eslint/eslintrc from 3.3.5 to 3.3.6 (#6773) chore(deps): bump prettier from 3.9.4 to 3.9.5 (#6771) chore(deps-dev): bump eslint from 10.6.0 to 10.7.0 (#6768) chore(deps): bump content-type and @types/content-type (#6608) feat(deps): bump eslint-plugin-jsdoc from 63.0.10 to 63.0.13 (#6751) feat(deps-dev): bump typedoc from 0.28.19 to 0.28.20 (#6752) feat(deps-dev): bump @types/node from 22.20.0 to 22.20.1 (#6756) feat(deps): bump the codeql group across 1 directory with 3 updates (#6763) chore: Remove devEngines constraint again (#6760) chore: Only allow single pnpm version for dependabot (corepack) (#6761) ...

davidkna-sap and others added 2 commits June 29, 2026 11:16

fix: Scope cached http(s) agents per-destination

48a3144

Changes from lint:fix

1428f52

davidkna-sap marked this pull request as ready for review June 29, 2026 09:29

davidkna-sap requested review from KavithaSiva and marikaner June 29, 2026 09:29

This was referenced Jun 29, 2026

SAP Cloud SDK 4.7.0 reuses HTTP connection across OnPremise destinations with different Cloud Connector Location IDs #6705

Closed

Connectivity issues to onPremise systems via cloud connector after upgrade to 4.7.0 #6715

Open

InjunPark-sap reviewed Jul 8, 2026

View reviewed changes

davidkna-sap requested a review from InjunPark-sap July 8, 2026 13:57

InjunPark-sap reviewed Jul 8, 2026

View reviewed changes

InjunPark-sap previously approved these changes Jul 8, 2026

View reviewed changes

Merge branch 'main' into davidkna-sap_sap-key-http-agent-cache

cc6ceee

davidkna-sap dismissed InjunPark-sap’s stale review via cc6ceee July 8, 2026 14:31

marikaner requested changes Jul 16, 2026

View reviewed changes

davidkna-sap changed the title ~~fix: Scope cached http(s) agents per-destination~~ fix: Scope cached http(s) agents more strongly Jul 16, 2026

davidkna-sap requested a review from marikaner July 16, 2026 12:40

davidkna-sap force-pushed the davidkna-sap_sap-key-http-agent-cache branch from a29f4ef to 41b51e9 Compare July 16, 2026 12:51

Changes from lint:fix

799cbcd

Uh oh!

Conversation

davidkna-sap commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

InjunPark-sap left a comment

Choose a reason for hiding this comment

Uh oh!

InjunPark-sap left a comment

Choose a reason for hiding this comment

Uh oh!

davidkna-sap commented Jul 8, 2026

Uh oh!

InjunPark-sap left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

davidkna-sap commented Jun 29, 2026 •

edited

Loading