From 9ec60d6d752cbe04856637ad6fd1296018f35241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Andr=C3=A9=20Padez?= Date: Thu, 27 Aug 2026 14:20:52 +0000 Subject: [PATCH] =?UTF-8?q?caldav=202026.08.27.03=20=E2=80=94=20filters=20?= =?UTF-8?q?on=20calendar=20and=20contacts?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/servers/hono-error-shape.test.ts | 57 +++++++++++++++++++++++++++ src/workspaces/hooks/src/useClient.ts | 14 ++++++- 2 files changed, 70 insertions(+), 1 deletion(-) create mode 100644 src/servers/hono-error-shape.test.ts diff --git a/src/servers/hono-error-shape.test.ts b/src/servers/hono-error-shape.test.ts new file mode 100644 index 00000000..7a59603d --- /dev/null +++ b/src/servers/hono-error-shape.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, test } from 'bun:test'; +import { buildHonoApp } from './hono'; +import { CustomError } from './custom-errors'; + +/* + The property: every error the Hono app renders is JSON — `{ error: string }`, or the object a + CustomError's returnValue supplied. Success responses were always JSON; a text/plain failure was the + one response every client had to special-case, and the one that broke `| jq`. + + The app under test is the REAL one: `buildHonoApp` is pure, and Hono builds its matcher lazily on the + first fetch, so a throwing route added here reaches the same `onError` production traffic does. +*/ + +const appThatThrows = (error: Error) => { + const app = buildHonoApp(); + app.get('/boom', () => { + throw error; + }); + return app; +}; + +const request = (app: ReturnType) => app.request('/boom'); + +describe('onError renders JSON', () => { + test('a CustomError with a message → { error: message }, its own status', async () => { + const res = await request(appThatThrows(new CustomError(404, 'no such playlist'))); + expect(res.status).toBe(404); + expect(res.headers.get('content-type')).toContain('application/json'); + expect(await res.json()).toEqual({ error: 'no such playlist' }); + }); + + // An object returnValue passes through UNWRAPPED — callers read its fields (TOO_MANY_REQUESTS + // carries `retryAfter`), so `{ error: {...} }` would break them. + test('a CustomError with an object returnValue → that object, untouched', async () => { + const res = await request(appThatThrows(new CustomError(429, 'Too many requests', { retryAfter: 30 }))); + expect(res.status).toBe(429); + expect(res.headers.get('content-type')).toContain('application/json'); + expect(await res.json()).toEqual({ retryAfter: 30 }); + }); + + // A string returnValue was the text/plain case before 2026-08-25; it takes the same shape as a message. + test('a CustomError with a string returnValue → { error: returnValue }', async () => { + const res = await request(appThatThrows(new CustomError(400, 'ignored', 'the field is required'))); + expect(res.status).toBe(400); + expect(res.headers.get('content-type')).toContain('application/json'); + expect(await res.json()).toEqual({ error: 'the field is required' }); + }); + + // A plain Error is a bug, not a message for the caller — the body must be JSON but must NOT leak + // error.message, which nobody wrote for an outside reader. + test('a plain Error → 500, { error: "Internal Server Error" }, message not leaked', async () => { + const res = await request(appThatThrows(new Error('secret internal detail'))); + expect(res.status).toBe(500); + expect(res.headers.get('content-type')).toContain('application/json'); + expect(await res.json()).toEqual({ error: 'Internal Server Error' }); + }); +}); diff --git a/src/workspaces/hooks/src/useClient.ts b/src/workspaces/hooks/src/useClient.ts index a51fbe9c..df34eb85 100644 --- a/src/workspaces/hooks/src/useClient.ts +++ b/src/workspaces/hooks/src/useClient.ts @@ -228,7 +228,19 @@ const handleRevokedSession = (url: string) => { const validateResponse = async (res: Response) => { if (res.status >= 400) { const { onError } = useClient.config; - const message = await res.text(); + // Read the body ONCE, as text, and parse it ourselves — a Response body cannot be read twice. + // The platform answers `{ error: string }` since 2026-08-25, but a plugin's own routes and any + // non-Hono surface still answer plain text, so the raw text stays as the fallback. + const body = await res.text(); + let message = body; + try { + const parsed: unknown = JSON.parse(body); + if (parsed && typeof parsed === 'object' && typeof (parsed as { error?: unknown }).error === 'string') { + message = (parsed as { error: string }).error; + } + } catch { + // Not JSON — keep the text as-is. + } if (onError) { onError({ status: res.status, message }); }