fix: return 503 instead of 400 for computer-route service errors

src/server/computer-routes.ts

@@ -4,17 +4,27 @@ import type { ComputerService } from './computer-service.js';
 import type { ComputerAction } from '../shared/computer-types.js';
 export function computerRoutes(computers: ComputerService) {
   const app = new Hono();
-  app.onError((error, c) =>
-    c.json(
-      {
-        error:
-          error instanceof z.ZodError
-            ? 'Invalid computer request.'
-            : error.message,
-      },
-      400,
-    ),
-  );
+  app.onError((error, c) => {
+    if (error instanceof SyntaxError)
+      return c.json({ error: 'Invalid JSON request.' }, 400);
+    if (error instanceof z.ZodError)
+      return c.json({ error: 'Invalid computer request.' }, 400);
+    const text = error.message;
+    if (text === 'Dot not found.') return c.json({ error: text }, 404);
+    if (
+      text === 'Computer permission is disabled.' ||
+      text === 'Human controls are owner-only.'
+    )
+      return c.json({ error: text }, 403);
+    if (
+      text === 'Start this Dot’s computer first.' ||
+      text === 'There is no active control request.'
+    )
+      return c.json({ error: text }, 409);
+    if (text === 'Unknown computer action.')
+      return c.json({ error: text }, 400);
+    return c.json({ error: text }, 503);
+  });
   app.get('/dots/:id/computer', async (c) =>
     c.json(await computers.status(c.req.param('id'))),
   );

src/server/computer-service.ts

@@ -141,8 +141,16 @@ export class ComputerService {
       signal,
     );
   }
+  private parseUpstream<T>(schema: z.ZodType<T>, raw: unknown): T {
+    const result = schema.safeParse(raw);
+    if (!result.success)
+      throw new Error('Computer service returned an invalid response.', {
+        cause: result.error,
+      });
+    return result.data;
+  }
   private endpoint(id: string, raw: unknown) {
-    const state = stateSchema.parse(raw);
+    const state = this.parseUpstream(stateSchema, raw);
     const ns = this.config.computerNamespace ?? 'opendots';
     if (!/^[A-Za-z0-9][A-Za-z0-9_-]{0,63}$/.test(ns))
       throw new Error('Invalid computer namespace.');
@@ -175,9 +183,10 @@ export class ComputerService {
     return url.origin;
   }
   private async existing(id: string, signal?: AbortSignal) {
-    const listing = z
-      .object({ computers: z.array(stateSchema) })
-      .parse(await this.supervisor('/computers', undefined, signal));
+    const listing = this.parseUpstream(
+      z.object({ computers: z.array(stateSchema) }),
+      await this.supervisor('/computers', undefined, signal),
+    );
     return listing.computers.find((c) => c.botId === id);
   }
   private async running(id: string, signal?: AbortSignal) {
@@ -264,7 +273,8 @@ export class ComputerService {
       if (verb === 'take') this.allowed(id, 'browser', 'owner');
       const url = await this.running(id);
       if (verb === 'take') this.allowed(id, 'browser', 'owner');
-      let control: ComputerControl = controlSchema.parse(
+      let control: ComputerControl = this.parseUpstream(
+        controlSchema,
         await this.json(
           `${url}/control`,
           this.token(id),
@@ -274,7 +284,8 @@ export class ComputerService {
         ),
       );
       if (verb === 'take' && !control.request)
-        control = controlSchema.parse(
+        control = this.parseUpstream(
+          controlSchema,
           await this.json(
             `${url}/control/request`,
             this.token(id),
@@ -288,7 +299,8 @@ export class ComputerService {
         control.request &&
         !['waiting', 'taken'].includes(control.request.status)
       )
-        control = controlSchema.parse(
+        control = this.parseUpstream(
+          controlSchema,
           await this.json(
             `${url}/control/request`,
             this.token(id),

tests/computer-routes.test.ts

@@ -0,0 +1,180 @@
+import { afterEach, expect, it } from 'vitest';
+import { WorkspaceStore } from '../src/server/workspace.js';
+import { ComputerService } from '../src/server/computer-service.js';
+import { computerRoutes } from '../src/server/computer-routes.js';
+
+const stores: WorkspaceStore[] = [];
+afterEach(() => {
+  for (const store of stores.splice(0)) store.close();
+});
+
+function fixture() {
+  const workspace = new WorkspaceStore(':memory:', 'owner');
+  stores.push(workspace);
+  const id = workspace.dots()[0].id;
+  const config = {
+    baseUrl: 'https://example.com',
+    voiceName: 'voice',
+    slackUsers: [],
+    runtimeUrl: 'http://localhost',
+  };
+  // No computer supervisor configured: `configured` is false, so any
+  // action that reaches `allowed()` fails with a plain domain error
+  // rather than attempting a network call.
+  const service = new ComputerService(workspace, config, () => false);
+  const app = computerRoutes(service);
+  return { workspace, id, app };
+}
+
+// `configured: true`, so `allowed()` reaches its permission check instead of
+// failing earlier with "Computer service is not configured."; `transport` is
+// never called because `allowed()` throws before any network request.
+function configuredFixture() {
+  const workspace = new WorkspaceStore(':memory:', 'owner');
+  stores.push(workspace);
+  const id = workspace.dots()[0].id;
+  const config = {
+    baseUrl: 'https://example.com',
+    voiceName: 'voice',
+    slackUsers: [],
+    runtimeUrl: 'http://localhost',
+    computerSupervisorUrl: 'https://supervisor.example.com',
+    computerSupervisorToken: 'supervisor-token',
+    computerToken: 'computer-token',
+  };
+  const transport = () => {
+    throw new Error('transport should not be called in this test');
+  };
+  const service = new ComputerService(
+    workspace,
+    config,
+    () => false,
+    transport,
+  );
+  const app = computerRoutes(service);
+  return { workspace, id, app };
+}
+
+it('answers malformed JSON on the actions route with 400, not a service error', async () => {
+  const { app, id } = fixture();
+  const response = await app.request(`/dots/${id}/computer/actions`, {
+    method: 'POST',
+    headers: { 'Content-Type': 'application/json' },
+    body: '{',
+  });
+  expect(response.status).toBe(400);
+  expect(await response.json()).toEqual({ error: 'Invalid JSON request.' });
+});
+
+it('answers an invalid action shape with 400 from the Zod schema', async () => {
+  const { app, id } = fixture();
+  const response = await app.request(`/dots/${id}/computer/actions`, {
+    method: 'POST',
+    headers: { 'Content-Type': 'application/json' },
+    body: JSON.stringify({ input: {} }),
+  });
+  expect(response.status).toBe(400);
+  expect(await response.json()).toEqual({ error: 'Invalid computer request.' });
+});
+
+it('answers an unconfigured computer service with 503, not 400', async () => {
+  const { app, id } = fixture();
+  const response = await app.request(`/dots/${id}/computer/start`, {
+    method: 'POST',
+  });
+  expect(response.status).toBe(503);
+  expect(await response.json()).toEqual({
+    error: 'Computer service is not configured.',
+  });
+});
+
+it('answers an unknown Dot id with 404, not a service error', async () => {
+  const { app } = fixture();
+  const response = await app.request('/dots/missing/computer/start', {
+    method: 'POST',
+  });
+  expect(response.status).toBe(404);
+  expect(await response.json()).toEqual({ error: 'Dot not found.' });
+});
+
+it('answers an unknown computer action with 400, not a service error', async () => {
+  const { app, id } = fixture();
+  const response = await app.request(`/dots/${id}/computer/actions`, {
+    method: 'POST',
+    headers: { 'Content-Type': 'application/json' },
+    body: JSON.stringify({ action: 'not_a_real_action', input: {} }),
+  });
+  expect(response.status).toBe(400);
+  expect(await response.json()).toEqual({ error: 'Unknown computer action.' });
+});
+
+it('answers a disabled computer permission with 403, not a service error', async () => {
+  const { app, id, workspace } = configuredFixture();
+  workspace.computers.patch(id, { enabled: false });
+  const response = await app.request(`/dots/${id}/computer/take`, {
+    method: 'POST',
+  });
+  expect(response.status).toBe(403);
+  expect(await response.json()).toEqual({
+    error: 'Computer permission is disabled.',
+  });
+});
+
+// `stateSchema`/`controlSchema` also validate the supervisor's own response,
+// so a malformed upstream body must not be mistaken for the caller's fault:
+// it is a service failure (503), not an invalid request (400).
+function upstreamFixture(responses: Record<string, unknown>) {
+  const workspace = new WorkspaceStore(':memory:', 'owner');
+  stores.push(workspace);
+  const id = workspace.dots()[0].id;
+  workspace.computers.patch(id, { enabled: true, browser: true });
+  const config = {
+    baseUrl: 'https://example.com',
+    voiceName: 'voice',
+    slackUsers: [],
+    runtimeUrl: 'http://localhost',
+    computerSupervisorUrl: 'https://supervisor.example.com',
+    computerSupervisorToken: 'supervisor-token',
+    computerToken: 'computer-token',
+  };
+  const transport = async (url: RequestInfo | URL) => {
+    for (const [suffix, body] of Object.entries(responses))
+      if (String(url).endsWith(suffix))
+        return new Response(JSON.stringify(body), { status: 200 });
+    throw new Error(`transport should not be called for ${url}`);
+  };
+  const service = new ComputerService(
+    workspace,
+    config,
+    () => false,
+    transport,
+  );
+  const app = computerRoutes(service);
+  return { id, app };
+}
+
+it('answers a malformed supervisor ensure response with 503, not a caller validation error', async () => {
+  const { app, id } = upstreamFixture({ '/ensure': {} });
+  const response = await app.request(`/dots/${id}/computer/start`, {
+    method: 'POST',
+  });
+  expect(response.status).toBe(503);
+  expect(await response.json()).toEqual({
+    error: 'Computer service returned an invalid response.',
+  });
+});
+
+it('answers a malformed supervisor listing response with 503, not a caller validation error', async () => {
+  const { app, id } = upstreamFixture({
+    '/computers': { computers: [{ botId: 'placeholder' }] },
+  });
+  const response = await app.request(`/dots/${id}/computer/actions`, {
+    method: 'POST',
+    headers: { 'Content-Type': 'application/json' },
+    body: JSON.stringify({ action: 'read', input: {} }),
+  });
+  expect(response.status).toBe(503);
+  expect(await response.json()).toEqual({
+    error: 'Computer service returned an invalid response.',
+  });
+});