feat: Improve handling of HTTP/HTTPS agent sessions

.changeset/heavy-foxes-deny.md

@@ -0,0 +1,6 @@
+---
+'@sap-cloud-sdk/connectivity': minor
+'@sap-cloud-sdk/http-client': minor
+---
+
+[Improvement] Cache custom http and https agents and enable the keep-alive option by default.

.changeset/swift-candies-bake.md

@@ -0,0 +1,6 @@
+---
+'@sap-cloud-sdk/connectivity': minor
+'@sap-cloud-sdk/http-client': minor
+---
+
+[Improvement] Use node's global http/https agent unless a custom agent is required by the destination configuration.

packages/connectivity/src/http-agent/http-agent.spec.ts

@@ -9,7 +9,7 @@ jest.mock('jks-js', () => ({
 import * as jks from 'jks-js';
 import { registerDestinationCache } from '../scp-cf/destination/register-destination-cache';
 import { certAsString } from '../../../../test-resources/test/test-util/test-certificate';
-import { getAgentConfig } from './http-agent';
+import { agentCreateCache, getAgentConfig } from './http-agent';
 import type { HttpDestination } from '../scp-cf/destination';
 import type { DestinationCertificate } from '../scp-cf';
 
@@ -257,6 +257,53 @@ describe('createAgent', () => {
   // Check coverage
 });
 
+describe('agent caching', () => {
+  beforeEach(() => {
+    agentCreateCache.clear();
+  });
+
+  it('returns the same agent instance for the same destination options', async () => {
+    const destination: HttpDestination = { url: 'https://example.com' };
+    const first = (await getAgentConfig(destination)) as any;
+    const second = (await getAgentConfig(destination)) as any;
+    expect(first['httpsAgent']).toBe(second['httpsAgent']);
+  });
+
+  it('returns different agent instances for different destinations', async () => {
+    const destA: HttpDestination = { url: 'https://a.example.com' };
+    const destB: HttpDestination = {
+      url: 'https://b.example.com',
+      isTrustingAllCertificates: true
+    };
+    const agentA = ((await getAgentConfig(destA)) as any)['httpsAgent'];
+    const agentB = ((await getAgentConfig(destB)) as any)['httpsAgent'];
+    expect(agentA).not.toBe(agentB);
+  });
+
+  it('enables keepAlive by default on created agents', async () => {
+    const destination: HttpDestination = { url: 'https://example.com' };
+    const agent = ((await getAgentConfig(destination)) as any)['httpsAgent'];
+    expect(agent.keepAlive).toBe(true);
+  });
+
+  it('keepAlive: false in options overrides the default (spread order)', () => {
+    // createAgent uses { keepAlive: true, ...options }
+    // verify that explicit keepAlive: false in options wins
+    const merged = { keepAlive: true, ...({ keepAlive: false } as any) };
+    expect(merged.keepAlive).toBe(false);
+  });
+
+  it('http and https destinations use separate cache entries and agent types', async () => {
+    const destHttps: HttpDestination = { url: 'https://example.com' };
+    const destHttp: HttpDestination = { url: 'http://example.com' };
+    const httpsResult = (await getAgentConfig(destHttps)) as any;
+    const httpResult = (await getAgentConfig(destHttp)) as any;
+    expect(httpsResult['httpsAgent']).toBeDefined();
+    expect(httpResult['httpAgent']).toBeDefined();
+    expect(httpsResult['httpsAgent']).not.toBe(httpResult['httpAgent']);
+  });
+});
+
 describe('getAgentConfig', () => {
   it('returns an object with key "httpsAgent" for destinations with protocol HTTPS', async () => {
     const destination: HttpDestination = {

packages/connectivity/src/http-agent/http-agent.ts

@@ -1,11 +1,13 @@
-import { readFile } from 'fs/promises';
-import http from 'http';
-import https from 'https';
+import { readFile } from 'node:fs/promises';
+import http from 'node:http';
+import https from 'node:https';
 import * as jks from 'jks-js';
 import { createLogger, last } from '@sap-cloud-sdk/util';
 /* Careful the proxy imports cause circular dependencies if imported from scp directly */
 // eslint-disable-next-line import/no-internal-modules
 import { getProtocolOrDefault } from '../scp-cf/get-protocol';
+// eslint-disable-next-line import/no-internal-modules
+import { Cache, hashCacheKey } from '../scp-cf/cache';
 import {
   addProxyConfigurationInternet,
   getProxyConfig,
@@ -15,11 +17,13 @@ import {
 // eslint-disable-next-line import/no-internal-modules
 import { registerDestinationCache } from '../scp-cf/destination/register-destination-cache';
 import type {
-  BasicProxyConfiguration,
   Destination,
   DestinationCertificate,
   HttpDestination
-} from '../scp-cf';
+  // eslint-disable-next-line import/no-internal-modules
+} from '../scp-cf/destination';
+// eslint-disable-next-line import/no-internal-modules
+import type { BasicProxyConfiguration } from '../scp-cf/connectivity-service-types';
 import type { HttpAgentConfig, HttpsAgentConfig } from './agent-config';
 
 const logger = createLogger({
@@ -283,17 +287,40 @@ function validateFormat(certificate: DestinationCertificate) {
   }
 }
 
+/**
+ * Cache for http(s) agents.
+ * Exported for testing purposes only.
+ * @internal
+ */
+export const agentCreateCache = new Cache<HttpAgentConfig | HttpsAgentConfig>(
+  3600000, // 1 hour
+  100 // max 100 LRU-cached agents
+);
+
 /**
  * @internal
+ * Agents are cache for up to one hour, but can be evicted earlier if more than 100 agents are created.
  * See https://nodejs.org/api/https.html#https_https_createserver_options_requestlistener for details on the possible options
  */
 function createAgent(
   destination: HttpDestination,
   options: https.AgentOptions
 ): HttpAgentConfig | HttpsAgentConfig {
-  return getProtocolOrDefault(destination) === 'https'
-    ? { httpsAgent: new https.Agent(options) }
-    : { httpAgent: new http.Agent(options) };
+  const protocol = getProtocolOrDefault(destination);
+  const cacheKey = hashCacheKey({ protocol, options });
+
+  return agentCreateCache.getOrInsertComputed(cacheKey, () => {
+    logger.debug(
+      `Creating new ${protocol.toUpperCase()} agent for destination ${destination.name || '<unknown>'}`
+    );
+    const optionsWithDefaults = { keepAlive: true, ...options };
+    const entry =
+      protocol === 'https'
+        ? { httpsAgent: new https.Agent(optionsWithDefaults) }
+        : { httpAgent: new http.Agent(optionsWithDefaults) };
+
+    return { entry };
+  });
 }
 
 /**

packages/connectivity/src/scp-cf/cache.spec.ts

@@ -97,4 +97,90 @@ describe('Cache', () => {
     const actual = cacheOne.get(undefined);
     expect(actual).toBeUndefined();
   });
+
+  it('item should be retrievable multiple times (non-LRU cache)', () => {
+    cacheOne.set('multi', { entry: destinationOne });
+    expect(cacheOne.get('multi')).toEqual(destinationOne);
+    expect(cacheOne.get('multi')).toEqual(destinationOne);
+  });
+
+  describe('LRU eviction (maxSize)', () => {
+    it('evicts the least recently used entry when maxSize is exceeded', () => {
+      const lruCache = new Cache<string>(0, 2);
+      lruCache.set('a', { entry: 'A' });
+      lruCache.set('b', { entry: 'B' });
+      lruCache.set('c', { entry: 'C' }); // should evict 'a'
+      expect(lruCache.get('a')).toBeUndefined();
+      expect(lruCache.get('b')).toEqual('B');
+      expect(lruCache.get('c')).toEqual('C');
+    });
+
+    it('updates LRU order on get, so recently accessed entry is not evicted', () => {
+      const lruCache = new Cache<string>(0, 2);
+      lruCache.set('a', { entry: 'A' });
+      lruCache.set('b', { entry: 'B' });
+      lruCache.get('a'); // 'a' is now most recently used; 'b' becomes LRU
+      lruCache.set('c', { entry: 'C' }); // should evict 'b'
+      expect(lruCache.get('b')).toBeUndefined();
+      expect(lruCache.get('a')).toEqual('A');
+      expect(lruCache.get('c')).toEqual('C');
+    });
+
+    it('repeated get keeps the accessed key as most recently used', () => {
+      const lruCache = new Cache<string>(0, 2);
+      lruCache.set('a', { entry: 'A' });
+      lruCache.set('b', { entry: 'B' });
+
+      lruCache.get('a');
+      lruCache.get('a');
+      lruCache.set('c', { entry: 'C' });
+
+      expect(lruCache.get('a')).toEqual('A');
+      expect(lruCache.get('b')).toBeUndefined();
+      expect(lruCache.get('c')).toEqual('C');
+    });
+
+    it('does not evict another entry when updating an existing key', () => {
+      const lruCache = new Cache<string>(0, 2);
+      lruCache.set('a', { entry: 'A' });
+      lruCache.set('b', { entry: 'B' });
+
+      lruCache.set('b', { entry: 'B updated' });
+
+      expect(lruCache.get('a')).toEqual('A');
+      expect(lruCache.get('b')).toEqual('B updated');
+    });
+  });
+
+  describe('getOrInsertComputed', () => {
+    it('calls computeFn on cache miss and stores result', () => {
+      const compute = jest.fn().mockReturnValue({ entry: destinationOne });
+      const result = cacheOne.getOrInsertComputed('new', compute);
+      expect(result).toEqual(destinationOne);
+      expect(compute).toHaveBeenCalledTimes(1);
+    });
+
+    it('returns cached value on subsequent calls without calling computeFn again', () => {
+      const compute = jest.fn().mockReturnValue({ entry: destinationOne });
+      cacheOne.getOrInsertComputed('cached', compute);
+      const result = cacheOne.getOrInsertComputed('cached', compute);
+      expect(result).toEqual(destinationOne);
+      expect(compute).toHaveBeenCalledTimes(1);
+    });
+
+    it('stores a custom expiration returned by computeFn', () => {
+      jest.useFakeTimers();
+      const timeToExpire = 5000;
+      const compute = jest.fn().mockReturnValue({
+        entry: destinationOne,
+        expires: Date.now() + timeToExpire
+      });
+
+      cacheOne.getOrInsertComputed('custom-expire', compute);
+      jest.advanceTimersByTime(timeToExpire + 1);
+
+      expect(cacheOne.get('custom-expire')).toBeUndefined();
+      expect(compute).toHaveBeenCalledTimes(1);
+    });
+  });
 });

packages/connectivity/src/scp-cf/cache.ts

@@ -1,8 +1,14 @@
+import { createHash } from 'node:crypto';
+
 interface CacheInterface<T> {
   hasKey(key: string): boolean;
   get(key: string | undefined): T | undefined;
   set(key: string | undefined, item: CacheEntry<T>): void;
   clear(): void;
+  getOrInsertComputed(
+    key: string | undefined,
+    computeFn: () => CacheEntry<T>
+  ): T;
 }
 
 /**
@@ -39,21 +45,25 @@ export class Cache<T> implements CacheInterface<T> {
   /**
    * Object that stores all cached entries.
    */
-  private cache: Record<string, CacheEntry<T>>;
+  private cache: Map<string, CacheEntry<T>>;
 
   /**
    * Creates an instance of Cache.
    * @param defaultValidityTime - The default validity time in milliseconds. Use 0 for unlimited cache duration.
+   * @param maxSize - The maximum number of entries in the cache. Use Infinity for unlimited size. Items are evicted based on a least recently used (LRU) strategy.
    */
-  constructor(private defaultValidityTime: number) {
-    this.cache = {};
+  constructor(
+    private defaultValidityTime: number,
+    private maxSize = Infinity
+  ) {
+    this.cache = new Map<string, CacheEntry<T>>();
   }
 
   /**
    * Clear all cached items.
    */
   clear(): void {
-    this.cache = {};
+    this.cache.clear();
   }
 
   /**
@@ -62,7 +72,7 @@ export class Cache<T> implements CacheInterface<T> {
    * @returns A boolean value that indicates whether the entry exists in cache.
    */
   hasKey(key: string): boolean {
-    return this.cache.hasOwnProperty(key);
+    return this.cache.has(key);
   }
 
   /**
@@ -71,9 +81,19 @@ export class Cache<T> implements CacheInterface<T> {
    * @returns The corresponding entry to the provided key if it is still valid, returns `undefined` otherwise.
    */
   get(key: string | undefined): T | undefined {
-    return key && this.hasKey(key) && !isExpired(this.cache[key])
-      ? this.cache[key].entry
-      : undefined;
+    const entry = key ? this.cache.get(key) : undefined;
+    if (entry) {
+      if (isExpired(entry)) {
+        this.cache.delete(key!);
+        return undefined;
+      }
+      // LRU cache: Move accessed entry to the end of the Map to mark it as recently used
+      if (this.maxSize !== Infinity) {
+        this.cache.delete(key!);
+        this.cache.set(key!, entry);
+      }
+    }
+    return entry?.entry;
   }
 
   /**
@@ -82,10 +102,41 @@ export class Cache<T> implements CacheInterface<T> {
    * @param item - The entry to cache.
    */
   set(key: string | undefined, item: CacheEntry<T>): void {
-    if (key) {
-      const expires = item.expires ?? this.inferDefaultExpirationTime();
-      this.cache[key] = { entry: item.entry, expires };
+    if (!key) {
+      return;
+    }
+    if (
+      this.cache.size >= this.maxSize &&
+      this.cache.size > 0 &&
+      !this.cache.has(key)
+    ) {
+      // Evict the least recently used (LRU) entry
+      const lruKey = this.cache.keys().next().value;
+      this.cache.delete(lruKey!); // SAFETY: size > 0
     }
+    if (this.maxSize !== Infinity && this.cache.has(key)) {
+      // If the key already exists, delete it to update its position in the LRU order
+      this.cache.delete(key);
+    }
+
+    const expires = item.expires ?? this.inferDefaultExpirationTime();
+    this.cache.set(key, { entry: item.entry, expires });
+  }
+
+  getOrInsertComputed(
+    key: string | undefined,
+    computeFn: () => CacheEntry<T>
+  ): T {
+    if (!key) {
+      return computeFn().entry;
+    }
+    const cachedEntry = this.get(key);
+    if (cachedEntry !== undefined) {
+      return cachedEntry;
+    }
+    const newEntry = computeFn();
+    this.set(key, newEntry);
+    return newEntry.entry;
   }
 
   private inferDefaultExpirationTime(): number | undefined {
@@ -98,6 +149,17 @@ export class Cache<T> implements CacheInterface<T> {
   }
 }
 
+/**
+ * Hashes the given value to create a cache key.
+ * @internal
+ * @param value - The value to hash.
+ * @returns A hash of the given value using a cryptographic hash function.
+ */
+export function hashCacheKey(value: Record<string, unknown>): string {
+  const serialized = JSON.stringify(value);
+  return createHash('blake2s256').update(serialized).digest('hex');
+}
+
 function isExpired<T>(item: CacheEntry<T>): boolean {
   if (item.expires === undefined) {
     return false;