diff --git a/src/linear.ts b/src/linear.ts index 8e1ef6d..0594e01 100644 --- a/src/linear.ts +++ b/src/linear.ts @@ -2,11 +2,59 @@ import { mapLinearError, networkError, AxiError } from './errors.js'; export const LINEAR_API_URL = 'https://api.linear.app/graphql'; +// --------------------------------------------------------------------------- +// Rate-limit retry (#30) +// --------------------------------------------------------------------------- + +/** + * On HTTP 429, linearRequest re-sends the request up to this many times (3 + * attempts total) before surfacing mapLinearError's structured RATE_LIMITED + * error — the retry is transport-level and bounded, so no caller or flag + * knows it exists. + */ +const RATE_LIMIT_MAX_RETRIES = 2; + +/** An honored Retry-After is clamped into [1s, 60s] so a hostile/garbage + * header cannot stall the CLI. */ +const RETRY_DELAY_MIN_MS = 1_000; +const RETRY_DELAY_MAX_MS = 60_000; + +function sleep(ms: number): Promise { + return new Promise((resolve) => setTimeout(resolve, ms)); +} + +/** + * How long to wait before retry attempt `retry` (0-based) after a 429: the + * response's `Retry-After` header when it parses as positive delay-seconds + * (clamped to [1s, 60s]), else a bounded exponential fallback (1s, 2s, ...). + * + * mapLinearError derives RATE_LIMITED from the HTTP status alone, so HTTP 429 + * is the only retry signal here — a GraphQL-level error (HTTP 200 + `errors`) + * is never rate-limit-typed by mapLinearError and is never retried. + */ +function rateLimitRetryDelayMs(response: Response, retry: number): number { + const raw = response.headers?.get?.('retry-after'); + const seconds = Number(raw); + if (Number.isFinite(seconds) && seconds > 0) { + return Math.min( + Math.max(seconds * 1_000, RETRY_DELAY_MIN_MS), + RETRY_DELAY_MAX_MS, + ); + } + // Missing/invalid Retry-After (null, "", "0", HTTP-date form, garbage). + return Math.min(RETRY_DELAY_MIN_MS * 2 ** retry, RETRY_DELAY_MAX_MS); +} + /** * Raw transport: POST a GraphQL document to Linear with the API key as the * Authorization header. Returns the parsed `data` on success; throws an AxiError * (translated from HTTP/GraphQL status) otherwise. * + * HTTP 429 responses are retried with bounded backoff honoring Retry-After + * (see rateLimitRetryDelayMs); once retries are exhausted — or a non-429 + * response arrives, including a non-429 after a 429 — the error path below + * surfaces mapLinearError's structured error unchanged. + * * Set LINEAR_AXI_DEBUG=1 to dump the raw response body to stderr on error, * which makes query-shape mistakes debuggable in one round trip. */ @@ -16,17 +64,21 @@ export async function linearRequest( variables: Record = {}, ): Promise { let response: Response; - try { - response = await fetch(LINEAR_API_URL, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - Authorization: apiKey, - }, - body: JSON.stringify({ query, variables }), - }); - } catch (cause) { - throw networkError(cause); + for (let retry = 0; ; retry++) { + try { + response = await fetch(LINEAR_API_URL, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + Authorization: apiKey, + }, + body: JSON.stringify({ query, variables }), + }); + } catch (cause) { + throw networkError(cause); + } + if (response.status !== 429 || retry >= RATE_LIMIT_MAX_RETRIES) break; + await sleep(rateLimitRetryDelayMs(response, retry)); } let body: unknown; diff --git a/test/rate-limit-retry.test.ts b/test/rate-limit-retry.test.ts new file mode 100644 index 0000000..18231bc --- /dev/null +++ b/test/rate-limit-retry.test.ts @@ -0,0 +1,301 @@ +// Tests for rate-limit retry in linearRequest (#30): HTTP 429 responses are +// retried with bounded backoff honoring Retry-After, then the SAME structured +// RATE_LIMITED AxiError mapLinearError already produces is surfaced. +// +// Network-free per the fetch-stub pattern from comment-update-delete.test.ts / +// description-file.test.ts (vi.stubGlobal('fetch', ...) + Response-like +// objects + afterEach unstub), with one addition for the delays: fake timers. +// Only setTimeout is faked (`toFake: ['setTimeout']`) so Date.now() stays REAL +// — every backoff test asserts it finished in real milliseconds, proving no +// actual sleep ran, while vi.advanceTimersByTimeAsync advances the logical +// clock one millisecond at a time to pin the exact honored delay boundaries. +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { linearRequest } from '../src/linear.js'; +import { AxiError, mapLinearError, exitCodeForError } from '../src/errors.js'; + +const FAKE_KEY = 'lin_api_fake'; +const QUERY = 'query { viewer { id name email } }'; +const VIEWER = { viewer: { id: 'u-1', name: 'Ada', email: 'ada@example.com' } }; + +function okResponse(data: unknown): Response { + return { + ok: true, + status: 200, + headers: new Headers(), + json: async () => ({ data }), + } as Response; +} + +/** + * A 429 with a real Headers instance — the retry reads + * `headers.get('retry-after')` off the mocked Response, so the stub must be + * Headers-like. Omit `retryAfterSeconds` for the missing-header fallback. + */ +function rateLimitedResponse(retryAfterSeconds?: number): Response { + const headers = new Headers(); + if (retryAfterSeconds !== undefined) { + headers.set('retry-after', String(retryAfterSeconds)); + } + return { + ok: false, + status: 429, + headers, + json: async () => ({ errors: [{ message: 'Too many requests' }] }), + } as Response; +} + +/** A non-429 HTTP failure (401/500/...) with a Headers-like headers object. */ +function httpErrorResponse(status: number): Response { + return { + ok: false, + status, + headers: new Headers(), + json: async () => ({ errors: [{ message: `HTTP ${status}` }] }), + } as Response; +} + +/** + * Stub global fetch to answer with `responses` in order; the last one repeats + * for any further calls. Returns the mock for exact call-count assertions. + */ +function stubSequencedFetch(...responses: Response[]) { + let call = 0; + const fetchMock = vi.fn( + async (_url: string | URL | Request, _init?: RequestInit) => { + const response = responses[Math.min(call, responses.length - 1)]!; + call += 1; + return response; + }, + ); + vi.stubGlobal('fetch', fetchMock); + return fetchMock; +} + +afterEach(() => { + vi.useRealTimers(); + vi.unstubAllGlobals(); +}); + +describe('429 then success (Retry-After honored)', () => { + it('retries once after the Retry-After delay and succeeds', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const startedAt = Date.now(); // real clock — only setTimeout is faked + const fetchMock = stubSequencedFetch( + rateLimitedResponse(2), + okResponse(VIEWER), + ); + + const pending = linearRequest(FAKE_KEY, QUERY); + // One millisecond before the honored 2s Retry-After: still waiting. + await vi.advanceTimersByTimeAsync(2_000 - 1); + expect(fetchMock).toHaveBeenCalledTimes(1); + // At exactly 2s the retry fires and the 200 resolves the request. + await vi.advanceTimersByTimeAsync(1); + const data = await pending; + + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(data).toEqual(VIEWER); + // 2s of logical backoff, but the test never actually slept. + expect(Date.now() - startedAt).toBeLessThan(500); + }); + + it('sends the same GraphQL document and variables on the retry', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const variables = { id: 'LIN-1' }; + const fetchMock = stubSequencedFetch( + rateLimitedResponse(1), + okResponse(VIEWER), + ); + + const pending = linearRequest(FAKE_KEY, QUERY, variables); + await vi.advanceTimersByTimeAsync(1_000); + await pending; + + expect(fetchMock).toHaveBeenCalledTimes(2); + const [firstUrl, firstInit] = fetchMock.mock.calls[0]!; + const [retryUrl, retryInit] = fetchMock.mock.calls[1]!; + expect(retryUrl).toBe(firstUrl); + expect(retryInit?.body).toBe(firstInit?.body); + expect(retryInit?.headers).toEqual(firstInit?.headers); + }); + + it('clamps an oversized Retry-After to 60s', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const fetchMock = stubSequencedFetch( + rateLimitedResponse(3_600), + okResponse(VIEWER), + ); + + const pending = linearRequest(FAKE_KEY, QUERY); + // Retry-After: 3600 must not be honored literally — the clamp caps it at + // 60s, so nothing fires before t=60_000. + await vi.advanceTimersByTimeAsync(60_000 - 1); + expect(fetchMock).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + await pending; + expect(fetchMock).toHaveBeenCalledTimes(2); + }); +}); + +describe('429 without a usable Retry-After (exponential fallback)', () => { + it('falls back to 1s then 2s when the header is missing', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const fetchMock = stubSequencedFetch( + rateLimitedResponse(), // no Retry-After + rateLimitedResponse(), // no Retry-After + okResponse(VIEWER), + ); + + const pending = linearRequest(FAKE_KEY, QUERY); + // First backoff: fallback 1s (not the 60s a missing header could imply). + await vi.advanceTimersByTimeAsync(999); + expect(fetchMock).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + expect(fetchMock).toHaveBeenCalledTimes(2); + // Second backoff: fallback doubles to 2s. + await vi.advanceTimersByTimeAsync(1_999); + expect(fetchMock).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1); + const data = await pending; + + expect(fetchMock).toHaveBeenCalledTimes(3); + expect(data).toEqual(VIEWER); + }); + + it('treats a non-numeric or zero Retry-After as missing', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const garbage = rateLimitedResponse(); + garbage.headers.set('retry-after', 'soon'); + const zero = rateLimitedResponse(0); + const fetchMock = stubSequencedFetch(garbage, zero, okResponse(VIEWER)); + + const pending = linearRequest(FAKE_KEY, QUERY); + // 'soon' does not parse as delay-seconds -> 1s exponential fallback. + await vi.advanceTimersByTimeAsync(999); + expect(fetchMock).toHaveBeenCalledTimes(1); + await vi.advanceTimersByTimeAsync(1); + // Retry-After: 0 is not a positive delay -> 2s fallback (not "immediately"). + await vi.advanceTimersByTimeAsync(1_999); + expect(fetchMock).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1); + await pending; + expect(fetchMock).toHaveBeenCalledTimes(3); + }); +}); + +describe('exhausted retries surface RATE_LIMITED', () => { + it('stops after 3 attempts and throws mapLinearError’s 429 mapping', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const startedAt = Date.now(); + const fetchMock = stubSequencedFetch( + rateLimitedResponse(1), + rateLimitedResponse(1), + rateLimitedResponse(1), + ); + + // Handler attached up-front: the promise rejects mid-advance, and a later + // .catch would briefly leave it unhandled. + const pending = linearRequest(FAKE_KEY, QUERY).catch((e: unknown) => e); + await vi.advanceTimersByTimeAsync(1_000); + expect(fetchMock).toHaveBeenCalledTimes(2); + await vi.advanceTimersByTimeAsync(1_000); + + const err = (await pending) as AxiError; + + // Exactly 3 attempts (1 initial + 2 retries), no fourth call. + expect(fetchMock).toHaveBeenCalledTimes(3); + // The surfaced error IS mapLinearError's RATE_LIMITED — same code, message, + // hint, and SDK exit code; retry adds nothing and changes nothing. + const expected = mapLinearError({ status: 429, body: null }); + expect(err).toBeInstanceOf(AxiError); + expect(err.code).toBe(expected.code); + expect(err.code).toBe('RATE_LIMITED'); + expect(err.message).toBe(expected.message); + expect(err.suggestions).toEqual(expected.suggestions); + expect(exitCodeForError(err)).toBe(exitCodeForError(expected)); + // 2s of logical backoff, zero real sleeping. + expect(Date.now() - startedAt).toBeLessThan(500); + }); +}); + +describe('non-429 failures are never retried', () => { + it('fails immediately with the 500 mapping when a 429 retry hits a 500', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const fetchMock = stubSequencedFetch( + rateLimitedResponse(1), + httpErrorResponse(500), + ); + + const pending = linearRequest(FAKE_KEY, QUERY).catch((e: unknown) => e); + await vi.advanceTimersByTimeAsync(1_000); + const err = (await pending) as AxiError; + + // The 429 was retried; the 500 was NOT — the last response's mapping wins. + expect(fetchMock).toHaveBeenCalledTimes(2); + expect(err.code).toBe('NETWORK_ERROR'); + expect(err.message).toContain('HTTP 500'); + }); + + it('does not retry a first-call 401 and sets up no timers', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const fetchMock = stubSequencedFetch(httpErrorResponse(401)); + + const err = (await linearRequest(FAKE_KEY, QUERY).catch( + (e: unknown) => e, + )) as AxiError; + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(err.code).toBe('AUTH_REQUIRED'); + expect(vi.getTimerCount()).toBe(0); + }); + + it('does not retry GraphQL-level errors (200 + errors array)', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const gqlError = okResponse(null); + // 200 with a top-level errors array — never rate-limit-typed by + // mapLinearError, so never a retry candidate. + const body = { errors: [{ message: 'GraphQL blew up' }] }; + const fetchMock = stubSequencedFetch({ + ...gqlError, + json: async () => body, + } as Response); + + const err = (await linearRequest(FAKE_KEY, QUERY).catch( + (e: unknown) => e, + )) as AxiError; + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(err.message).toContain('GraphQL blew up'); + expect(vi.getTimerCount()).toBe(0); + }); + + it('does not retry a network-level fetch failure', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const fetchMock = vi.fn(async () => { + throw new Error('ECONNRESET'); + }); + vi.stubGlobal('fetch', fetchMock); + + const err = (await linearRequest(FAKE_KEY, QUERY).catch( + (e: unknown) => e, + )) as AxiError; + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(err.code).toBe('NETWORK_ERROR'); + expect(err.message).toContain('ECONNRESET'); + expect(vi.getTimerCount()).toBe(0); + }); +}); + +describe('first-call success', () => { + it('returns immediately without setting up any timer', async () => { + vi.useFakeTimers({ toFake: ['setTimeout'] }); + const fetchMock = stubSequencedFetch(okResponse(VIEWER)); + + const data = await linearRequest(FAKE_KEY, QUERY); + + expect(fetchMock).toHaveBeenCalledTimes(1); + expect(data).toEqual(VIEWER); + expect(vi.getTimerCount()).toBe(0); + }); +});