Skip to content

EPMRPP-109667 || Tune proxy agents configs. Update retries policy #243

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.

Sign up for GitHub

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

Merged
AmsterGet merged 1 commit into from
Nov 18, 2025
Merged

EPMRPP-109667 || Tune proxy agents configs. Update retries policy #243

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 61 additions & 28 deletions __tests__/rest.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down Expand Up @@ -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) => {
Expand All @@ -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) => {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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();
});
});

Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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();

Expand All @@ -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();
Expand All @@ -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();
Expand Down
67 changes: 47 additions & 20 deletions lib/proxyHelper.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}

/**
Expand Down
Loading
Loading