Improve API JSON error handling
- Keep ky HTTPError instances intact instead of flattening them - Use the parsed error payload when the server sends a useful message - Fix the Issues default search type so issueId stays optional - Add regression tests for the JSON helper behavior
This commit is contained in:
parent
350c147d1e
commit
9cde9442b7
3 changed files with 82 additions and 14 deletions
54
webui/src/app/api-client.test.ts
Normal file
54
webui/src/app/api-client.test.ts
Normal file
|
|
@ -0,0 +1,54 @@
|
|||
import { describe, expect, it, vi } from 'vite-plus/test';
|
||||
|
||||
import { HTTPError } from 'ky';
|
||||
|
||||
import type { ResponsePromise } from 'ky';
|
||||
|
||||
import { readJson } from './api-client';
|
||||
|
||||
function createHttpError(body: unknown, status = 400) {
|
||||
const response = new Response(JSON.stringify(body), {
|
||||
status,
|
||||
statusText: 'Bad Request',
|
||||
headers: {
|
||||
'Content-Type': 'application/json',
|
||||
},
|
||||
});
|
||||
const request = new Request('https://example.com/api/test');
|
||||
const error = new HTTPError(response, request, {} as any);
|
||||
error.data = body;
|
||||
return error;
|
||||
}
|
||||
|
||||
describe('readJson', () => {
|
||||
it('returns parsed JSON', async () => {
|
||||
const json = vi.fn().mockResolvedValue({ ok: true });
|
||||
const promise = { json } as unknown as ResponsePromise<{ ok: boolean }>;
|
||||
|
||||
await expect(readJson(promise)).resolves.toEqual({ ok: true });
|
||||
expect(json).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('keeps HTTPError instances intact and uses the payload message', async () => {
|
||||
const error = createHttpError({ error: 'Nope' }, 403);
|
||||
const json = vi.fn().mockRejectedValue(error);
|
||||
const promise = { json } as unknown as ResponsePromise<unknown>;
|
||||
const result = readJson(promise);
|
||||
|
||||
await expect(result).rejects.toBe(error);
|
||||
await expect(result).rejects.toHaveProperty('message', 'Nope');
|
||||
});
|
||||
|
||||
it('falls back to the HTTPError message when the payload is unhelpful', async () => {
|
||||
const error = createHttpError({ detail: 'missing error field' }, 404);
|
||||
const json = vi.fn().mockRejectedValue(error);
|
||||
const promise = { json } as unknown as ResponsePromise<unknown>;
|
||||
const result = readJson(promise);
|
||||
|
||||
await expect(result).rejects.toBe(error);
|
||||
await expect(result).rejects.toHaveProperty(
|
||||
'message',
|
||||
error.message,
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
@ -12,16 +12,30 @@ export const apiClient = ky.create({
|
|||
retry: 0,
|
||||
});
|
||||
|
||||
export async function parseJsonResponse<T>(promise: ResponsePromise<T>): Promise<T> {
|
||||
export async function readJson<T>(promise: ResponsePromise<T>): Promise<T> {
|
||||
try {
|
||||
return (await promise.json()) as T;
|
||||
return await promise.json<T>();
|
||||
} catch (error) {
|
||||
if (error instanceof HTTPError) {
|
||||
const payload = (await error.response.json().catch(() => null)) as { error?: string } | null;
|
||||
|
||||
throw new Error(payload?.error || `Request failed with status ${error.response.status}`);
|
||||
error.message = getHttpErrorMessage(error.data, error.message);
|
||||
}
|
||||
|
||||
throw error;
|
||||
}
|
||||
}
|
||||
|
||||
type JsonErrorPayload = {
|
||||
error?: unknown;
|
||||
message?: unknown;
|
||||
};
|
||||
|
||||
function getHttpErrorMessage(data: unknown, fallback: string): string {
|
||||
if (typeof data === 'string' && data.trim()) return data;
|
||||
if (!data || typeof data !== 'object') return fallback;
|
||||
|
||||
const payload = data as JsonErrorPayload;
|
||||
if (typeof payload.error === 'string' && payload.error.trim()) return payload.error;
|
||||
if (typeof payload.message === 'string' && payload.message.trim()) return payload.message;
|
||||
|
||||
return fallback;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -1,6 +1,6 @@
|
|||
import { queryOptions } from '@tanstack/react-query';
|
||||
|
||||
import { apiClient, parseJsonResponse } from '@/app/api-client';
|
||||
import { apiClient, readJson } from '@/app/api-client';
|
||||
|
||||
import type {
|
||||
IssueCounts,
|
||||
|
|
@ -18,10 +18,10 @@ const DEFAULT_LIMIT = 100;
|
|||
export const REFRESH_EVENT = 'ss:issues-refresh';
|
||||
export const CLOSE_EVENT = 'ss:issues-close-detail';
|
||||
|
||||
export const DEFAULT_ISSUES_SEARCH: Required<IssuesSearch> = {
|
||||
export const DEFAULT_ISSUES_SEARCH = {
|
||||
status: 'open',
|
||||
category: 'all',
|
||||
};
|
||||
} satisfies Required<Pick<IssuesSearch, 'status' | 'category'>>;
|
||||
|
||||
export const ISSUE_CATEGORY_META: Record<
|
||||
string,
|
||||
|
|
@ -146,7 +146,7 @@ export function normalizeIssuesSearch(search: IssuesSearch | undefined): Normali
|
|||
}
|
||||
|
||||
export async function fetchIssueCounts(profileId: number): Promise<IssueCounts> {
|
||||
const payload = await parseJsonResponse<IssueCountsResponse>(
|
||||
const payload = await readJson<IssueCountsResponse>(
|
||||
apiClient.get('issues/counts', {
|
||||
headers: createIssueHeaders(profileId),
|
||||
}),
|
||||
|
|
@ -170,7 +170,7 @@ export async function fetchIssueList(
|
|||
params.set('category', search.category);
|
||||
}
|
||||
|
||||
const payload = await parseJsonResponse<IssueListResponse>(
|
||||
const payload = await readJson<IssueListResponse>(
|
||||
apiClient.get('issues', {
|
||||
headers: createIssueHeaders(profileId),
|
||||
searchParams: params,
|
||||
|
|
@ -183,7 +183,7 @@ export async function fetchIssueList(
|
|||
}
|
||||
|
||||
export async function fetchIssue(profileId: number, issueId: number): Promise<IssueRecord> {
|
||||
const payload = await parseJsonResponse<IssueDetailResponse>(
|
||||
const payload = await readJson<IssueDetailResponse>(
|
||||
apiClient.get(`issues/${issueId}`, {
|
||||
headers: createIssueHeaders(profileId),
|
||||
}),
|
||||
|
|
@ -199,7 +199,7 @@ export async function updateIssue(
|
|||
issueId: number,
|
||||
updates: { status?: string; admin_response?: string },
|
||||
): Promise<void> {
|
||||
const payload = await parseJsonResponse<{ success: boolean; error?: string }>(
|
||||
const payload = await readJson<{ success: boolean; error?: string }>(
|
||||
apiClient.put(`issues/${issueId}`, {
|
||||
headers: createIssueHeaders(profileId),
|
||||
json: updates,
|
||||
|
|
@ -214,7 +214,7 @@ export async function createIssue(
|
|||
profileId: number,
|
||||
payload: CreateIssuePayload,
|
||||
): Promise<IssueRecord | null> {
|
||||
const response = await parseJsonResponse<{
|
||||
const response = await readJson<{
|
||||
success: boolean;
|
||||
issue?: IssueRecord;
|
||||
error?: string;
|
||||
|
|
@ -238,7 +238,7 @@ export async function createIssue(
|
|||
}
|
||||
|
||||
export async function deleteIssue(profileId: number, issueId: number): Promise<void> {
|
||||
const payload = await parseJsonResponse<{ success: boolean; error?: string }>(
|
||||
const payload = await readJson<{ success: boolean; error?: string }>(
|
||||
apiClient.delete(`issues/${issueId}`, {
|
||||
headers: createIssueHeaders(profileId),
|
||||
}),
|
||||
|
|
|
|||
Loading…
Reference in a new issue