diff --git a/CHANGELOG.md b/CHANGELOG.md index a3a93c0..483df65 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Changed +- HTTP retries delay calculation to use exponential backoff strategy with jitter. +- HTTP retries condition to retry on axios timeout errors. +- http/https agents now use `keep-alive` by default. ## [5.5.4] - 2025-11-13 ### Fixed diff --git a/README.md b/README.md index e58f891..f78c9f2 100644 --- a/README.md +++ b/README.md @@ -119,34 +119,31 @@ rpClient.checkConnect().then(() => { | headers | Optional | {} | The object with custom headers for internal http client. | | debug | Optional | false | This flag allows seeing the logs of the client. Useful for debugging. | | isLaunchMergeRequired | Optional | false | Allows client to merge launches into one at the end of the run via saving their UUIDs to the temp files at filesystem. At the end of the run launches can be merged using `mergeLaunches` method. Temp file format: `rplaunch-${launch_uuid}.tmp`. | -| restClientConfig | Optional | Not set | `axios` like http client [config](https://github.com/axios/axios#request-config). Supports `proxy` and `noProxy` for proxy configuration (see [Proxy configuration](#proxy-configuration)), `agent` property for [http(s)](https://nodejs.org/api/https.html#https_https_request_url_options_callback) client options, `timeout`, `debug: true` for debugging, and `retry` property (number or [`axios-retry`](https://github.com/softonic/axios-retry#options) config) for automatic retries. | +| restClientConfig | Optional | Not set | Check the details in the [HTTP client config](#http-client-options). | | launchUuidPrint | Optional | false | Whether to print the current launch UUID. | | launchUuidPrintOutput | Optional | 'STDOUT' | Launch UUID printing output. Possible values: 'STDOUT', 'STDERR', 'FILE', 'ENVIRONMENT'. Works only if `launchUuidPrint` set to `true`. File format: `rp-launch-uuid-${launch_uuid}.tmp`. Env variable: `RP_LAUNCH_UUID`. | | token | Deprecated | Not set | Use `apiKey` or `oauth` instead. | -## Asynchronous reporting +### HTTP client options -The client supports an asynchronous reporting (via the ReportPortal asynchronous API). -If you want the client to report through the asynchronous API, change `v1` to `v2` in the `endpoint` address. +`axios` like http client [config](https://github.com/axios/axios#request-config). -## API - -Each method (except checkConnect) returns an object in a specific format: -```javascript -{ - tempId: '4ds43fs', // generated by the client id for further work with the created item - promise: Promise // An object indicating the completion of an operation -} -``` -The client works synchronously, so it is not necessary to wait for the end of the previous requests to send following ones. - -### Timeout (30000ms) on axios requests +#### Timeout (30000ms) on axios requests There is a timeout on axios requests. If for instance the server your making a request to is taking too long to load, then axios timeout will work and you will see the error 'Error: timeout of 30000ms exceeded'. You can simply change this timeout by adding a `timeout` property to `restClientConfig` with your desired numeric value (in _ms_) or *0* to disable it. -### Retry configuration +#### Debug + +Use `debug: true` for debugging. + +#### Agent + +Use `agent` property to provide [http(s)](https://nodejs.org/api/https.html#https_https_request_url_options_callback) agent options. +Use this property in case the direct `httpAgent/httpsAgent` instance property cannot be used due to config serializations. + +#### Retry configuration The client retries failed HTTP calls up to 6 times with an exponential backoff (starting at 200 ms and capping at 5 s) and resets the axios timeout before each retry. Provide a `retry` option in `restClientConfig` to change that behaviour. The value can be either a number (overriding just the retry count) or a full [`axios-retry` configuration object](https://github.com/softonic/axios-retry#options): @@ -166,13 +163,13 @@ const client = new RPClient({ Setting `retry: 0` disables automatic retries. -### Proxy configuration +#### Proxy configuration The client supports comprehensive proxy configuration for both HTTP and HTTPS requests, including ReportPortal API calls and OAuth token requests. Proxy settings can be configured via `restClientConfig` or environment variables. -#### Basic proxy configuration +##### Basic proxy configuration -##### Via configuration object +###### Via configuration object ```javascript const RPClient = require('@reportportal/client-javascript'); @@ -197,7 +194,7 @@ const rpClient = new RPClient({ }); ``` -##### Via proxy URL string +###### Via proxy URL string ```javascript const rpClient = new RPClient({ @@ -208,7 +205,7 @@ const rpClient = new RPClient({ }); ``` -##### Via environment variables +###### Via environment variables The client automatically detects and uses proxy environment variables: @@ -218,7 +215,7 @@ export HTTP_PROXY=http://127.0.0.1:8080 export NO_PROXY=localhost,127.0.0.1,.local ``` -#### Bypassing proxy for specific domains (noProxy) +##### Bypassing proxy for specific domains (noProxy) Use the `noProxy` option to exclude specific domains from being proxied. This is useful when some services are accessible directly while others require a proxy. @@ -245,7 +242,7 @@ const rpClient = new RPClient({ **Priority:** Configuration `noProxy` takes precedence over `NO_PROXY` environment variable. -#### Proxy with OAuth authentication +##### Proxy with OAuth authentication When using OAuth authentication, the proxy configuration is automatically applied to both: - OAuth token endpoint requests @@ -273,9 +270,9 @@ const rpClient = new RPClient({ }); ``` -#### Advanced proxy scenarios +##### Advanced proxy scenarios -##### Disable proxy explicitly +###### Disable proxy explicitly ```javascript restClientConfig: { @@ -283,7 +280,7 @@ restClientConfig: { } ``` -##### Debug proxy configuration +###### Debug proxy configuration Enable debug mode to see detailed proxy decision logs: @@ -308,7 +305,7 @@ Debug output example: Proxy URL: https://127.0.0.1:8080 ``` -#### Proxy configuration options +##### Proxy configuration options | Option | Type | Description | |---------------------|------------------------------|-------------------------------------------------------------------------------------------------| @@ -321,7 +318,7 @@ Debug output example: | `proxy.auth.password` | `string` | Proxy password | | `noProxy` | `string` | Comma-separated list of domains to bypass proxy | -#### How proxy handling works +##### How proxy handling works 1. **Per-request proxy decision:** Each request (API or OAuth) determines its proxy configuration based on the target URL 2. **noProxy checking:** URLs matching `noProxy` patterns bypass the proxy and connect directly @@ -329,6 +326,22 @@ Debug output example: 4. **Environment variable support:** `HTTP_PROXY`, `HTTPS_PROXY`, and `NO_PROXY` are automatically detected and used if no explicit configuration is provided 5. **Priority:** Explicit configuration takes precedence over environment variables +## Asynchronous reporting + +The client supports an asynchronous reporting (via the ReportPortal asynchronous API). +If you want the client to report through the asynchronous API, change `v1` to `v2` in the `endpoint` address. + +## API + +Each method (except checkConnect) returns an object in a specific format: +```javascript +{ + tempId: '4ds43fs', // generated by the client id for further work with the created item + promise: Promise // An object indicating the completion of an operation +} +``` +The client works synchronously, so it is not necessary to wait for the end of the previous requests to send following ones. + ### checkConnect `checkConnect` - asynchronous method for verifying the correctness of the client connection diff --git a/VERSION b/VERSION index c8f1d09..a022a20 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -5.5.4 +5.5.5-SNAPSHOT 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 = {}) {