diff --git a/src/client/http.ts b/src/client/http.ts index 6a4a5127..08acc753 100644 --- a/src/client/http.ts +++ b/src/client/http.ts @@ -130,7 +130,14 @@ export async function request(config: Config, opts: RequestOpts): Promise 0) { + const nearestMs = Math.round(timeoutMs); + // Remove floating-point noise around whole milliseconds before rounding + // genuine sub-millisecond budgets down, never extending them to a full ms. + timeoutMs = Math.abs(timeoutMs - nearestMs) <= Number.EPSILON * timeoutMs + ? nearestMs : Math.floor(timeoutMs); + } const streamAbortController = opts.stream ? new AbortController() : undefined; const headerTimeout = streamAbortController @@ -165,7 +172,14 @@ export async function request(config: Config, opts: RequestOpts): Promise(config: Config, opts: RequestOpts): Promise let data: T & { base_resp?: { status_code?: number; status_msg?: string } }; try { data = (await res.json()) as T & { base_resp?: { status_code?: number; status_msg?: string } }; - } catch { + } catch (error) { + if (error instanceof Error && (error.name === 'AbortError' || error.name === 'TimeoutError')) { + throw error; + } const contentType = res.headers.get('content-type') || ''; throw new CLIError( `API returned non-JSON response (${contentType || 'unknown type'}). Server may be experiencing issues.`, diff --git a/src/polling/poll.ts b/src/polling/poll.ts index 7619ea48..54410ece 100644 --- a/src/polling/poll.ts +++ b/src/polling/poll.ts @@ -21,8 +21,14 @@ export async function poll(config: Config, opts: PollOptions): Promise { if (!config.quiet) spinner.start(); try { - while (Date.now() < deadline) { - const data = await requestJson(config, { url: opts.url }); + while (true) { + const remainingMs = deadline - Date.now(); + if (remainingMs <= 0) break; + + // Cap the HTTP wait by the polling budget remaining at this point, + // without increasing a shorter configured per-request timeout. + const timeout = Math.min(config.timeout, remainingMs / 1000); + const data = await requestJson(config, { url: opts.url, timeout }); if (opts.getStatus && !config.quiet) { spinner.update(`Status: ${opts.getStatus(data)}`); diff --git a/test/client/http.test.ts b/test/client/http.test.ts index 0e577672..30a8f0b3 100644 --- a/test/client/http.test.ts +++ b/test/client/http.test.ts @@ -68,6 +68,102 @@ describe('HTTP client', () => { expect(result.echo).toEqual({ hello: 'world' }); }); + it.each([ + [1.001, 1001], + [2.007, 2007], + [2.0075, 2007], + [0.0015, 1], + [0.0005, 0], + [0, 0], + ])('converts a %s second timeout to %s whole milliseconds', async (seconds, milliseconds) => { + const timeout = spyOn(AbortSignal, 'timeout'); + const fetchSpy = spyOn(globalThis, 'fetch').mockImplementation((async () => + jsonResponse({ result: 'ok' })) as unknown as typeof fetch); + + try { + await requestJson(makeConfig('https://example.com'), { + url: 'https://example.com/test', + timeout: seconds, + noAuth: true, + }); + + expect(timeout).toHaveBeenCalledWith(milliseconds); + } finally { + fetchSpy.mockRestore(); + timeout.mockRestore(); + } + }); + + it('keeps malformed JSON responses as general API errors', async () => { + server = createMockServer({ + routes: { '/v1/test': () => new Response('not json') }, + }); + + await expect(requestJson(makeConfig(server.url), { + url: `${server.url}/v1/test`, + })).rejects.toMatchObject({ + exitCode: 1, + message: expect.stringContaining('API returned non-JSON response'), + }); + }); + + it.each([-1, -0.0005])('leaves a negative %s second timeout to platform validation', async (seconds) => { + const timeout = spyOn(AbortSignal, 'timeout') + .mockImplementation(() => new AbortController().signal); + const fetchSpy = spyOn(globalThis, 'fetch').mockImplementation((async () => + jsonResponse({ result: 'ok' })) as unknown as typeof fetch); + + try { + await requestJson(makeConfig('https://example.com'), { + url: 'https://example.com/test', + timeout: seconds, + noAuth: true, + }); + + expect(timeout).toHaveBeenCalledWith(seconds * 1000); + } finally { + fetchSpy.mockRestore(); + timeout.mockRestore(); + } + }); + + it('keeps non-JSON HTTP 500 responses as API errors', async () => { + server = createMockServer({ + routes: { '/v1/test': () => new Response('upstream failure', { status: 500 }) }, + }); + + await expect(requestJson(makeConfig(server.url), { + url: `${server.url}/v1/test`, + })).rejects.toMatchObject({ + exitCode: 1, + message: expect.stringContaining('HTTP 500'), + }); + }); + + it.each([ + [200, 'AbortError'], + [200, 'TimeoutError'], + [500, 'AbortError'], + [500, 'TimeoutError'], + ])('preserves HTTP %s body %s', async (status, name) => { + const error = new DOMException('request stopped', name); + const fetchSpy = spyOn(globalThis, 'fetch').mockImplementation((async () => ({ + ok: status === 200, + status, + headers: new Headers({ 'Content-Type': 'application/json' }), + json: async () => { throw error; }, + })) as unknown as typeof fetch); + + try { + await expect(requestJson(makeConfig('https://example.com'), { + url: 'https://example.com/test', + noAuth: true, + })).rejects.toBe(error); + } finally { + fetchSpy.mockRestore(); + } + }); + it('throws CLIError on 401', async () => { server = createMockServer({ routes: { diff --git a/test/polling/poll.test.ts b/test/polling/poll.test.ts index efb5db6d..6a485de1 100644 --- a/test/polling/poll.test.ts +++ b/test/polling/poll.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, mock, afterEach } from 'bun:test'; +import { describe, it, expect, mock, afterEach, spyOn } from 'bun:test'; import type { Config } from '../../src/config/schema'; // Silence the spinner — purely visual, no impact on logic. @@ -163,4 +163,117 @@ describe('poll', () => { expect(performance.now() - started).toBeLessThan(1000); }); + + it('aborts a pending status request at the polling deadline', async () => { + let signal: AbortSignal | undefined; + let callCount = 0; + setFetch(async (_input: unknown, init: RequestInit) => { + callCount++; + signal = init.signal ?? undefined; + return await new Promise((_resolve, reject) => { + signal!.addEventListener('abort', () => reject(signal!.reason), { once: true }); + }); + }); + const { poll } = await import('../../src/polling/poll'); + const started = performance.now(); + + await expect(poll({ ...baseConfig, timeout: 1 }, { + url: 'https://api.mmx.io/poll', + intervalSec: 0, + timeoutSec: 0.05, + isComplete: () => false, + isFailed: () => false, + })).rejects.toMatchObject({ name: 'TimeoutError' }); + + expect(signal?.aborted).toBe(true); + expect(callCount).toBe(1); + expect(performance.now() - started).toBeLessThan(500); + }); + + it.each([ + [100, 80, 20], + [3000, 993, 2007], + [2000, 999, 1001], + ])('reduces a %s ms polling budget by %s ms to %s ms', async (budget, elapsed, remaining) => { + let now = 1_000; + let callCount = 0; + const clock = spyOn(Date, 'now').mockImplementation(() => now); + const timeout = spyOn(AbortSignal, 'timeout'); + setFetch(async () => { + callCount++; + if (callCount === 1) now += elapsed; + return jsonRes({ status: callCount === 1 ? 'Processing' : 'Success' }); + }); + + try { + const { poll } = await import('../../src/polling/poll'); + await poll(baseConfig, { + url: 'https://api.mmx.io/poll', + intervalSec: 0, + timeoutSec: budget / 1000, + isComplete: (data) => (data as { status: string }).status === 'Success', + isFailed: () => false, + }); + + expect(timeout.mock.calls.map(([milliseconds]) => milliseconds)).toEqual([budget, remaining]); + expect(callCount).toBe(2); + } finally { + timeout.mockRestore(); + clock.mockRestore(); + } + }); + + it.each([0.01, 2.007])('preserves a shorter %s second per-request timeout', async (seconds) => { + const timeout = spyOn(AbortSignal, 'timeout'); + setFetch(async () => jsonRes({ status: 'Success' })); + + try { + const { poll } = await import('../../src/polling/poll'); + await poll({ ...baseConfig, timeout: seconds }, { + url: 'https://api.mmx.io/poll', + intervalSec: 0, + timeoutSec: 5, + isComplete: () => true, + isFailed: () => false, + }); + + expect(timeout).toHaveBeenCalledWith(Math.round(seconds * 1000)); + } finally { + timeout.mockRestore(); + } + }); + + it.each(['headers', 'body', 'error body'])('times out when real HTTP response %s stall', async (phase) => { + const server = Bun.serve({ + hostname: '127.0.0.1', + port: 0, + fetch() { + if (phase === 'headers') return new Promise(() => {}); + return new Response(new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('{"status":')); + }, + }), { + status: phase === 'error body' ? 500 : 200, + headers: { 'Content-Type': 'application/json' }, + }); + }, + }); + setFetch(originalFetch); + const started = performance.now(); + + try { + const { poll } = await import('../../src/polling/poll'); + await expect(poll({ ...baseConfig, timeout: 1 }, { + url: `http://127.0.0.1:${server.port}/poll`, + intervalSec: 0, + timeoutSec: 0.05, + isComplete: () => false, + isFailed: () => false, + })).rejects.toMatchObject({ name: 'TimeoutError' }); + expect(performance.now() - started).toBeLessThan(500); + } finally { + server.stop(true); + } + }); });