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

@@ -1,4 +1,4 @@
-import { Cache } from './cache';
+import { Cache, hashCacheKey } from './cache';
 import { clientCredentialsTokenCache } from './client-credentials-token-cache';
 import { destinationCache } from './destination';
 import type { AuthenticationType, Destination } from './destination';
@@ -97,4 +97,138 @@ 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('hashCacheKey', () => {
+    it('produces the same hash for plain objects with different key insertion order', () => {
+      expect(hashCacheKey({ a: 1, b: 2 })).toEqual(
+        hashCacheKey({ b: 2, a: 1 })
+      );
+    });
+
+    it('produces different hashes for plain objects with different values', () => {
+      expect(hashCacheKey({ a: 1 })).not.toEqual(hashCacheKey({ a: 2 }));
+    });
+
+    it('produces the same hash for Maps with different insertion order', () => {
+      const m1 = new Map([
+        ['x', 1],
+        ['y', 2]
+      ]);
+      const m2 = new Map([
+        ['y', 2],
+        ['x', 1]
+      ]);
+      expect(hashCacheKey({ m: m1 })).toEqual(hashCacheKey({ m: m2 }));
+    });
+
+    it('produces the same hash for Sets with different insertion order', () => {
+      const s1 = new Set([1, 2, 3]);
+      const s2 = new Set([3, 1, 2]);
+      expect(hashCacheKey({ s: s1 })).toEqual(hashCacheKey({ s: s2 }));
+    });
+
+    it('produces the same hash for class instances regardless of property insertion order', () => {
+      class Point {
+        [key: string]: number;
+      }
+      const p1 = new Point();
+      p1.y = 2;
+      p1.x = 1;
+      const p2 = new Point();
+      p2.x = 1;
+      p2.y = 2;
+      expect(hashCacheKey({ p: p1 })).toEqual(hashCacheKey({ p: p2 }));
+    });
+    it('preserves arrays as arrays (does not coerce to object)', () => {
+      expect(hashCacheKey({ arr: [1, 2, 3] })).not.toEqual(
+        hashCacheKey({ arr: { 0: 1, 1: 2, 2: 3 } })
+      );
+    });
+  });
+
+  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);
+    });
+  });
 });