diff --git a/src/api/client.ts b/src/api/client.ts index 4bde605..8b3f821 100644 --- a/src/api/client.ts +++ b/src/api/client.ts @@ -78,7 +78,21 @@ export class MonidAPI { return undefined as T; } - const data = await res.json() as T & ApiErrorResponse; + // Not every response on this socket comes from the API. A gateway, proxy + // or load balancer in front of it answers 5xx with an HTML error page, + // and an unparsed body must not cost us the status: parsing FIRST turned + // a plain 502 into `Unexpected token '<', "..." is not valid JSON` + // and threw the status away, so an upstream blip read as a broken CLI. + // Parse defensively and let the status drive the error instead. + const raw = await res.text(); + let data: (T & ApiErrorResponse) | undefined; + let parsed = false; + try { + data = JSON.parse(raw) as T & ApiErrorResponse; + parsed = true; + } catch { + // Body is not JSON — handled below, by status first. + } if (!res.ok) { const message = @@ -87,7 +101,17 @@ export class MonidAPI { throw new MonidError(code, message, res.status); } - return data; + // 2xx that is not JSON is still a broken response — name it, and show + // enough of the body to tell a captive portal from a bad deploy. + if (!parsed) { + throw new MonidError( + 'INVALID_RESPONSE', + `Expected JSON from ${url} but got ${describeBody(res, raw)}`, + res.status, + ); + } + + return data as T; } async discover( @@ -218,6 +242,21 @@ export class MonidAPI { } } +/** + * Describe a body that failed to parse as JSON, for an error message. + * Reports the declared content type and a short, whitespace-collapsed + * snippet — enough to identify an HTML error page or a captive portal + * without dumping a whole document into the terminal. + */ +function describeBody(res: Response, raw: string): string { + const type = res.headers.get('content-type')?.split(';')[0]?.trim(); + const snippet = raw.trim().replace(/\s+/g, ' ').slice(0, 80); + if (!snippet) { + return type ? `an empty ${type} body` : 'an empty body'; + } + return `${type ?? 'an unknown content type'}: ${snippet}${raw.trim().length > 80 ? '…' : ''}`; +} + function statusToCode(status: number): string { switch (status) { case 401: diff --git a/test/api/client.test.ts b/test/api/client.test.ts new file mode 100644 index 0000000..417c0aa --- /dev/null +++ b/test/api/client.test.ts @@ -0,0 +1,92 @@ +import { describe, it, expect, afterEach } from 'bun:test'; +import { MonidAPI } from '../../src/api/client.js'; +import { MonidError } from '../../src/utils/error.js'; + +const realFetch = globalThis.fetch; +afterEach(() => { + globalThis.fetch = realFetch; +}); + +/** Answer the next request with this exact body, status and content type. */ +function stubFetch(body: string, status: number, contentType: string): void { + globalThis.fetch = (async () => + new Response(body, { + status, + headers: { 'content-type': contentType }, + })) as typeof fetch; +} + +const api = new MonidAPI({ apiKey: 'test-key', baseUrl: 'https://api.example.test' }); + +/** The bare `` + CRLF error page a gateway returns — not the API. */ +const GATEWAY_HTML = '\r\n502 Bad Gateway\r\n...\r\n'; + +describe('MonidAPI request error handling', () => { + it('reports the HTTP status when a gateway answers 5xx with HTML', async () => { + // Regression: the body was parsed BEFORE the status was checked, so this + // surfaced as `Unexpected token '<', "..." is not valid JSON` and + // the 502 was lost entirely. + stubFetch(GATEWAY_HTML, 502, 'text/html'); + + const err = await api.whoami().then(() => null, (e: unknown) => e); + expect(err).toBeInstanceOf(MonidError); + expect((err as MonidError).statusCode).toBe(502); + expect((err as MonidError).code).toBe('HTTP_502'); + expect((err as MonidError).message).toBe('HTTP 502'); + expect((err as MonidError).message).not.toContain('Unexpected token'); + }); + + it('keeps the mapped code for a non-JSON 401', async () => { + stubFetch(GATEWAY_HTML, 401, 'text/html'); + + const err = await api.whoami().then(() => null, (e: unknown) => e); + expect((err as MonidError).code).toBe('AUTH_FAILED'); + expect((err as MonidError).statusCode).toBe(401); + }); + + it('still prefers a structured API error message over the status', async () => { + stubFetch( + JSON.stringify({ error: { code: 'INSUFFICIENT_BALANCE', message: 'Balance too low' } }), + 402, + 'application/json', + ); + + const err = await api.whoami().then(() => null, (e: unknown) => e); + expect((err as MonidError).code).toBe('INSUFFICIENT_BALANCE'); + expect((err as MonidError).message).toBe('Balance too low'); + }); + + it('names a 2xx that is not JSON instead of throwing a parse error', async () => { + stubFetch('\r\ncaptive portal\r\n', 200, 'text/html'); + + const err = await api.whoami().then(() => null, (e: unknown) => e); + expect(err).toBeInstanceOf(MonidError); + expect((err as MonidError).code).toBe('INVALID_RESPONSE'); + expect((err as MonidError).message).toContain('Expected JSON'); + expect((err as MonidError).message).toContain('text/html'); + expect((err as MonidError).message).not.toContain('Unexpected token'); + }); + + it('describes an empty 2xx body rather than reporting a parse error', async () => { + stubFetch('', 200, 'application/json'); + + const err = await api.whoami().then(() => null, (e: unknown) => e); + expect((err as MonidError).code).toBe('INVALID_RESPONSE'); + expect((err as MonidError).message).toContain('empty'); + }); + + it('truncates a long non-JSON body in the error message', async () => { + stubFetch('x'.repeat(5000), 200, 'text/plain'); + + const err = await api.whoami().then(() => null, (e: unknown) => e); + expect((err as MonidError).message).toContain('…'); + expect((err as MonidError).message.length).toBeLessThan(200); + }); + + it('returns the parsed payload on a normal 200', async () => { + stubFetch(JSON.stringify({ workspace: 'Test Workspace' }), 200, 'application/json'); + + const out = await api.whoami() as unknown as { workspace: string }; + expect(out.workspace).toBe('Test Workspace'); + }); +});