diff --git a/lib/jira-client.js b/lib/jira-client.js index ceaa376..21cb393 100644 --- a/lib/jira-client.js +++ b/lib/jira-client.js @@ -5,6 +5,33 @@ const { expandHomePath } = require('./utils'); const ATLASSIAN_GATEWAY_HOST = 'https://api.atlassian.com'; +const DEFAULT_TIMEOUT_MS = 30000; +const DEFAULT_MAX_RETRIES = 3; +const RETRYABLE_NETWORK_CODES = new Set([ + 'ECONNRESET', + 'ECONNABORTED', + 'ETIMEDOUT', + 'ENOTFOUND', + 'EAI_AGAIN' +]); + +function readPositiveIntEnv(name, fallback, { allowZero = false } = {}) { + const raw = process.env[name]; + if (raw === undefined || raw === '') return fallback; + const parsed = parseInt(raw, 10); + if (!Number.isFinite(parsed)) return fallback; + if (allowZero ? parsed < 0 : parsed <= 0) return fallback; + return parsed; +} + +function getTimeoutMs() { + return readPositiveIntEnv('JIRA_CLI_TIMEOUT_MS', DEFAULT_TIMEOUT_MS); +} + +function getMaxRetries() { + return readPositiveIntEnv('JIRA_CLI_MAX_RETRIES', DEFAULT_MAX_RETRIES, { allowZero: true }); +} + class JiraClient { constructor(config) { this.config = config; @@ -324,37 +351,83 @@ class JiraClient { } createApiClient(version, { auth, headers, httpsAgent }) { - const clientOptions = { + return this.createAxiosClient({ baseURL: this.buildApiBaseUrl(`/rest/api/${version}`), auth, - headers - }; - if (httpsAgent) { - clientOptions.httpsAgent = httpsAgent; - } - const client = axios.create(clientOptions); - client.interceptors.response.use( - response => response, - error => Promise.reject(this.toJiraError(error)) - ); - return client; + headers, + httpsAgent + }); } createAgileClient({ auth, headers, httpsAgent }) { - const clientOptions = { + return this.createAxiosClient({ baseURL: this.buildApiBaseUrl('/rest/agile/1.0'), auth, - headers + headers, + httpsAgent + }); + } + + createAxiosClient({ baseURL, auth, headers, httpsAgent }) { + const clientOptions = { + baseURL, + auth, + headers, + timeout: getTimeoutMs() }; if (httpsAgent) { clientOptions.httpsAgent = httpsAgent; } const client = axios.create(clientOptions); + this.attachResilienceInterceptor(client); + return client; + } + + attachResilienceInterceptor(client) { client.interceptors.response.use( response => response, - error => Promise.reject(this.toJiraError(error)) + async error => { + const config = error && error.config; + const maxRetries = getMaxRetries(); + + if (config && maxRetries > 0 && this.shouldRetry(error)) { + config.__retryCount = config.__retryCount || 0; + if (config.__retryCount < maxRetries) { + config.__retryCount += 1; + const delayMs = this.computeRetryDelay(error, config.__retryCount); + await this.sleep(delayMs); + return client.request(config); + } + } + + return Promise.reject(this.toJiraError(error)); + } ); - return client; + } + + shouldRetry(error) { + if (!error) return false; + if (error.response) { + const status = error.response.status; + return status === 429 || (status >= 500 && status < 600); + } + if (error.code && RETRYABLE_NETWORK_CODES.has(error.code)) return true; + return false; + } + + computeRetryDelay(error, attempt) { + if (error && error.response && error.response.status === 429) { + const headers = error.response.headers || {}; + const retryAfter = parseInt(headers['retry-after'] || headers['Retry-After'] || '', 10); + if (Number.isFinite(retryAfter) && retryAfter > 0) { + return retryAfter * 1000; + } + } + return 1000 * Math.pow(2, attempt - 1); + } + + sleep(ms) { + return new Promise(resolve => setTimeout(resolve, ms)); } toJiraError(error) { diff --git a/tests/jira-client.test.js b/tests/jira-client.test.js index 42d2a43..af91b52 100644 --- a/tests/jira-client.test.js +++ b/tests/jira-client.test.js @@ -540,4 +540,229 @@ describe('JiraClient', () => { expect(errorMessage).toContain('client certificate'); }); }); + + describe('resilience', () => { + describe('timeout', () => { + const ORIGINAL_TIMEOUT = process.env.JIRA_CLI_TIMEOUT_MS; + + afterEach(() => { + if (ORIGINAL_TIMEOUT === undefined) { + delete process.env.JIRA_CLI_TIMEOUT_MS; + } else { + process.env.JIRA_CLI_TIMEOUT_MS = ORIGINAL_TIMEOUT; + } + }); + + test('defaults to 30000ms when env var unset', () => { + delete process.env.JIRA_CLI_TIMEOUT_MS; + const c = new JiraClient(mockConfig); + expect(c.clientV3.defaults.timeout).toBe(30000); + expect(c.clientV2.defaults.timeout).toBe(30000); + expect(c.agileClient.defaults.timeout).toBe(30000); + }); + + test('respects JIRA_CLI_TIMEOUT_MS env override', () => { + process.env.JIRA_CLI_TIMEOUT_MS = '5000'; + const c = new JiraClient(mockConfig); + expect(c.clientV3.defaults.timeout).toBe(5000); + }); + + test('falls back to default for invalid env values', () => { + process.env.JIRA_CLI_TIMEOUT_MS = 'not-a-number'; + const c = new JiraClient(mockConfig); + expect(c.clientV3.defaults.timeout).toBe(30000); + }); + + test('falls back to default for non-positive env values', () => { + process.env.JIRA_CLI_TIMEOUT_MS = '0'; + const c = new JiraClient(mockConfig); + expect(c.clientV3.defaults.timeout).toBe(30000); + }); + }); + + describe('shouldRetry', () => { + test('retries on 429', () => { + expect(client.shouldRetry({ response: { status: 429 } })).toBe(true); + }); + + test('retries on 5xx', () => { + expect(client.shouldRetry({ response: { status: 500 } })).toBe(true); + expect(client.shouldRetry({ response: { status: 503 } })).toBe(true); + expect(client.shouldRetry({ response: { status: 599 } })).toBe(true); + }); + + test('does not retry on 4xx other than 429', () => { + expect(client.shouldRetry({ response: { status: 400 } })).toBe(false); + expect(client.shouldRetry({ response: { status: 401 } })).toBe(false); + expect(client.shouldRetry({ response: { status: 404 } })).toBe(false); + }); + + test('retries on retryable network error codes', () => { + expect(client.shouldRetry({ code: 'ECONNRESET' })).toBe(true); + expect(client.shouldRetry({ code: 'ETIMEDOUT' })).toBe(true); + expect(client.shouldRetry({ code: 'ECONNABORTED' })).toBe(true); + expect(client.shouldRetry({ code: 'ENOTFOUND' })).toBe(true); + expect(client.shouldRetry({ code: 'EAI_AGAIN' })).toBe(true); + }); + + test('does not retry on unknown network error codes', () => { + expect(client.shouldRetry({ code: 'EWHATEVER' })).toBe(false); + }); + + test('does not retry without response or code', () => { + expect(client.shouldRetry({})).toBe(false); + expect(client.shouldRetry(null)).toBe(false); + }); + }); + + describe('computeRetryDelay', () => { + test('uses exponential backoff: 1s, 2s, 4s', () => { + expect(client.computeRetryDelay({ response: { status: 503 } }, 1)).toBe(1000); + expect(client.computeRetryDelay({ response: { status: 503 } }, 2)).toBe(2000); + expect(client.computeRetryDelay({ response: { status: 503 } }, 3)).toBe(4000); + }); + + test('respects Retry-After header on 429 (lowercase)', () => { + const error = { response: { status: 429, headers: { 'retry-after': '7' } } }; + expect(client.computeRetryDelay(error, 1)).toBe(7000); + }); + + test('respects Retry-After header on 429 (capitalized)', () => { + const error = { response: { status: 429, headers: { 'Retry-After': '12' } } }; + expect(client.computeRetryDelay(error, 1)).toBe(12000); + }); + + test('falls back to exponential backoff when Retry-After missing on 429', () => { + const error = { response: { status: 429, headers: {} } }; + expect(client.computeRetryDelay(error, 2)).toBe(2000); + }); + + test('ignores Retry-After on non-429 responses', () => { + const error = { response: { status: 503, headers: { 'retry-after': '99' } } }; + expect(client.computeRetryDelay(error, 1)).toBe(1000); + }); + }); + + describe('retry interceptor', () => { + const ORIGINAL_MAX = process.env.JIRA_CLI_MAX_RETRIES; + let sleepSpy; + + beforeEach(() => { + sleepSpy = jest.spyOn(JiraClient.prototype, 'sleep').mockResolvedValue(); + }); + + afterEach(() => { + sleepSpy.mockRestore(); + if (ORIGINAL_MAX === undefined) { + delete process.env.JIRA_CLI_MAX_RETRIES; + } else { + process.env.JIRA_CLI_MAX_RETRIES = ORIGINAL_MAX; + } + }); + + function makeAdapterClient() { + const c = new JiraClient(mockConfig); + const adapter = jest.fn(); + c.clientV3.defaults.adapter = adapter; + return { c, adapter }; + } + + function networkErrorAdapter(config, code) { + const err = new Error('Network error'); + err.code = code; + err.config = config; + return Promise.reject(err); + } + + function httpErrorAdapter(config, status, headers = {}, data = {}) { + const err = new Error('Request failed with status code ' + status); + err.config = config; + err.response = { status, headers, data, statusText: 'err', config }; + err.isAxiosError = true; + return Promise.reject(err); + } + + function okResponse(config) { + return Promise.resolve({ + status: 200, + statusText: 'OK', + headers: {}, + config, + request: {}, + data: { key: 'TEST-1' } + }); + } + + test('retries on 503 and eventually succeeds', async () => { + const { c, adapter } = makeAdapterClient(); + adapter + .mockImplementationOnce(config => httpErrorAdapter(config, 503)) + .mockImplementationOnce(config => okResponse(config)); + + const result = await c.getIssue('TEST-1'); + + expect(result).toEqual({ key: 'TEST-1' }); + expect(adapter).toHaveBeenCalledTimes(2); + expect(sleepSpy).toHaveBeenCalledWith(1000); + }); + + test('retries on 429 with Retry-After delay', async () => { + const { c, adapter } = makeAdapterClient(); + adapter + .mockImplementationOnce(config => httpErrorAdapter(config, 429, { 'retry-after': '3' })) + .mockImplementationOnce(config => okResponse(config)); + + await c.getIssue('TEST-1'); + + expect(sleepSpy).toHaveBeenCalledWith(3000); + }); + + test('retries on retryable network errors', async () => { + const { c, adapter } = makeAdapterClient(); + adapter + .mockImplementationOnce(config => networkErrorAdapter(config, 'ECONNRESET')) + .mockImplementationOnce(config => okResponse(config)); + + await c.getIssue('TEST-1'); + + expect(adapter).toHaveBeenCalledTimes(2); + }); + + test('gives up after exhausting max retries and rejects', async () => { + const { c, adapter } = makeAdapterClient(); + adapter.mockImplementation(config => httpErrorAdapter(config, 503)); + + await expect(c.getIssue('TEST-1')).rejects.toBeDefined(); + expect(adapter).toHaveBeenCalledTimes(4); // initial + 3 retries + }); + + test('does not retry on 404', async () => { + const { c, adapter } = makeAdapterClient(); + adapter.mockImplementation(config => httpErrorAdapter(config, 404)); + + await expect(c.getIssue('TEST-1')).rejects.toThrow('Resource not found'); + expect(adapter).toHaveBeenCalledTimes(1); + }); + + test('respects JIRA_CLI_MAX_RETRIES=0 (no retries)', async () => { + process.env.JIRA_CLI_MAX_RETRIES = '0'; + const { c, adapter } = makeAdapterClient(); + adapter.mockImplementation(config => httpErrorAdapter(config, 503)); + + await expect(c.getIssue('TEST-1')).rejects.toBeDefined(); + expect(adapter).toHaveBeenCalledTimes(1); + }); + + test('uses exponential backoff between retries', async () => { + const { c, adapter } = makeAdapterClient(); + adapter.mockImplementation(config => httpErrorAdapter(config, 503)); + + await expect(c.getIssue('TEST-1')).rejects.toBeDefined(); + + expect(sleepSpy).toHaveBeenNthCalledWith(1, 1000); + expect(sleepSpy).toHaveBeenNthCalledWith(2, 2000); + expect(sleepSpy).toHaveBeenNthCalledWith(3, 4000); + }); + }); + }); });