fix(HAI-018): complete Step 1 — fix server-side error handling in GET /tasks/:id
- Route now returns 404 only for ENOENT, 500 for transient errors - store.getTask() retries once after 50ms on non-ENOENT failures - Added route tests for success, 404, and 500 cases - Updated vitest config to include src/**/*.test.* files
This commit is contained in:
@@ -118,10 +118,29 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
|||||||
return task;
|
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> {
|
async getTask(id: string): Promise<TaskDetail> {
|
||||||
const dir = this.taskDir(id);
|
const dir = this.taskDir(id);
|
||||||
const data = await readFile(join(dir, "task.json"), "utf-8");
|
const taskPath = join(dir, "task.json");
|
||||||
const task = JSON.parse(data) as Task;
|
|
||||||
|
let task: Task;
|
||||||
|
try {
|
||||||
|
const data = await readFile(taskPath, "utf-8");
|
||||||
|
task = JSON.parse(data) as Task;
|
||||||
|
} catch (err: any) {
|
||||||
|
// If the file doesn't exist, propagate immediately (true 404)
|
||||||
|
if (err.code === "ENOENT") throw err;
|
||||||
|
// Transient error (e.g. partial read / JSON parse failure) — retry once
|
||||||
|
await new Promise((r) => setTimeout(r, 50));
|
||||||
|
const data = await readFile(taskPath, "utf-8");
|
||||||
|
task = JSON.parse(data) as Task;
|
||||||
|
}
|
||||||
|
|
||||||
let prompt = "";
|
let prompt = "";
|
||||||
const promptPath = join(dir, "PROMPT.md");
|
const promptPath = join(dir, "PROMPT.md");
|
||||||
|
|||||||
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);
|
const task = await store.getTask(req.params.id);
|
||||||
res.json(task);
|
res.json(task);
|
||||||
} catch (err: any) {
|
} 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: {
|
test: {
|
||||||
environment: "jsdom",
|
environment: "jsdom",
|
||||||
include: ["app/**/*.test.{ts,tsx}"],
|
include: ["app/**/*.test.{ts,tsx}", "src/**/*.test.{ts,tsx}"],
|
||||||
},
|
},
|
||||||
});
|
});
|
||||||
|
|||||||
2
pnpm-lock.yaml
generated
2
pnpm-lock.yaml
generated
@@ -115,7 +115,7 @@ importers:
|
|||||||
version: 5.9.3
|
version: 5.9.3
|
||||||
vitest:
|
vitest:
|
||||||
specifier: ^4.1.1
|
specifier: ^4.1.1
|
||||||
version: 4.1.1(@types/node@25.5.0)(vite@6.4.1(@types/node@25.5.0)(tsx@4.21.0)(yaml@2.8.3))
|
version: 4.1.1(@types/node@25.5.0)(jsdom@29.0.1)(vite@6.4.1(@types/node@25.5.0)(tsx@4.21.0)(yaml@2.8.3))
|
||||||
|
|
||||||
packages:
|
packages:
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user