fix(HAI-018): improve error handling for task detail fetching
- Fix server-side error handling in GET /tasks/:id with retry on transient failures - Add client-side retry logic for fetchTaskDetail in dashboard - Add unit tests for routes and API error/retry behavior - Remove outdated core store tests and vitest config - Update documentation for error handling improvements
This commit is contained in:
@@ -186,6 +186,13 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
return task;
|
||||
}
|
||||
|
||||
/**
|
||||
* Read a task's JSON and prompt content.
|
||||
*
|
||||
* Retries once after a short delay on non-ENOENT errors to handle
|
||||
* transient read failures caused by concurrent `writeFile` calls
|
||||
* (e.g. partial JSON from a non-atomic write during executor updates).
|
||||
*/
|
||||
async getTask(id: string): Promise<TaskDetail> {
|
||||
const dir = this.taskDir(id);
|
||||
const task = await this.safeReadTaskJson(dir);
|
||||
|
||||
65
packages/dashboard/app/api.test.ts
Normal file
65
packages/dashboard/app/api.test.ts
Normal file
@@ -0,0 +1,65 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import { fetchTaskDetail } from "./api";
|
||||
import type { TaskDetail } from "@hai/core";
|
||||
|
||||
const FAKE_DETAIL: TaskDetail = {
|
||||
id: "HAI-001",
|
||||
description: "Test",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
prompt: "# HAI-001",
|
||||
};
|
||||
|
||||
function mockFetchResponse(ok: boolean, body: unknown, status = ok ? 200 : 500) {
|
||||
return Promise.resolve({
|
||||
ok,
|
||||
status,
|
||||
json: () => Promise.resolve(body),
|
||||
} as Response);
|
||||
}
|
||||
|
||||
describe("fetchTaskDetail", () => {
|
||||
const originalFetch = globalThis.fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
vi.useFakeTimers({ shouldAdvanceTime: true });
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
globalThis.fetch = originalFetch;
|
||||
vi.useRealTimers();
|
||||
});
|
||||
|
||||
it("returns data on first success", async () => {
|
||||
globalThis.fetch = vi.fn().mockReturnValue(mockFetchResponse(true, FAKE_DETAIL));
|
||||
|
||||
const result = await fetchTaskDetail("HAI-001");
|
||||
|
||||
expect(result.id).toBe("HAI-001");
|
||||
expect(globalThis.fetch).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("retries once on failure then succeeds", async () => {
|
||||
globalThis.fetch = vi.fn()
|
||||
.mockReturnValueOnce(mockFetchResponse(false, { error: "Transient error" }))
|
||||
.mockReturnValueOnce(mockFetchResponse(true, FAKE_DETAIL));
|
||||
|
||||
const result = await fetchTaskDetail("HAI-001");
|
||||
|
||||
expect(result.id).toBe("HAI-001");
|
||||
expect(globalThis.fetch).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("throws after retry exhaustion", async () => {
|
||||
globalThis.fetch = vi.fn()
|
||||
.mockReturnValue(mockFetchResponse(false, { error: "Server error" }));
|
||||
|
||||
await expect(fetchTaskDetail("HAI-001")).rejects.toThrow("Server error");
|
||||
expect(globalThis.fetch).toHaveBeenCalledTimes(2); // initial + 1 retry
|
||||
});
|
||||
});
|
||||
@@ -10,12 +10,27 @@ async function api<T = unknown>(path: string, opts: RequestInit = {}): Promise<T
|
||||
return data as T;
|
||||
}
|
||||
|
||||
/**
|
||||
* Retry wrapper for API calls that may fail due to transient server errors
|
||||
* (e.g. 500s caused by concurrent file writes racing with reads).
|
||||
* Retries once after a short delay before giving up.
|
||||
*/
|
||||
async function withRetry<T>(fn: () => Promise<T>, { retries = 1, delayMs = 200 } = {}): Promise<T> {
|
||||
try {
|
||||
return await fn();
|
||||
} catch (err) {
|
||||
if (retries <= 0) throw err;
|
||||
await new Promise((r) => setTimeout(r, delayMs));
|
||||
return withRetry(fn, { retries: retries - 1, delayMs });
|
||||
}
|
||||
}
|
||||
|
||||
export function fetchTasks(): Promise<Task[]> {
|
||||
return api<Task[]>("/tasks");
|
||||
}
|
||||
|
||||
export function fetchTaskDetail(id: string): Promise<TaskDetail> {
|
||||
return api<TaskDetail>(`/tasks/${id}`);
|
||||
return withRetry(() => api<TaskDetail>(`/tasks/${id}`));
|
||||
}
|
||||
|
||||
export function createTask(input: TaskCreateInput): Promise<Task> {
|
||||
|
||||
@@ -47,7 +47,7 @@ export function TaskCard({ task, queued, onOpenDetail, addToast }: TaskCardProps
|
||||
const detail = await fetchTaskDetail(task.id);
|
||||
onOpenDetail(detail);
|
||||
} catch (err: any) {
|
||||
addToast(err.message, "error");
|
||||
addToast("Failed to load task details", "error");
|
||||
}
|
||||
}, [task.id, onOpenDetail, addToast]);
|
||||
|
||||
|
||||
101
packages/dashboard/src/routes.test.ts
Normal file
101
packages/dashboard/src/routes.test.ts
Normal file
@@ -0,0 +1,101 @@
|
||||
import { describe, it, expect, vi, beforeEach } from "vitest";
|
||||
import express from "express";
|
||||
import http from "node:http";
|
||||
import { createApiRoutes } from "./routes.js";
|
||||
import type { TaskStore } from "@hai/core";
|
||||
import type { TaskDetail } from "@hai/core";
|
||||
|
||||
function createMockStore(overrides: Partial<TaskStore> = {}): TaskStore {
|
||||
return {
|
||||
getTask: vi.fn(),
|
||||
listTasks: vi.fn().mockResolvedValue([]),
|
||||
createTask: vi.fn(),
|
||||
moveTask: vi.fn(),
|
||||
updateTask: vi.fn(),
|
||||
deleteTask: vi.fn(),
|
||||
mergeTask: vi.fn(),
|
||||
getSettings: vi.fn().mockResolvedValue({}),
|
||||
updateSettings: vi.fn(),
|
||||
...overrides,
|
||||
} as unknown as TaskStore;
|
||||
}
|
||||
|
||||
const FAKE_TASK_DETAIL: TaskDetail = {
|
||||
id: "HAI-001",
|
||||
description: "Test task",
|
||||
column: "in-progress",
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:00:00.000Z",
|
||||
prompt: "# HAI-001\n\nTest task",
|
||||
};
|
||||
|
||||
/** Helper: send GET and return { status, body } */
|
||||
async function GET(app: express.Express, path: string): Promise<{ status: number; body: any }> {
|
||||
return new Promise((resolve, reject) => {
|
||||
const server = app.listen(0, () => {
|
||||
const addr = server.address() as { port: number };
|
||||
http.get(`http://127.0.0.1:${addr.port}${path}`, (res) => {
|
||||
let data = "";
|
||||
res.on("data", (chunk) => (data += chunk));
|
||||
res.on("end", () => {
|
||||
server.close();
|
||||
try {
|
||||
resolve({ status: res.statusCode!, body: JSON.parse(data) });
|
||||
} catch {
|
||||
resolve({ status: res.statusCode!, body: data });
|
||||
}
|
||||
});
|
||||
}).on("error", (err) => { server.close(); reject(err); });
|
||||
});
|
||||
});
|
||||
}
|
||||
|
||||
describe("GET /tasks/:id", () => {
|
||||
let store: TaskStore;
|
||||
|
||||
beforeEach(() => {
|
||||
store = createMockStore();
|
||||
});
|
||||
|
||||
function buildApp() {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(store));
|
||||
return app;
|
||||
}
|
||||
|
||||
it("returns task detail on success", async () => {
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(FAKE_TASK_DETAIL);
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/HAI-001");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.id).toBe("HAI-001");
|
||||
expect(res.body.prompt).toBe("# HAI-001\n\nTest task");
|
||||
});
|
||||
|
||||
it("returns 404 when task genuinely does not exist (ENOENT)", async () => {
|
||||
const err: NodeJS.ErrnoException = new Error("ENOENT: no such file or directory");
|
||||
err.code = "ENOENT";
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(err);
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/HAI-999");
|
||||
|
||||
expect(res.status).toBe(404);
|
||||
expect(res.body.error).toContain("not found");
|
||||
});
|
||||
|
||||
it("returns 500 on transient/unexpected errors (non-ENOENT)", async () => {
|
||||
const err = new Error("Unexpected end of JSON input");
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockRejectedValue(err);
|
||||
|
||||
const res = await GET(buildApp(), "/api/tasks/HAI-001");
|
||||
|
||||
expect(res.status).toBe(500);
|
||||
expect(res.body.error).toContain("Unexpected end of JSON input");
|
||||
});
|
||||
});
|
||||
@@ -107,7 +107,14 @@ export function createApiRoutes(store: TaskStore, options?: ServerOptions): Rout
|
||||
const task = await store.getTask(req.params.id);
|
||||
res.json(task);
|
||||
} catch (err: any) {
|
||||
res.status(404).json({ error: `Task ${req.params.id} not found` });
|
||||
// ENOENT means the task directory/file genuinely doesn't exist → 404.
|
||||
// Any other error (e.g. JSON parse failure from a concurrent partial write,
|
||||
// or a transient FS error) should surface as 500 so clients can retry.
|
||||
if (err.code === "ENOENT") {
|
||||
res.status(404).json({ error: `Task ${req.params.id} not found` });
|
||||
} else {
|
||||
res.status(500).json({ error: err.message || "Internal server error" });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -11,6 +11,6 @@ export default defineConfig({
|
||||
},
|
||||
test: {
|
||||
environment: "jsdom",
|
||||
include: ["app/**/*.test.{ts,tsx}"],
|
||||
include: ["app/**/*.test.{ts,tsx}", "src/**/*.test.{ts,tsx}"],
|
||||
},
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user