diff --git a/packages/cli/src/rest/__tests__/retry.spec.ts b/packages/cli/src/rest/__tests__/retry.spec.ts new file mode 100644 index 00000000..4a6c49fd --- /dev/null +++ b/packages/cli/src/rest/__tests__/retry.spec.ts @@ -0,0 +1,310 @@ +import { Readable } from 'node:stream' +import { describe, it, expect, vi } from 'vitest' +import axios, { AxiosError, AxiosResponse, InternalAxiosRequestConfig } from 'axios' +import { createRetryInterceptor, parseRetryAfter, RetryOptions } from '../retry.js' +import { + handleErrorResponse, + MiscellaneousError, + RequestTimeoutError, + ServerError, + ValidationError, +} from '../errors.js' +import { api as productionApi } from '../api.js' + +type Adapter = (config: InternalAxiosRequestConfig) => Promise + +function failWith (status: number, headers: Record = {}): Adapter { + return config => Promise.reject(new AxiosError( + `Request failed with status code ${status}`, + 'ERR_BAD_RESPONSE', + config, + {}, + { + status, + statusText: 'Error', + headers, + config, + data: 'Upstream error', + } as AxiosResponse, + )) +} + +function failWithCode (code: string): Adapter { + return config => Promise.reject(new AxiosError('connection error', code, config)) +} + +function succeedWith (data: any): Adapter { + return config => Promise.resolve({ + status: 200, + statusText: 'OK', + headers: {}, + config, + data, + }) +} + +// Mirrors the interceptor wiring of init() in api.ts: retry first (sees the +// raw AxiosError), error mapping second. A separate test below pins the +// production instance itself. +function createApi (options?: RetryOptions) { + const adapter = vi.fn() + const delays: number[] = [] + const api = axios.create({ adapter }) + api.interceptors.response.use(undefined, createRetryInterceptor(api, { + sleep: ms => { + delays.push(ms) + return Promise.resolve() + }, + random: () => 1, + ...options, + })) + api.interceptors.response.use( + response => response, + error => handleErrorResponse(error), + ) + return { api, adapter, delays } +} + +describe('createRetryInterceptor', () => { + it('retries a GET that received a 502 and returns the successful response', async () => { + const { api, adapter } = createApi() + adapter + .mockImplementationOnce(failWith(502)) + .mockImplementationOnce(succeedWith({ ok: true })) + + const response = await api.get('/test') + + expect(response.data).toEqual({ ok: true }) + expect(adapter).toHaveBeenCalledTimes(2) + }) + + it('gives up after 3 attempts and surfaces the mapped ServerError', async () => { + const { api, adapter } = createApi() + adapter.mockImplementation(failWith(502)) + + await expect(api.get('/test')).rejects.toThrowError(ServerError) + expect(adapter).toHaveBeenCalledTimes(3) + }) + + it('does not retry a POST by default', async () => { + const { api, adapter } = createApi() + adapter.mockImplementation(failWith(502)) + + await expect(api.post('/test', { name: 'test' })).rejects.toThrowError(ServerError) + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('retries a POST that opted in with checklyRetry', async () => { + const { api, adapter } = createApi() + adapter + .mockImplementationOnce(failWith(502)) + .mockImplementationOnce(succeedWith({ ok: true })) + + const response = await api.post('/test', { name: 'test' }, { checklyRetry: true }) + + expect(response.data).toEqual({ ok: true }) + expect(adapter).toHaveBeenCalledTimes(2) + // The replay re-runs the (idempotent) default JSON transform over the + // already-serialized body; the bytes on the wire must not change. + expect(adapter.mock.calls[1][0].data).toBe(adapter.mock.calls[0][0].data) + }) + + it('does not retry a 400', async () => { + const { api, adapter } = createApi() + adapter.mockImplementation(failWith(400)) + + await expect(api.get('/test')).rejects.toThrowError(ValidationError) + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('does not retry a 408 (long-poll callers own that cadence)', async () => { + const { api, adapter } = createApi() + adapter.mockImplementation(failWith(408)) + + await expect(api.get('/test')).rejects.toThrowError(RequestTimeoutError) + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('honors Retry-After on a 429', async () => { + const { api, adapter, delays } = createApi() + adapter + .mockImplementationOnce(failWith(429, { 'retry-after': '1' })) + .mockImplementationOnce(succeedWith({ ok: true })) + + await api.get('/test') + + expect(delays).toEqual([1000]) + expect(adapter).toHaveBeenCalledTimes(2) + }) + + it('honors a 429 Retry-After larger than the backoff cap, up to the Retry-After cap', async () => { + const { api, adapter, delays } = createApi() + adapter + .mockImplementationOnce(failWith(429, { 'retry-after': '10' })) + .mockImplementationOnce(succeedWith({ ok: true })) + + await api.get('/test') + + expect(delays).toEqual([10_000]) + expect(adapter).toHaveBeenCalledTimes(2) + }) + + it('does not retry a 429 whose Retry-After exceeds the Retry-After cap', async () => { + const { api, adapter } = createApi() + adapter.mockImplementation(failWith(429, { 'retry-after': '30' })) + + await expect(api.get('/test')).rejects.toThrowError(MiscellaneousError) + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('honors a 5xx Retry-After between the backoff cap and the Retry-After cap', async () => { + const { api, adapter, delays } = createApi() + adapter + .mockImplementationOnce(failWith(503, { 'retry-after': '5' })) + .mockImplementationOnce(succeedWith({ ok: true })) + + await api.get('/test') + + expect(delays).toEqual([5000]) + expect(adapter).toHaveBeenCalledTimes(2) + }) + + it('retries a 503 whose Retry-After exceeds the Retry-After cap using backoff instead', async () => { + const { api, adapter, delays } = createApi() + adapter + .mockImplementationOnce(failWith(503, { 'retry-after': '30' })) + .mockImplementationOnce(succeedWith({ ok: true })) + + await api.get('/test') + + expect(delays).toEqual([250]) + expect(adapter).toHaveBeenCalledTimes(2) + }) + + it('does not retry stream responses', async () => { + const { api, adapter } = createApi() + adapter.mockImplementation(failWith(502)) + + await expect(api.get('/test', { responseType: 'stream' })).rejects.toThrowError(ServerError) + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('does not retry a stream request body even when opted in', async () => { + const { api, adapter } = createApi() + adapter.mockImplementation(failWith(502)) + + const body = Readable.from(['payload']) + await expect(api.post('/test', body, { checklyRetry: true })).rejects.toThrowError(ServerError) + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('does not retry a request that was canceled', async () => { + const { api, adapter } = createApi() + adapter.mockImplementation(failWithCode('ERR_CANCELED')) + + await expect(api.get('/test')).rejects.toThrow() + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('does not retry when the signal aborts during the backoff sleep', async () => { + const controller = new AbortController() + const { api, adapter } = createApi({ + sleep: () => { + controller.abort() + return Promise.resolve() + }, + }) + adapter.mockImplementation(failWith(502)) + + await expect(api.get('/test', { signal: controller.signal })).rejects.toThrowError(ServerError) + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('stops sleeping as soon as the signal aborts', async () => { + const controller = new AbortController() + // A sleep that never resolves: only the abort can end the backoff wait. + const { api, adapter } = createApi({ sleep: () => new Promise(() => {}) }) + adapter.mockImplementation(failWith(502)) + + const request = api.get('/test', { signal: controller.signal }) + const assertion = expect(request).rejects.toThrowError(ServerError) + await new Promise(resolve => setTimeout(resolve, 10)) + controller.abort() + + await assertion + expect(adapter).toHaveBeenCalledTimes(1) + }) + + it('retries connection-level errors with no response', async () => { + const { api, adapter } = createApi() + adapter + .mockImplementationOnce(failWithCode('ECONNRESET')) + .mockImplementationOnce(succeedWith({ ok: true })) + + const response = await api.get('/test') + + expect(response.data).toEqual({ ok: true }) + expect(adapter).toHaveBeenCalledTimes(2) + }) + + it('backs off exponentially up to the per-delay cap', async () => { + const { api, adapter, delays } = createApi({ baseDelayMs: 1500 }) + adapter.mockImplementation(failWith(502)) + + await expect(api.get('/test')).rejects.toThrowError(ServerError) + expect(delays).toEqual([1500, 2000]) + }) + + it('re-runs request interceptors on each retry', async () => { + const { api, adapter } = createApi() + let requestCount = 0 + api.interceptors.request.use(config => { + requestCount += 1 + config.headers['x-request-count'] = String(requestCount) + return config + }) + adapter + .mockImplementationOnce(failWith(502)) + .mockImplementationOnce(succeedWith({ ok: true })) + + await api.get('/test') + + expect(adapter.mock.calls[0][0].headers['x-request-count']).toBe('1') + expect(adapter.mock.calls[1][0].headers['x-request-count']).toBe('2') + }) +}) + +describe('parseRetryAfter', () => { + it('parses delay-seconds', () => { + expect(parseRetryAfter('2')).toBe(2000) + expect(parseRetryAfter('0')).toBe(0) + }) + + it('parses an HTTP-date relative to now', () => { + const value = parseRetryAfter(new Date(Date.now() + 5000).toUTCString()) + expect(value).toBeGreaterThan(3000) + expect(value).toBeLessThanOrEqual(5000) + }) + + it('returns 0 for an HTTP-date in the past', () => { + expect(parseRetryAfter(new Date(Date.now() - 5000).toUTCString())).toBe(0) + }) + + it('returns undefined for absent or malformed values', () => { + expect(parseRetryAfter(undefined)).toBeUndefined() + expect(parseRetryAfter('')).toBeUndefined() + expect(parseRetryAfter('soon')).toBeUndefined() + expect(parseRetryAfter('-1')).toBeUndefined() + }) +}) + +describe('production api instance', () => { + it('is wired to retry before mapping errors', async () => { + const adapter = vi.fn(failWith(502)) + + // The per-request adapter leaves the shared instance untouched for other + // tests. Uses real (jittered) backoff delays: worst case ~750ms. + await expect(productionApi.get('/test', { adapter })).rejects.toThrowError(ServerError) + expect(adapter).toHaveBeenCalledTimes(3) + }) +}) diff --git a/packages/cli/src/rest/api.ts b/packages/cli/src/rest/api.ts index ae04086e..5177c260 100644 --- a/packages/cli/src/rest/api.ts +++ b/packages/cli/src/rest/api.ts @@ -32,6 +32,7 @@ import AlertNotifications from './alert-notifications.js' import Rca from './rca.js' import Cancel from './cancel.js' import { handleErrorResponse, UnauthorizedError } from './errors.js' +import { createRetryInterceptor } from './retry.js' import { detectOperator } from '../helpers/cli-mode.js' export function getDefaults () { @@ -102,6 +103,11 @@ function init (): AxiosInstance { api.interceptors.request.use(requestInterceptor) + // Must be registered before the error-mapping interceptor: this handler + // needs the raw AxiosError, and its resolved retries flow into the next + // interceptor's fulfilled handler. + api.interceptors.response.use(undefined, createRetryInterceptor(api)) + api.interceptors.response.use( response => response, responseErrorInterceptor, diff --git a/packages/cli/src/rest/retry.ts b/packages/cli/src/rest/retry.ts new file mode 100644 index 00000000..7cfcf454 --- /dev/null +++ b/packages/cli/src/rest/retry.ts @@ -0,0 +1,256 @@ +import { setTimeout as delay } from 'node:timers/promises' +import { AxiosError, AxiosInstance, AxiosResponse, GenericAbortSignal, isAxiosError } from 'axios' +import Debug from 'debug' + +const debug = Debug('checkly:cli:rest:retry') + +declare module 'axios' { + export interface AxiosRequestConfig { + /** + * Opts a mutating request (POST/PUT/PATCH/DELETE) into transient-error + * retries. Only set this on endpoints known to be idempotent: a mutation + * that received a gateway error may still have been applied upstream, and + * a retry would apply it again. + * + * Only honored by the shared REST instance created in api.ts — bare + * axios calls (e.g. presigned-URL asset downloads) register no retry + * interceptor, so the flag is inert there. + * + * Do not combine with a custom transformRequest: a retry replays the + * already-transformed body through the transform again, and reuses the + * previous attempt's Content-Length header, so a non-idempotent or + * length-changing transform silently corrupts or truncates the body on + * the wire. (The default JSON transform is idempotent, and + * stream-producing transforms are blocked by the stream-body guard.) + */ + checklyRetry?: boolean + /** Internal: attempts already made for this logical request. */ + checklyRetryAttempt?: number + } +} + +export interface RetryOptions { + /** Total attempts including the initial request. */ + attempts?: number + /** Base delay for exponential backoff, in milliseconds. */ + baseDelayMs?: number + /** Cap for a single jittered backoff delay, in milliseconds. */ + maxBackoffDelayMs?: number + /** Largest server-provided Retry-After honored verbatim, in milliseconds. */ + maxRetryAfterMs?: number + /** + * Overridable in tests to avoid real sleeps. The abort signal fires when + * the wait is abandoned early; a sleep should cancel its timer then. + */ + sleep?: (ms: number, abortSignal?: AbortSignal) => Promise + /** Overridable in tests to make jitter deterministic. */ + random?: () => number +} + +const DEFAULT_ATTEMPTS = 3 +const DEFAULT_BASE_DELAY_MS = 250 +const DEFAULT_MAX_BACKOFF_DELAY_MS = 2000 +// An explicit server hint is trusted for longer than our own backoff would +// wait, but still bounded so interactive commands cannot stall indefinitely. +const DEFAULT_MAX_RETRY_AFTER_MS = 10_000 + +// The transient gateway class only. 408 is deliberately excluded: the API +// uses it to end long-poll requests, and the polling callers own that retry +// cadence. Plain 500 is also a conscious exclusion, even though brief API +// degradations have been observed to produce the occasional 500 alongside +// 502s: a 500 is just as often a deterministic application failure, and +// retrying those multiplies load and latency for no benefit. +const RETRYABLE_STATUS_CODES = new Set([429, 502, 503, 504]) + +// Connection-level failures where no response arrived. Safe to retry for +// idempotent requests even if the request reached the server. +const RETRYABLE_ERROR_CODES = new Set([ + 'ECONNRESET', + 'ETIMEDOUT', + 'ECONNABORTED', + 'EAI_AGAIN', + 'ERR_NETWORK', + 'EPIPE', +]) + +const IDEMPOTENT_METHODS = new Set(['get', 'head', 'options']) + +// The timer must stay ref'd while the wait is live: the caller is awaiting +// the retried request, and an unref'd timer would let the event loop drain +// mid-backoff, silently exiting the process with the request unresolved. +// When the wait is abandoned early (request aborted), the timer is cancelled +// through the signal so it cannot keep the process alive for the remainder +// of the delay either. +function defaultSleep (ms: number, abortSignal?: AbortSignal): Promise { + return delay(ms, undefined, { signal: abortSignal }).catch((err: unknown) => { + // A cancelled backoff sleep is not an error; the caller re-checks the + // request's own signal after waking up. + if ((err as Error)?.name !== 'AbortError') { + throw err + } + }) +} + +// Resolves when the sleep finishes or the signal aborts, whichever comes +// first, so an aborted caller is not kept waiting out the backoff delay. +async function sleepUnlessAborted ( + sleep: (ms: number, abortSignal?: AbortSignal) => Promise, + ms: number, + signal: GenericAbortSignal | undefined, +): Promise { + if (signal?.aborted) { + return + } + + // A real AbortController of our own (config.signal is only a + // GenericAbortSignal, which node's timers reject): aborted once the race + // settles, so a lost sleep timer cannot linger and keep the process alive. + const sleepAbort = new AbortController() + let onAbort: (() => void) | undefined + try { + await Promise.race([ + sleep(ms, sleepAbort.signal), + new Promise(resolve => { + onAbort = resolve + signal?.addEventListener?.('abort', onAbort) + }), + ]) + } finally { + sleepAbort.abort() + if (onAbort !== undefined) { + signal?.removeEventListener?.('abort', onAbort) + } + } +} + +/** + * Parses a Retry-After header value (delay-seconds or HTTP-date) into a + * delay in milliseconds, or undefined if the value is absent or malformed. + */ +export function parseRetryAfter (value: unknown): number | undefined { + if (typeof value !== 'string' || value === '') { + return undefined + } + + const seconds = Number(value) + if (!Number.isNaN(seconds)) { + return seconds >= 0 ? seconds * 1000 : undefined + } + + const date = Date.parse(value) + if (Number.isNaN(date)) { + return undefined + } + + return Math.max(0, date - Date.now()) +} + +function isStream (value: any): boolean { + return value !== null && typeof value === 'object' && typeof value.pipe === 'function' +} + +/** + * Whether the request itself may be replayed: idempotent method (or explicit + * opt-in), not aborted, and no single-use stream involved on either side. + */ +function isRetryableRequest (config: NonNullable): boolean { + if (config.signal?.aborted) { + return false + } + + // Streamed responses (e.g. the deployment progress SSE stream) may have + // been partially consumed by the caller and cannot be transparently + // reissued. Streamed request bodies (e.g. gzipped payloads and file + // uploads) are single-use and already drained by the failed attempt. If + // upload resilience is wanted later, it belongs at the call site, which + // can re-create the stream (e.g. re-open the file) before retrying. + if (config.responseType === 'stream' || isStream(config.data)) { + return false + } + + const method = (config.method ?? 'get').toLowerCase() + return IDEMPOTENT_METHODS.has(method) || config.checklyRetry === true +} + +/** + * Whether the failure looks transient: a retryable gateway status, or a + * connection-level error with no response at all. + */ +function isRetryableFailure (error: AxiosError): boolean { + if (error.response !== undefined) { + return RETRYABLE_STATUS_CODES.has(error.response.status) + } + + return error.code !== undefined && RETRYABLE_ERROR_CODES.has(error.code) +} + +/** + * Creates an axios response rejection handler that retries transient upstream + * failures with bounded, jittered exponential backoff. Register it before the + * error-mapping interceptor so it sees the raw AxiosError; a successful retry + * resolves into the next interceptor's fulfilled handler, and an exhausted + * one rethrows into its rejection handler. + */ +export function createRetryInterceptor (api: AxiosInstance, options?: RetryOptions) { + const { + attempts = DEFAULT_ATTEMPTS, + baseDelayMs = DEFAULT_BASE_DELAY_MS, + maxBackoffDelayMs = DEFAULT_MAX_BACKOFF_DELAY_MS, + maxRetryAfterMs = DEFAULT_MAX_RETRY_AFTER_MS, + sleep = defaultSleep, + random = Math.random, + } = options ?? {} + + return async function retryInterceptor (error: unknown): Promise { + if (!isAxiosError(error) || error.config === undefined) { + throw error + } + + const { config } = error + + if (!isRetryableRequest(config) || !isRetryableFailure(error)) { + throw error + } + + const attempt = config.checklyRetryAttempt ?? 1 + if (attempt >= attempts) { + throw error + } + + const status = error.response?.status + const retryAfterMs = parseRetryAfter(error.response?.headers?.['retry-after']) + + let delayMs: number + if (retryAfterMs !== undefined && retryAfterMs <= maxRetryAfterMs) { + delayMs = retryAfterMs + } else if (status === 429 && retryAfterMs !== undefined) { + // The server asked for a longer wait than our delay budget allows. + // Retrying earlier than requested would likely just get throttled + // again, so give up instead. + throw error + } else { + // Full jitter: a uniformly random delay up to the exponential bound + // spreads out concurrent clients hitting the same degraded gateway. + delayMs = Math.round(random() * Math.min(maxBackoffDelayMs, baseDelayMs * 2 ** (attempt - 1))) + } + + debug( + 'Retrying %s %s after %s (attempt %d of %d, delay %dms)', + (config.method ?? 'get').toUpperCase(), + config.url, + status ?? error.code, + attempt + 1, + attempts, + delayMs, + ) + + await sleepUnlessAborted(sleep, delayMs, config.signal) + + if (config.signal?.aborted) { + throw error + } + + config.checklyRetryAttempt = attempt + 1 + return api.request(config) + } +}