From abb7370e446dfcc31300f9b5a6528ea91a8e14f6 Mon Sep 17 00:00:00 2001 From: Ilya_Hancharyk Date: Tue, 18 Nov 2025 12:02:05 +0100 Subject: [PATCH] EPMRPP-109667 || Tune proxy agents configs. Update retries policy --- __tests__/rest.spec.js | 89 +++++++++++++++++++++++++++++------------- lib/proxyHelper.js | 67 +++++++++++++++++++++---------- lib/rest.js | 36 +++++++++++++++-- 3 files changed, 140 insertions(+), 52 deletions(-) diff --git a/__tests__/rest.spec.js b/__tests__/rest.spec.js index ccb7b63..ef70c95 100644 --- a/__tests__/rest.spec.js +++ b/__tests__/rest.spec.js @@ -21,6 +21,13 @@ describe('RestClient', () => { const noOptions = {}; const getRetryAttempts = (client) => client.getRetryConfig().retries + 1; const restClient = new RestClient(options); + const restClientNoRetry = new RestClient({ + ...options, + restClientConfig: { + ...options.restClientConfig, + retry: 0, + }, + }); const retryAttempts = getRetryAttempts(restClient); const unathorizedError = { @@ -58,13 +65,20 @@ describe('RestClient', () => { describe('retry configuration', () => { it('uses a production-ready retry policy by default', () => { const retryConfig = restClient.getRetryConfig(); + const mathRandomSpy = jest.spyOn(Math, 'random').mockImplementationOnce(() => 0); expect(retryConfig.retries).toBe(6); expect(retryAttempts).toBe(retryConfig.retries + 1); expect(retryConfig.shouldResetTimeout).toBe(true); expect(retryConfig.retryDelay(1)).toBe(200); - expect(retryConfig.retryDelay(4)).toBe(1600); + + mathRandomSpy.mockImplementationOnce(() => 1); + expect(retryConfig.retryDelay(4)).toBeCloseTo(1600 * 0.6); + + mathRandomSpy.mockImplementationOnce(() => 0); expect(retryConfig.retryDelay(10)).toBe(5000); + + mathRandomSpy.mockRestore(); }); it('uses custom retry attempts when a numeric value is provided', (done) => { @@ -80,6 +94,7 @@ describe('RestClient', () => { const scope = nock(options.baseURL) .get('/users/custom-retry-number') + .times(getRetryAttempts(client)) .replyWithError(netErrConnectionResetError); client.retrieve('users/custom-retry-number', noOptions).catch((error) => { @@ -111,6 +126,14 @@ describe('RestClient', () => { expect(retryConfig.retryDelay).toBe(customDelay); expect(retryConfig.shouldResetTimeout).toBe(true); }); + + it('retries axios timeout errors even without ECONNABORTED code', () => { + const retryConfig = restClient.getRetryConfig(); + const timeoutError = { + message: 'timeout of 1ms exceeded', + }; + expect(retryConfig.retryCondition(timeoutError)).toBe(true); + }); }); describe('buildPath', () => { @@ -122,22 +145,32 @@ describe('RestClient', () => { }); describe('getRestConfig', () => { - it("return {} in case agent property is doesn't exist", () => { - restClient.restClientConfig = {}; - expect(restClient.getRestConfig()).toEqual({}); + it("return {} in case agent property doesn't exist", () => { + const client = new RestClient({ + ...options, + restClientConfig: {}, + }); + + expect(client.getRestConfig()).toEqual({}); }); it('creates object with correct properties with http(s) agent', () => { - restClient.restClientConfig = { - agent: { - rejectUnauthorized: false, + const client = new RestClient({ + ...options, + restClientConfig: { + agent: { + rejectUnauthorized: false, + }, + timeout: 10000, }, - timeout: 10000, - }; - expect(restClient.getRestConfig().httpAgent).toBeDefined(); - expect(restClient.getRestConfig().httpAgent).toBeInstanceOf(http.Agent); - expect(restClient.getRestConfig().timeout).toBe(10000); - expect(restClient.getRestConfig().agent).toBeUndefined(); + }); + + const config = client.getRestConfig(); + + expect(config.httpAgent).toBeDefined(); + expect(config.httpAgent).toBeInstanceOf(http.Agent); + expect(config.timeout).toBe(10000); + expect(config.agent).toBeUndefined(); }); }); @@ -147,7 +180,7 @@ describe('RestClient', () => { const scope = nock(options.baseURL).get('/users').reply(200, listOfUsers); - restClient.retrieve('users').then((result) => { + restClientNoRetry.retrieve('users').then((result) => { expect(result).toEqual(listOfUsers); expect(scope.isDone()).toBeTruthy(); @@ -158,7 +191,7 @@ describe('RestClient', () => { it('catches NETWORK errors', (done) => { const scope = nock(options.baseURL).get('/users').replyWithError(netErrConnectionResetError); - restClient.retrieve('users', noOptions).catch((error) => { + restClientNoRetry.retrieve('users', noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(netErrConnectionResetError.message); expect(scope.isDone()).toBeTruthy(); @@ -170,7 +203,7 @@ describe('RestClient', () => { it('catches API errors', (done) => { const scope = nock(options.baseURL).get('/users').reply(403, unathorizedError); - restClient.retrieve('users', noOptions).catch((error) => { + restClientNoRetry.retrieve('users', noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(unauthorizedErrorMessage); expect(scope.isDone()).toBeTruthy(); @@ -189,7 +222,7 @@ describe('RestClient', () => { .post('/users', (body) => isEqual(body, newUser)) .reply(201, userCreated); - restClient.create('users', newUser).then((result) => { + restClientNoRetry.create('users', newUser).then((result) => { expect(result).toEqual(userCreated); expect(scope.isDone()).toBeTruthy(); @@ -204,7 +237,7 @@ describe('RestClient', () => { .post('/users', (body) => isEqual(body, newUser)) .replyWithError(netErrConnectionResetError); - restClient.create('users', newUser, noOptions).catch((error) => { + restClientNoRetry.create('users', newUser, noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(netErrConnectionResetError.message); expect(scope.isDone()).toBeTruthy(); @@ -220,7 +253,7 @@ describe('RestClient', () => { .post('/users', (body) => isEqual(body, newUser)) .reply(403, unathorizedError); - restClient.create('users', newUser, noOptions).catch((error) => { + restClientNoRetry.create('users', newUser, noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(unauthorizedErrorMessage); expect(scope.isDone()).toBeTruthy(); @@ -239,7 +272,7 @@ describe('RestClient', () => { .put('/users/1', (body) => isEqual(body, newUserInfo)) .reply(200, userUpdated); - restClient.update('users/1', newUserInfo).then((result) => { + restClientNoRetry.update('users/1', newUserInfo).then((result) => { expect(result).toEqual(userUpdated); expect(scope.isDone()).toBeTruthy(); @@ -254,7 +287,7 @@ describe('RestClient', () => { .put('/users/1', (body) => isEqual(body, newUserInfo)) .replyWithError(netErrConnectionResetError); - restClient.update('users/1', newUserInfo, noOptions).catch((error) => { + restClientNoRetry.update('users/1', newUserInfo, noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(netErrConnectionResetError.message); expect(scope.isDone()).toBeTruthy(); @@ -270,7 +303,7 @@ describe('RestClient', () => { .put('/users/1', (body) => isEqual(body, newUserInfo)) .reply(403, unathorizedError); - restClient.update('users/1', newUserInfo, noOptions).catch((error) => { + restClientNoRetry.update('users/1', newUserInfo, noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(unauthorizedErrorMessage); expect(scope.isDone()).toBeTruthy(); @@ -287,7 +320,7 @@ describe('RestClient', () => { const scope = nock(options.baseURL).delete('/users/1').reply(200, userDeleted); - restClient.delete('users/1', emptyBody).then((result) => { + restClientNoRetry.delete('users/1', emptyBody).then((result) => { expect(result).toEqual(userDeleted); expect(scope.isDone()).toBeTruthy(); @@ -302,7 +335,7 @@ describe('RestClient', () => { .delete('/users/1') .replyWithError(netErrConnectionResetError); - restClient.delete('users/1', emptyBody, noOptions).catch((error) => { + restClientNoRetry.delete('users/1', emptyBody, noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(netErrConnectionResetError.message); expect(scope.isDone()).toBeTruthy(); @@ -316,7 +349,7 @@ describe('RestClient', () => { const scope = nock(options.baseURL).delete('/users/1').reply(403, unathorizedError); - restClient.delete('users/1', emptyBody, noOptions).catch((error) => { + restClientNoRetry.delete('users/1', emptyBody, noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(unauthorizedErrorMessage); expect(scope.isDone()).toBeTruthy(); @@ -332,7 +365,7 @@ describe('RestClient', () => { const scope = nock(options.baseURL).get('/users').reply(200, listOfUsers); - restClient.retrieveSyncAPI('users').then((result) => { + restClientNoRetry.retrieveSyncAPI('users').then((result) => { expect(result).toEqual(listOfUsers); expect(scope.isDone()).toBeTruthy(); @@ -343,7 +376,7 @@ describe('RestClient', () => { it('catches NETWORK errors', (done) => { const scope = nock(options.baseURL).get('/users').replyWithError(netErrConnectionResetError); - restClient.retrieveSyncAPI('users', noOptions).catch((error) => { + restClientNoRetry.retrieveSyncAPI('users', noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(netErrConnectionResetError.message); expect(scope.isDone()).toBeTruthy(); @@ -355,7 +388,7 @@ describe('RestClient', () => { it('catches API errors', (done) => { const scope = nock(options.baseURL).get('/users').reply(403, unathorizedError); - restClient.retrieveSyncAPI('users', noOptions).catch((error) => { + restClientNoRetry.retrieveSyncAPI('users', noOptions).catch((error) => { expect(error instanceof Error).toBeTruthy(); expect(error.message).toMatch(unauthorizedErrorMessage); expect(scope.isDone()).toBeTruthy(); diff --git a/lib/proxyHelper.js b/lib/proxyHelper.js index e3ab904..f6e25d4 100644 --- a/lib/proxyHelper.js +++ b/lib/proxyHelper.js @@ -136,8 +136,22 @@ function getProxyConfig(url, proxyConfig = {}) { return null; } +// Cache for proxy agents to enable connection reuse +const agentCache = new Map(); + +/** + * Creates a cache key for proxy agents based on proxy URL and protocol + * @param {string} proxyUrl - The proxy URL + * @param {boolean} isHttps - Whether target is HTTPS + * @returns {string} - Cache key + */ +function getAgentCacheKey(proxyUrl, isHttps) { + return `${isHttps ? 'https' : 'http'}:${proxyUrl}`; +} + /** * Creates an HTTP/HTTPS agent with proxy configuration for a specific URL + * Agents are cached and reused to enable connection pooling and keepAlive * @param {string} url - The target URL for the request * @param {object} restClientConfig - The rest client configuration * @returns {object} - Object with httpAgent and/or httpsAgent @@ -147,43 +161,56 @@ function createProxyAgents(url, restClientConfig = {}) { const isHttps = urlObj.protocol === 'https:'; const proxyConfig = getProxyConfig(url, restClientConfig); + // Agent options for connection reuse and keepAlive + const agentOptions = { + keepAlive: true, + keepAliveMsecs: 3000, + maxSockets: 50, + maxFreeSockets: 10, + }; + if (!proxyConfig) { if (restClientConfig.debug) { console.log('[ProxyHelper] No proxy for URL (bypassed or not configured):', url); console.log(' Using default agent to prevent axios from using env proxy'); } + + const cacheKey = getAgentCacheKey('no-proxy', isHttps); + if (agentCache.has(cacheKey)) { + return agentCache.get(cacheKey); + } + // Return a default agent to prevent axios from using HTTP_PROXY/HTTPS_PROXY env vars // This ensures that URLs in noProxy truly bypass the proxy - if (isHttps) { - return { - httpsAgent: new https.Agent(), - }; - } else { - return { - httpAgent: new http.Agent(), - }; - } + const agents = isHttps + ? { httpsAgent: new https.Agent(agentOptions) } + : { httpAgent: new http.Agent(agentOptions) }; + agentCache.set(cacheKey, agents); + return agents; } const { proxyUrl } = proxyConfig; + const cacheKey = getAgentCacheKey(proxyUrl, isHttps); + if (agentCache.has(cacheKey)) { + if (restClientConfig.debug) { + console.log('[ProxyHelper] Reusing cached proxy agent:', sanitizeUrlForLogging(proxyUrl)); + } + return agentCache.get(cacheKey); + } + if (restClientConfig.debug) { console.log('[ProxyHelper] Creating proxy agent:'); console.log(' URL:', url); - console.log(' Protocol:', urlObj.protocol); console.log(' Proxy URL:', sanitizeUrlForLogging(proxyUrl)); } - // Create appropriate agent based on target protocol - if (isHttps) { - return { - httpsAgent: new HttpsProxyAgent(proxyUrl), - }; - } else { - return { - httpAgent: new HttpProxyAgent(proxyUrl), - }; - } + const agents = isHttps + ? { httpsAgent: new HttpsProxyAgent(proxyUrl, agentOptions) } + : { httpAgent: new HttpProxyAgent(proxyUrl, agentOptions) }; + + agentCache.set(cacheKey, agents); + return agents; } /** diff --git a/lib/rest.js b/lib/rest.js index b5f752f..e8ff4d9 100644 --- a/lib/rest.js +++ b/lib/rest.js @@ -10,11 +10,32 @@ const DEFAULT_MAX_CONNECTION_TIME_MS = 30000; const DEFAULT_RETRY_ATTEMPTS = 6; const RETRY_BASE_DELAY_MS = 200; const RETRY_MAX_DELAY_MS = 5000; + +const isTimeoutError = (error) => { + if (!error) return false; + const message = error.message ? error.message.toLowerCase() : ''; + + return ( + message.includes(`timeout`) || + error.code === 'ECONNABORTED' || + error.code === 'ETIMEDOUT' || + error?.cause?.code === 'ECONNABORTED' || + error?.cause?.code === 'ETIMEDOUT' + ); +}; + +const retryCondition = (error) => { + return axiosRetry.isRetryableError(error) || isTimeoutError(error); +}; + const DEFAULT_RETRY_CONFIG = { - retryDelay: (retryCount = 1) => - Math.min(RETRY_BASE_DELAY_MS * 2 ** Math.max(retryCount - 1, 0), RETRY_MAX_DELAY_MS), + retryDelay: (retryCount = 1) => { + const base = Math.min(RETRY_BASE_DELAY_MS * 2 ** Math.max(retryCount - 1, 0), RETRY_MAX_DELAY_MS); + const jitter = Math.random() * 0.4 * base; // +/-40% + return base - jitter; + }, retries: DEFAULT_RETRY_ATTEMPTS, - retryCondition: axiosRetry.isRetryableError, + retryCondition, shouldResetTimeout: true, }; const SKIPPED_REST_CONFIG_KEYS = ['agent', 'retry', 'proxy', 'noProxy']; @@ -128,9 +149,15 @@ method: ${method}`, getRetryConfig() { const retryOption = this.restClientConfig?.retry; + const onRetry = (retryCount, error, requestConfig) => { + if (this.restClientConfig.debug) { + console.log(`[retry #${retryCount}] ${requestConfig.method?.toUpperCase()} ${requestConfig.url} -> ${error.code || error.message}`); + } + }; if (typeof retryOption === 'number') { return { + onRetry, ...DEFAULT_RETRY_CONFIG, retries: retryOption, }; @@ -138,12 +165,13 @@ method: ${method}`, if (retryOption && typeof retryOption === 'object') { return { + onRetry, ...DEFAULT_RETRY_CONFIG, ...retryOption, }; } - return { ...DEFAULT_RETRY_CONFIG }; + return { onRetry, ...DEFAULT_RETRY_CONFIG }; } create(path, data, options = {}) {