Skip to content

Commit 7b40528

Browse files
AmsterGetAmsterGet
authored
EPMRPP-109667 || Tune proxy agents configs. Update retries policy (#243)
1 parent ef971ac commit 7b40528

File tree

3 files changed

+140
-52
lines changed

3 files changed

+140
-52
lines changed

‎__tests__/rest.spec.js‎

Lines changed: 61 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@ describe('RestClient', () => {
2121
const noOptions = {};
2222
const getRetryAttempts = (client) => client.getRetryConfig().retries + 1;
2323
const restClient = new RestClient(options);
24+
const restClientNoRetry = new RestClient({
25+
...options,
26+
restClientConfig: {
27+
...options.restClientConfig,
28+
retry: 0,
29+
},
30+
});
2431
const retryAttempts = getRetryAttempts(restClient);
2532

2633
const unathorizedError = {
@@ -58,13 +65,20 @@ describe('RestClient', () => {
5865
describe('retry configuration', () => {
5966
it('uses a production-ready retry policy by default', () => {
6067
const retryConfig = restClient.getRetryConfig();
68+
const mathRandomSpy = jest.spyOn(Math, 'random').mockImplementationOnce(() => 0);
6169

6270
expect(retryConfig.retries).toBe(6);
6371
expect(retryAttempts).toBe(retryConfig.retries + 1);
6472
expect(retryConfig.shouldResetTimeout).toBe(true);
6573
expect(retryConfig.retryDelay(1)).toBe(200);
66-
expect(retryConfig.retryDelay(4)).toBe(1600);
74+
75+
mathRandomSpy.mockImplementationOnce(() => 1);
76+
expect(retryConfig.retryDelay(4)).toBeCloseTo(1600 * 0.6);
77+
78+
mathRandomSpy.mockImplementationOnce(() => 0);
6779
expect(retryConfig.retryDelay(10)).toBe(5000);
80+
81+
mathRandomSpy.mockRestore();
6882
});
6983

7084
it('uses custom retry attempts when a numeric value is provided', (done) => {
@@ -80,6 +94,7 @@ describe('RestClient', () => {
8094

8195
const scope = nock(options.baseURL)
8296
.get('/users/custom-retry-number')
97+
.times(getRetryAttempts(client))
8398
.replyWithError(netErrConnectionResetError);
8499

85100
client.retrieve('users/custom-retry-number', noOptions).catch((error) => {
@@ -111,6 +126,14 @@ describe('RestClient', () => {
111126
expect(retryConfig.retryDelay).toBe(customDelay);
112127
expect(retryConfig.shouldResetTimeout).toBe(true);
113128
});
129+
130+
it('retries axios timeout errors even without ECONNABORTED code', () => {
131+
const retryConfig = restClient.getRetryConfig();
132+
const timeoutError = {
133+
message: 'timeout of 1ms exceeded',
134+
};
135+
expect(retryConfig.retryCondition(timeoutError)).toBe(true);
136+
});
114137
});
115138

116139
describe('buildPath', () => {
@@ -122,22 +145,32 @@ describe('RestClient', () => {
122145
});
123146

124147
describe('getRestConfig', () => {
125-
it("return {} in case agent property is doesn't exist", () => {
126-
restClient.restClientConfig = {};
127-
expect(restClient.getRestConfig()).toEqual({});
148+
it("return {} in case agent property doesn't exist", () => {
149+
const client = new RestClient({
150+
...options,
151+
restClientConfig: {},
152+
});
153+
154+
expect(client.getRestConfig()).toEqual({});
128155
});
129156

130157
it('creates object with correct properties with http(s) agent', () => {
131-
restClient.restClientConfig = {
132-
agent: {
133-
rejectUnauthorized: false,
158+
const client = new RestClient({
159+
...options,
160+
restClientConfig: {
161+
agent: {
162+
rejectUnauthorized: false,
163+
},
164+
timeout: 10000,
134165
},
135-
timeout: 10000,
136-
};
137-
expect(restClient.getRestConfig().httpAgent).toBeDefined();
138-
expect(restClient.getRestConfig().httpAgent).toBeInstanceOf(http.Agent);
139-
expect(restClient.getRestConfig().timeout).toBe(10000);
140-
expect(restClient.getRestConfig().agent).toBeUndefined();
166+
});
167+
168+
const config = client.getRestConfig();
169+
170+
expect(config.httpAgent).toBeDefined();
171+
expect(config.httpAgent).toBeInstanceOf(http.Agent);
172+
expect(config.timeout).toBe(10000);
173+
expect(config.agent).toBeUndefined();
141174
});
142175
});
143176

@@ -147,7 +180,7 @@ describe('RestClient', () => {
147180

148181
const scope = nock(options.baseURL).get('/users').reply(200, listOfUsers);
149182

150-
restClient.retrieve('users').then((result) => {
183+
restClientNoRetry.retrieve('users').then((result) => {
151184
expect(result).toEqual(listOfUsers);
152185
expect(scope.isDone()).toBeTruthy();
153186

@@ -158,7 +191,7 @@ describe('RestClient', () => {
158191
it('catches NETWORK errors', (done) => {
159192
const scope = nock(options.baseURL).get('/users').replyWithError(netErrConnectionResetError);
160193

161-
restClient.retrieve('users', noOptions).catch((error) => {
194+
restClientNoRetry.retrieve('users', noOptions).catch((error) => {
162195
expect(error instanceof Error).toBeTruthy();
163196
expect(error.message).toMatch(netErrConnectionResetError.message);
164197
expect(scope.isDone()).toBeTruthy();
@@ -170,7 +203,7 @@ describe('RestClient', () => {
170203
it('catches API errors', (done) => {
171204
const scope = nock(options.baseURL).get('/users').reply(403, unathorizedError);
172205

173-
restClient.retrieve('users', noOptions).catch((error) => {
206+
restClientNoRetry.retrieve('users', noOptions).catch((error) => {
174207
expect(error instanceof Error).toBeTruthy();
175208
expect(error.message).toMatch(unauthorizedErrorMessage);
176209
expect(scope.isDone()).toBeTruthy();
@@ -189,7 +222,7 @@ describe('RestClient', () => {
189222
.post('/users', (body) => isEqual(body, newUser))
190223
.reply(201, userCreated);
191224

192-
restClient.create('users', newUser).then((result) => {
225+
restClientNoRetry.create('users', newUser).then((result) => {
193226
expect(result).toEqual(userCreated);
194227
expect(scope.isDone()).toBeTruthy();
195228

@@ -204,7 +237,7 @@ describe('RestClient', () => {
204237
.post('/users', (body) => isEqual(body, newUser))
205238
.replyWithError(netErrConnectionResetError);
206239

207-
restClient.create('users', newUser, noOptions).catch((error) => {
240+
restClientNoRetry.create('users', newUser, noOptions).catch((error) => {
208241
expect(error instanceof Error).toBeTruthy();
209242
expect(error.message).toMatch(netErrConnectionResetError.message);
210243
expect(scope.isDone()).toBeTruthy();
@@ -220,7 +253,7 @@ describe('RestClient', () => {
220253
.post('/users', (body) => isEqual(body, newUser))
221254
.reply(403, unathorizedError);
222255

223-
restClient.create('users', newUser, noOptions).catch((error) => {
256+
restClientNoRetry.create('users', newUser, noOptions).catch((error) => {
224257
expect(error instanceof Error).toBeTruthy();
225258
expect(error.message).toMatch(unauthorizedErrorMessage);
226259
expect(scope.isDone()).toBeTruthy();
@@ -239,7 +272,7 @@ describe('RestClient', () => {
239272
.put('/users/1', (body) => isEqual(body, newUserInfo))
240273
.reply(200, userUpdated);
241274

242-
restClient.update('users/1', newUserInfo).then((result) => {
275+
restClientNoRetry.update('users/1', newUserInfo).then((result) => {
243276
expect(result).toEqual(userUpdated);
244277
expect(scope.isDone()).toBeTruthy();
245278

@@ -254,7 +287,7 @@ describe('RestClient', () => {
254287
.put('/users/1', (body) => isEqual(body, newUserInfo))
255288
.replyWithError(netErrConnectionResetError);
256289

257-
restClient.update('users/1', newUserInfo, noOptions).catch((error) => {
290+
restClientNoRetry.update('users/1', newUserInfo, noOptions).catch((error) => {
258291
expect(error instanceof Error).toBeTruthy();
259292
expect(error.message).toMatch(netErrConnectionResetError.message);
260293
expect(scope.isDone()).toBeTruthy();
@@ -270,7 +303,7 @@ describe('RestClient', () => {
270303
.put('/users/1', (body) => isEqual(body, newUserInfo))
271304
.reply(403, unathorizedError);
272305

273-
restClient.update('users/1', newUserInfo, noOptions).catch((error) => {
306+
restClientNoRetry.update('users/1', newUserInfo, noOptions).catch((error) => {
274307
expect(error instanceof Error).toBeTruthy();
275308
expect(error.message).toMatch(unauthorizedErrorMessage);
276309
expect(scope.isDone()).toBeTruthy();
@@ -287,7 +320,7 @@ describe('RestClient', () => {
287320

288321
const scope = nock(options.baseURL).delete('/users/1').reply(200, userDeleted);
289322

290-
restClient.delete('users/1', emptyBody).then((result) => {
323+
restClientNoRetry.delete('users/1', emptyBody).then((result) => {
291324
expect(result).toEqual(userDeleted);
292325
expect(scope.isDone()).toBeTruthy();
293326

@@ -302,7 +335,7 @@ describe('RestClient', () => {
302335
.delete('/users/1')
303336
.replyWithError(netErrConnectionResetError);
304337

305-
restClient.delete('users/1', emptyBody, noOptions).catch((error) => {
338+
restClientNoRetry.delete('users/1', emptyBody, noOptions).catch((error) => {
306339
expect(error instanceof Error).toBeTruthy();
307340
expect(error.message).toMatch(netErrConnectionResetError.message);
308341
expect(scope.isDone()).toBeTruthy();
@@ -316,7 +349,7 @@ describe('RestClient', () => {
316349

317350
const scope = nock(options.baseURL).delete('/users/1').reply(403, unathorizedError);
318351

319-
restClient.delete('users/1', emptyBody, noOptions).catch((error) => {
352+
restClientNoRetry.delete('users/1', emptyBody, noOptions).catch((error) => {
320353
expect(error instanceof Error).toBeTruthy();
321354
expect(error.message).toMatch(unauthorizedErrorMessage);
322355
expect(scope.isDone()).toBeTruthy();
@@ -332,7 +365,7 @@ describe('RestClient', () => {
332365

333366
const scope = nock(options.baseURL).get('/users').reply(200, listOfUsers);
334367

335-
restClient.retrieveSyncAPI('users').then((result) => {
368+
restClientNoRetry.retrieveSyncAPI('users').then((result) => {
336369
expect(result).toEqual(listOfUsers);
337370
expect(scope.isDone()).toBeTruthy();
338371

@@ -343,7 +376,7 @@ describe('RestClient', () => {
343376
it('catches NETWORK errors', (done) => {
344377
const scope = nock(options.baseURL).get('/users').replyWithError(netErrConnectionResetError);
345378

346-
restClient.retrieveSyncAPI('users', noOptions).catch((error) => {
379+
restClientNoRetry.retrieveSyncAPI('users', noOptions).catch((error) => {
347380
expect(error instanceof Error).toBeTruthy();
348381
expect(error.message).toMatch(netErrConnectionResetError.message);
349382
expect(scope.isDone()).toBeTruthy();
@@ -355,7 +388,7 @@ describe('RestClient', () => {
355388
it('catches API errors', (done) => {
356389
const scope = nock(options.baseURL).get('/users').reply(403, unathorizedError);
357390

358-
restClient.retrieveSyncAPI('users', noOptions).catch((error) => {
391+
restClientNoRetry.retrieveSyncAPI('users', noOptions).catch((error) => {
359392
expect(error instanceof Error).toBeTruthy();
360393
expect(error.message).toMatch(unauthorizedErrorMessage);
361394
expect(scope.isDone()).toBeTruthy();

‎lib/proxyHelper.js‎

Lines changed: 47 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -136,8 +136,22 @@ function getProxyConfig(url, proxyConfig = {}) {
136136
return null;
137137
}
138138

139+
// Cache for proxy agents to enable connection reuse
140+
const agentCache = new Map();
141+
142+
/**
143+
* Creates a cache key for proxy agents based on proxy URL and protocol
144+
* @param {string} proxyUrl - The proxy URL
145+
* @param {boolean} isHttps - Whether target is HTTPS
146+
* @returns {string} - Cache key
147+
*/
148+
function getAgentCacheKey(proxyUrl, isHttps) {
149+
return `${isHttps ? 'https' : 'http'}:${proxyUrl}`;
150+
}
151+
139152
/**
140153
* Creates an HTTP/HTTPS agent with proxy configuration for a specific URL
154+
* Agents are cached and reused to enable connection pooling and keepAlive
141155
* @param {string} url - The target URL for the request
142156
* @param {object} restClientConfig - The rest client configuration
143157
* @returns {object} - Object with httpAgent and/or httpsAgent
@@ -147,43 +161,56 @@ function createProxyAgents(url, restClientConfig = {}) {
147161
const isHttps = urlObj.protocol === 'https:';
148162
const proxyConfig = getProxyConfig(url, restClientConfig);
149163

164+
// Agent options for connection reuse and keepAlive
165+
const agentOptions = {
166+
keepAlive: true,
167+
keepAliveMsecs: 3000,
168+
maxSockets: 50,
169+
maxFreeSockets: 10,
170+
};
171+
150172
if (!proxyConfig) {
151173
if (restClientConfig.debug) {
152174
console.log('[ProxyHelper] No proxy for URL (bypassed or not configured):', url);
153175
console.log(' Using default agent to prevent axios from using env proxy');
154176
}
177+
178+
const cacheKey = getAgentCacheKey('no-proxy', isHttps);
179+
if (agentCache.has(cacheKey)) {
180+
return agentCache.get(cacheKey);
181+
}
182+
155183
// Return a default agent to prevent axios from using HTTP_PROXY/HTTPS_PROXY env vars
156184
// This ensures that URLs in noProxy truly bypass the proxy
157-
if (isHttps) {
158-
return {
159-
httpsAgent: new https.Agent(),
160-
};
161-
} else {
162-
return {
163-
httpAgent: new http.Agent(),
164-
};
165-
}
185+
const agents = isHttps
186+
? { httpsAgent: new https.Agent(agentOptions) }
187+
: { httpAgent: new http.Agent(agentOptions) };
188+
agentCache.set(cacheKey, agents);
189+
return agents;
166190
}
167191

168192
const { proxyUrl } = proxyConfig;
169193

194+
const cacheKey = getAgentCacheKey(proxyUrl, isHttps);
195+
if (agentCache.has(cacheKey)) {
196+
if (restClientConfig.debug) {
197+
console.log('[ProxyHelper] Reusing cached proxy agent:', sanitizeUrlForLogging(proxyUrl));
198+
}
199+
return agentCache.get(cacheKey);
200+
}
201+
170202
if (restClientConfig.debug) {
171203
console.log('[ProxyHelper] Creating proxy agent:');
172204
console.log(' URL:', url);
173-
console.log(' Protocol:', urlObj.protocol);
174205
console.log(' Proxy URL:', sanitizeUrlForLogging(proxyUrl));
175206
}
176207

177-
// Create appropriate agent based on target protocol
178-
if (isHttps) {
179-
return {
180-
httpsAgent: new HttpsProxyAgent(proxyUrl),
181-
};
182-
} else {
183-
return {
184-
httpAgent: new HttpProxyAgent(proxyUrl),
185-
};
186-
}
208+
const agents = isHttps
209+
? { httpsAgent: new HttpsProxyAgent(proxyUrl, agentOptions) }
210+
: { httpAgent: new HttpProxyAgent(proxyUrl, agentOptions) };
211+
212+
agentCache.set(cacheKey, agents);
213+
return agents;
187214
}
188215

189216
/**

0 commit comments

Comments
 (0)