diff --git a/src/servers/sidecar/email/http.ts b/src/servers/sidecar/email/http.ts index 1411aa5d..ec098804 100644 --- a/src/servers/sidecar/email/http.ts +++ b/src/servers/sidecar/email/http.ts @@ -1,13 +1,15 @@ import { Hono } from 'hono'; import { getUserById } from 'officerdb'; +import { CustomError } from '../../custom-errors'; +import { bodyParser } from '../../_middlewares/body-parser'; import { emailRouter } from './routes'; // The email sidecar's own listener. `/api/email/*` on the platform is a proxy onto this — the platform // authenticates the owner, injects X-Officer-User, and forwards without reading the body. // -// The routes moved here verbatim, Hono and all, so this file's only real job is to reconstruct the two -// things the platform's middleware used to provide: the authenticated user on the context, and an error -// handler that turns thrown custom-errors into status codes. +// The routes moved here verbatim, Hono and all, so this file's only real job is to reconstruct the three +// things the platform's middleware used to provide: the authenticated user on the context, the parsed +// body on the context, and an error handler that turns thrown custom-errors into status codes. type HonoVariables = { user: { id: number }; body: Record; origin: string }; @@ -34,16 +36,31 @@ app.use('*', async (ctx, next) => { await next(); }); +// Handlers read the body off the context (`ctx.get('body')`), which only the platform's middleware used to +// provide. Without this every write route here destructured undefined — `POST /accounts` threw on +// `body.provider` and surfaced as a bare "failed to add account" toast. `isProxiedPath` is inert in this +// process (the prefix set is populated by `createSidecarProxy`, which runs in the platform), so nothing is +// skipped; multipart on `/send` is parsed once and served from Hono's cache when the route asks again. +app.use('*', bodyParser()); + app.route('/', emailRouter as never); -// custom-errors carry a `status`; anything else is a 500 with no detail leaked to the caller. +// Mirrors the platform's own handler (`hono.ts` onError) rather than inventing a second error shape. Two +// bugs lived in the version that didn't: custom-errors carry `statusCode`, NOT `status`, so every +// deliberate 4xx fell through to the 500 branch with its message replaced by "internal error"; and the +// body was JSON where the whole API answers errors as plain text, which `useClient` reads with +// `res.text()` — so even a correct message reached the client wrapped in `{"error":…}`. app.onError((err, ctx) => { - const status = (err as { status?: number }).status; - if (typeof status === 'number' && status >= 400 && status < 600) { - return ctx.json({ error: err.message }, status as 400); + if (err instanceof CustomError) { + if (err.returnValue) { + return typeof err.returnValue === 'string' + ? ctx.text(err.returnValue, err.statusCode) + : ctx.json(err.returnValue, err.statusCode); + } + return ctx.text(err.message, err.statusCode); } console.error('[email] unhandled error', err); - return ctx.json({ error: 'internal error' }, 500); + return ctx.text('Internal Server Error', 500); }); export function startEmailServer(): number { diff --git a/src/workspaces/hooks/src/useClient.ts b/src/workspaces/hooks/src/useClient.ts index df766d54..2792cb7f 100644 --- a/src/workspaces/hooks/src/useClient.ts +++ b/src/workspaces/hooks/src/useClient.ts @@ -151,6 +151,20 @@ export const DELETE = async (uri: string, payload?: any, baseUrl = '') => { return parseBody(res); }; +/** Thrown by every verb on a 4xx/5xx. A real `Error` subclass, because ~33 call sites narrow with + * `err instanceof Error ? err.message : ` — against the plain object this used to throw that test + * was always false, so every one of them showed its generic fallback and discarded what the server said. + * Keeps `status` and `message`, so code reading either is unaffected. */ +export class ApiError extends Error { + constructor( + public status: number, + message: string, + ) { + super(message); + this.name = 'ApiError'; + } +} + const validateResponse = async (res: Response) => { if (res.status >= 400) { const { onError } = useClient.config; @@ -158,7 +172,7 @@ const validateResponse = async (res: Response) => { if (onError) { onError({ status: res.status, message }); } - throw { status: res.status, message }; + throw new ApiError(res.status, message); } };