diff --git a/CHANGELOG.md b/CHANGELOG.md index 4eba4ed..016be7f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,7 @@ +### Added +- Allow configuring the HTTP retry strategy via `restClientConfig.retry` and tune the default policy. +### Security +- Updated versions of vulnerable packages (axios). ## [5.4.1] - 2025-07-24 ### Added diff --git a/README.md b/README.md index 841a3ad..85c678b 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,7 @@ When creating a client instance, you need to specify the following options: | 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). May contain `agent` property for configure [http(s)](https://nodejs.org/api/https.html#https_https_request_url_options_callback) client, and other client options eg. `timeout`. For debugging and displaying logs you can set `debug: true`. | +| restClientConfig | Optional | Not set | `axios` like http client [config](https://github.com/axios/axios#request-config). May contain `agent` property for configure [http(s)](https://nodejs.org/api/https.html#https_https_request_url_options_callback) client, and other client options eg. `timeout`. For debugging and displaying logs you can set `debug: true`. Use the `retry` property (number or [`axios-retry`](https://github.com/softonic/axios-retry#options) config) to customise automatic retries. | | 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` instead. | @@ -88,6 +88,26 @@ There is a timeout on axios requests. If for instance the server your making a r 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 + +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): + +```javascript +const axiosRetry = require('axios-retry').default; + +const client = new RPClient({ + // ... other options + restClientConfig: { + retry: { + retries: 5, + retryDelay: axiosRetry.exponentialDelay, + }, + }, +}); +``` + +Setting `retry: 0` disables automatic retries. + ### checkConnect `checkConnect` - asynchronous method for verifying the correctness of the client connection diff --git a/VERSION b/VERSION index ade6522..1b18ae2 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -5.4.1 +5.4.2-SNAPSHOT diff --git a/__tests__/client-id.spec.js b/__tests__/client-id.spec.js index 130b9ff..c65a457 100644 --- a/__tests__/client-id.spec.js +++ b/__tests__/client-id.spec.js @@ -1,18 +1,33 @@ const fs = require('fs'); const util = require('util'); -const os = require('os'); const path = require('path'); const { v4: uuidv4 } = require('uuid'); + +const testHomeDir = path.join(__dirname, '__tmp__', 'rp-home'); +process.env.RP_CLIENT_JS_HOME = testHomeDir; const { getClientId } = require('../statistics/client-id'); const uuidv4Validation = /^[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}$/i; -const clientIdFile = path.join(os.homedir(), '.rp', 'rp.properties'); +const clientIdFile = path.join(testHomeDir, '.rp', 'rp.properties'); const unlink = util.promisify(fs.unlink); const readFile = util.promisify(fs.readFile); const writeFile = util.promisify(fs.writeFile); +const removeTestHomeDir = () => fs.promises.rm(testHomeDir, { recursive: true, force: true }); +const unlinkFile = async (filePath) => { + try { + await unlink(filePath); + } catch (error) { + if (error.code !== 'ENOENT') { + throw error; + } + } +}; describe('Client ID test suite', () => { + beforeAll(removeTestHomeDir); + afterAll(removeTestHomeDir); + it('getClientId should return the same client ID for two calls', async () => { const clientId1 = await getClientId(); const clientId2 = await getClientId(); @@ -22,7 +37,7 @@ describe('Client ID test suite', () => { it('getClientId should return different client IDs if store file removed', async () => { const clientId1 = await getClientId(); - await unlink(clientIdFile); + await unlinkFile(clientIdFile); const clientId2 = await getClientId(); expect(clientId2).not.toEqual(clientId1); }); @@ -33,14 +48,14 @@ describe('Client ID test suite', () => { }); it('getClientId should save client ID to ~/.rp/rp.properties', async () => { - await unlink(clientIdFile); + await unlinkFile(clientIdFile); const clientId = await getClientId(); const content = await readFile(clientIdFile, 'utf-8'); expect(content).toMatch(new RegExp(`^client\\.id\\s*=\\s*${clientId}\\s*(?:$|\n)`)); }); it('getClientId should read client ID from ~/.rp/rp.properties', async () => { - await unlink(clientIdFile); + await unlinkFile(clientIdFile); const clientId = uuidv4(undefined, undefined, 0); await writeFile(clientIdFile, `client.id=${clientId}\n`, 'utf-8'); expect(await getClientId()).toEqual(clientId); @@ -50,7 +65,7 @@ describe('Client ID test suite', () => { 'getClientId should read client ID from ~/.rp/rp.properties if it is not empty and client ID is the ' + 'first line', async () => { - await unlink(clientIdFile); + await unlinkFile(clientIdFile); const clientId = uuidv4(undefined, undefined, 0); await writeFile(clientIdFile, `client.id=${clientId}\ntest.property=555\n`, 'utf-8'); expect(await getClientId()).toEqual(clientId); @@ -61,7 +76,7 @@ describe('Client ID test suite', () => { 'getClientId should read client ID from ~/.rp/rp.properties if it is not empty and client ID is not the ' + 'first line', async () => { - await unlink(clientIdFile); + await unlinkFile(clientIdFile); const clientId = uuidv4(undefined, undefined, 0); await writeFile(clientIdFile, `test.property=555\nclient.id=${clientId}\n`, 'utf-8'); expect(await getClientId()).toEqual(clientId); @@ -69,7 +84,7 @@ describe('Client ID test suite', () => { ); it('getClientId should write client ID to ~/.rp/rp.properties if it is not empty', async () => { - await unlink(clientIdFile); + await unlinkFile(clientIdFile); await writeFile(clientIdFile, `test.property=555`, 'utf-8'); const clientId = await getClientId(); const content = await readFile(clientIdFile, 'utf-8'); diff --git a/__tests__/rest.spec.js b/__tests__/rest.spec.js index 850b8e4..ccb7b63 100644 --- a/__tests__/rest.spec.js +++ b/__tests__/rest.spec.js @@ -19,7 +19,9 @@ describe('RestClient', () => { }, }; const noOptions = {}; + const getRetryAttempts = (client) => client.getRetryConfig().retries + 1; const restClient = new RestClient(options); + const retryAttempts = getRetryAttempts(restClient); const unathorizedError = { error: 'unauthorized', @@ -53,6 +55,64 @@ describe('RestClient', () => { }); }); + describe('retry configuration', () => { + it('uses a production-ready retry policy by default', () => { + const retryConfig = restClient.getRetryConfig(); + + 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); + expect(retryConfig.retryDelay(10)).toBe(5000); + }); + + it('uses custom retry attempts when a numeric value is provided', (done) => { + const customRetries = 2; + const client = new RestClient({ + ...options, + restClientConfig: { + ...options.restClientConfig, + retry: customRetries, + }, + }); + expect(getRetryAttempts(client)).toBe(customRetries + 1); + + const scope = nock(options.baseURL) + .get('/users/custom-retry-number') + .replyWithError(netErrConnectionResetError); + + client.retrieve('users/custom-retry-number', noOptions).catch((error) => { + expect(error instanceof Error).toBeTruthy(); + expect(error.message).toMatch(netErrConnectionResetError.message); + expect(scope.isDone()).toBeTruthy(); + + done(); + }); + }); + + it('merges retry configuration object from settings', () => { + const customDelay = () => 250; + const client = new RestClient({ + ...options, + restClientConfig: { + ...options.restClientConfig, + retry: { + retries: 4, + retryDelay: customDelay, + shouldResetTimeout: true, + }, + }, + }); + + const retryConfig = client.getRetryConfig(); + + expect(retryConfig.retries).toBe(4); + expect(retryConfig.retryDelay).toBe(customDelay); + expect(retryConfig.shouldResetTimeout).toBe(true); + }); + }); + describe('buildPath', () => { it('compose path basing on base', () => { expect(restClient.buildPath('users')).toBe(`${options.baseURL}/users`); diff --git a/lib/rest.js b/lib/rest.js index 3528329..a065e4d 100644 --- a/lib/rest.js +++ b/lib/rest.js @@ -5,12 +5,17 @@ const https = require('https'); const logger = require('./logger'); const DEFAULT_MAX_CONNECTION_TIME_MS = 30000; - -axiosRetry(axios, { - retryDelay: () => 100, - retries: 10, +const DEFAULT_RETRY_ATTEMPTS = 6; +const RETRY_BASE_DELAY_MS = 200; +const RETRY_MAX_DELAY_MS = 5000; +const DEFAULT_RETRY_CONFIG = { + retryDelay: (retryCount = 1) => + Math.min(RETRY_BASE_DELAY_MS * 2 ** Math.max(retryCount - 1, 0), RETRY_MAX_DELAY_MS), + retries: DEFAULT_RETRY_ATTEMPTS, retryCondition: axiosRetry.isRetryableError, -}); + shouldResetTimeout: true, +}; +const SKIPPED_REST_CONFIG_KEYS = ['agent', 'retry']; class RestClient { constructor(options) { @@ -24,6 +29,8 @@ class RestClient { ...this.getRestConfig(this.restClientConfig), }); + axiosRetry(this.axiosInstance, this.getRetryConfig()); + if (this.restClientConfig?.debug) { logger.addLogger(this.axiosInstance); } @@ -69,7 +76,7 @@ method: ${method}`, if (!this.restClientConfig) return {}; const config = Object.keys(this.restClientConfig).reduce((acc, key) => { - if (key !== 'agent') { + if (!SKIPPED_REST_CONFIG_KEYS.includes(key)) { acc[key] = this.restClientConfig[key]; } return acc; @@ -87,6 +94,26 @@ method: ${method}`, return config; } + getRetryConfig() { + const retryOption = this.restClientConfig?.retry; + + if (typeof retryOption === 'number') { + return { + ...DEFAULT_RETRY_CONFIG, + retries: retryOption, + }; + } + + if (retryOption && typeof retryOption === 'object') { + return { + ...DEFAULT_RETRY_CONFIG, + ...retryOption, + }; + } + + return { ...DEFAULT_RETRY_CONFIG }; + } + create(path, data, options = {}) { return this.request('POST', this.buildPath(path), data, { ...options, diff --git a/package-lock.json b/package-lock.json index 8d20bba..e469fa5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -6,11 +6,11 @@ "packages": { "": { "name": "@reportportal/client-javascript", - "version": "5.4.0", + "version": "5.4.1", "license": "Apache-2.0", "dependencies": { - "axios": "^1.8.4", - "axios-retry": "^4.1.0", + "axios": "^1.12.2", + "axios-retry": "^4.5.0", "glob": "^8.1.0", "ini": "^2.0.0", "uniqid": "^5.4.0", @@ -1857,19 +1857,21 @@ } }, "node_modules/axios": { - "version": "1.8.4", - "resolved": "https://registry.npmjs.org/axios/-/axios-1.8.4.tgz", - "integrity": "sha512-eBSYY4Y68NNlHbHBMdeDmKNtDgXWhQsJcGqzO3iLUM0GraQFSS9cVgPX5I9b3lbdFKyYoAEGAZF1DwhTaljNAw==", + "version": "1.12.2", + "resolved": "https://registry.npmjs.org/axios/-/axios-1.12.2.tgz", + "integrity": "sha512-vMJzPewAlRyOgxV2dU0Cuz2O8zzzx9VYtbJOaBgXFeLc4IV/Eg50n4LowmehOOR61S8ZMpc2K5Sa7g6A4jfkUw==", + "license": "MIT", "dependencies": { "follow-redirects": "^1.15.6", - "form-data": "^4.0.0", + "form-data": "^4.0.4", "proxy-from-env": "^1.1.0" } }, "node_modules/axios-retry": { - "version": "4.1.0", - "resolved": "https://registry.npmjs.org/axios-retry/-/axios-retry-4.1.0.tgz", - "integrity": "sha512-svdth4H00yhlsjBbjfLQ/sMLkXqeLxhiFC1nE1JtkN/CIssGxqk0UwTEdrVjwA2gr3yJkAulwvDSIm4z4HyPvg==", + "version": "4.5.0", + "resolved": "https://registry.npmjs.org/axios-retry/-/axios-retry-4.5.0.tgz", + "integrity": "sha512-aR99oXhpEDGo0UuAlYcn2iGRds30k366Zfa05XWScR9QaQD4JYiP3/1Qt1u7YlefUOK+cn0CcwoL1oefavQUlQ==", + "license": "Apache-2.0", "dependencies": { "is-retry-allowed": "^2.2.0" }, diff --git a/package.json b/package.json index 86f8690..a119bd2 100644 --- a/package.json +++ b/package.json @@ -25,8 +25,8 @@ "node": ">=14.x" }, "dependencies": { - "axios": "^1.8.4", - "axios-retry": "^4.1.0", + "axios": "^1.12.2", + "axios-retry": "^4.5.0", "glob": "^8.1.0", "ini": "^2.0.0", "uniqid": "^5.4.0", diff --git a/statistics/constants.js b/statistics/constants.js index 0fbc600..8095e27 100644 --- a/statistics/constants.js +++ b/statistics/constants.js @@ -8,7 +8,8 @@ const PJSON_NAME = pjson.name; const CLIENT_ID_KEY = 'client.id'; const RP_FOLDER = '.rp'; const RP_PROPERTIES_FILE = 'rp.properties'; -const RP_FOLDER_PATH = path.join(os.homedir(), RP_FOLDER); +const HOME_DIRECTORY = process.env.RP_CLIENT_JS_HOME || os.homedir(); +const RP_FOLDER_PATH = path.join(HOME_DIRECTORY, RP_FOLDER); const RP_PROPERTIES_FILE_PATH = path.join(RP_FOLDER_PATH, RP_PROPERTIES_FILE); const CLIENT_INFO = Buffer.from( 'Ry1XUDU3UlNHOFhMOmVFazhPMGJ0UXZ5MmI2VXVRT19TOFE=',