From 7b7147001bebd72a1c392033cb2958646fc0792b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Fri, 7 Aug 2026 19:31:31 +0000 Subject: [PATCH] fix email account creation: body parser, error shape, and non-Error throws MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit adding an email account failed with a bare "failed to add account" toast. three defects stacked, each hiding the next. the email sidecar's http.ts reconstructs what the platform's middleware used to provide, but only did two of three — bodyParser was never remounted, so every write route read ctx.get('body') as undefined and POST /accounts threw on body.provider before ever reaching the credentials. its onError then read `.status` off the thrown custom-error, which carries `statusCode`. every deliberate 4xx fell through to the 500 branch and had its message replaced with "internal error", so a rejected IMAP login and a genuine crash looked identical. it also answered JSON where the rest of the api answers errors as plain text. now mirrors hono.ts's handler rather than inventing a second shape. useClient threw a plain object, so the ~33 sites narrowing with `err instanceof Error ? err.message : ` always took the fallback and discarded the server's message. now throws an ApiError subclass keeping both status and message, so those sites start surfacing real errors. only email reads ctx.get('body'); every other sidecar is a pure proxy. Co-Authored-By: Claude Opus 5 --- src/servers/sidecar/email/http.ts | 33 ++++++++++++++++++++------- src/workspaces/hooks/src/useClient.ts | 16 ++++++++++++- 2 files changed, 40 insertions(+), 9 deletions(-) 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); } };