Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 26 additions & 9 deletions src/lib/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,14 @@ export class ApiError extends Error {
readonly method: string,
readonly path: string,
readonly detail: string,
// Per-request id the server stamps on every response; quote it to us so a
// failure maps to an exact log line. Absent only for pre-request failures.
readonly requestId?: string,
) {
super(`${method} ${path} failed: ${status} ${detail}`)
super(
`${method} ${path} failed: ${status} ${detail}` +
(requestId ? ` (request id: ${requestId})` : ''),
)
this.name = 'ApiError'
}
}
Expand Down Expand Up @@ -67,7 +73,8 @@ export class ApiClient {
body: body === undefined ? undefined : JSON.stringify(body),
})
if (!res.ok) {
throw new ApiError(res.status, method, path, await errorDetail(res))
const { detail, requestId } = await parseErrorResponse(res)
throw new ApiError(res.status, method, path, detail, requestId)
}
// Some endpoints (DELETEs, acks) return empty bodies; tolerate that.
const text = await res.text()
Expand Down Expand Up @@ -211,15 +218,25 @@ export function buildQuery(query?: Record<string, unknown>): string {
return qs ? `?${qs}` : ''
}

// Surface the server's `{"detail": ...}` message when present; fall back to the
// status text. Keeps `agent` error output actionable instead of bare codes.
export async function errorDetail(res: Response): Promise<string> {
// Pull the server's `{"detail": ...}` message and request id off a non-2xx
// response. The detail keeps `agent` error output actionable instead of bare
// codes; the id comes from the `X-Request-ID` header (set on every API response)
// and falls back to the `request_id` field the server's 500 handler includes in
// its body, so an error carries something we can grep our logs for.
export async function parseErrorResponse(
res: Response,
): Promise<{ detail: string; requestId?: string }> {
const headerRequestId = res.headers.get('x-request-id') ?? undefined
try {
const body = (await res.json()) as { detail?: unknown }
if (typeof body.detail === 'string') return body.detail
if (body.detail) return JSON.stringify(body.detail)
const body = (await res.json()) as { detail?: unknown; request_id?: unknown }
const requestId =
headerRequestId ??
(typeof body.request_id === 'string' ? body.request_id : undefined)
if (typeof body.detail === 'string') return { detail: body.detail, requestId }
if (body.detail) return { detail: JSON.stringify(body.detail), requestId }
return { detail: res.statusText, requestId }
} catch {
// not JSON
return { detail: res.statusText, requestId: headerRequestId }
}
return res.statusText
}
61 changes: 56 additions & 5 deletions test/api.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { afterEach, describe, expect, it, vi } from 'vitest'
import { ApiClient, ApiError, buildQuery, errorDetail } from '../src/lib/api'
import { ApiClient, ApiError, buildQuery, parseErrorResponse } from '../src/lib/api'

describe('buildQuery', () => {
it('returns empty string for no/empty query', () => {
Expand All @@ -22,24 +22,55 @@ describe('buildQuery', () => {
})
})

describe('errorDetail', () => {
describe('parseErrorResponse', () => {
it('extracts a string detail', async () => {
const res = new Response(JSON.stringify({ detail: 'Invalid credentials' }), {
status: 401,
})
expect(await errorDetail(res)).toBe('Invalid credentials')
expect((await parseErrorResponse(res)).detail).toBe('Invalid credentials')
})

it('stringifies a structured detail (FastAPI 422)', async () => {
const res = new Response(JSON.stringify({ detail: [{ loc: ['query', 'limit'] }] }), {
status: 422,
})
expect(await errorDetail(res)).toContain('limit')
expect((await parseErrorResponse(res)).detail).toContain('limit')
})

it('falls back to status text for non-JSON bodies', async () => {
const res = new Response('<html>oops</html>', { status: 502, statusText: 'Bad Gateway' })
expect(await errorDetail(res)).toBe('Bad Gateway')
expect((await parseErrorResponse(res)).detail).toBe('Bad Gateway')
})

it('reads the request id from the X-Request-ID header', async () => {
const res = new Response(JSON.stringify({ detail: 'boom' }), {
status: 500,
headers: { 'x-request-id': 'request_abc123' },
})
expect(await parseErrorResponse(res)).toEqual({
detail: 'boom',
requestId: 'request_abc123',
})
})

it('falls back to the body request_id when the header is absent', async () => {
const res = new Response(
JSON.stringify({ detail: 'Internal Server Error', request_id: 'request_xyz' }),
{ status: 500 },
)
expect((await parseErrorResponse(res)).requestId).toBe('request_xyz')
})

it('still parses a non-JSON body that carries the header', async () => {
const res = new Response('<html>oops</html>', {
status: 502,
statusText: 'Bad Gateway',
headers: { 'x-request-id': 'request_gw' },
})
expect(await parseErrorResponse(res)).toEqual({
detail: 'Bad Gateway',
requestId: 'request_gw',
})
})
})

Expand Down Expand Up @@ -80,6 +111,26 @@ describe('ApiClient.request', () => {
await expect(api.whoami()).rejects.toThrow(/403 nope/)
})

it('surfaces the request id from the response on non-2xx', async () => {
vi.stubGlobal(
'fetch',
vi.fn(
async () =>
new Response(JSON.stringify({ detail: 'Internal Server Error' }), {
status: 500,
headers: { 'x-request-id': 'request_deadbeef' },
}),
),
)
const api = new ApiClient('http://api.test', 't')
await expect(api.whoami()).rejects.toMatchObject({
name: 'ApiError',
status: 500,
requestId: 'request_deadbeef',
})
await expect(api.whoami()).rejects.toThrow(/request id: request_deadbeef/)
})

it('tolerates empty response bodies', async () => {
vi.stubGlobal('fetch', vi.fn(async () => new Response(null, { status: 204 })))
const api = new ApiClient('http://api.test', 't')
Expand Down
Loading