diff --git a/.changeset/23365-hono-declared-envelope.md b/.changeset/23365-hono-declared-envelope.md new file mode 100644 index 00000000000..a072eb05396 --- /dev/null +++ b/.changeset/23365-hono-declared-envelope.md @@ -0,0 +1,58 @@ +--- +'@objectstack/hono': minor +'@objectstack/plugin-hono-server': minor +--- + +fix(hono): `createHonoApp` answers an escaped throw that declares an ADR-0112 envelope as that envelope, not as `500 INTERNAL_ERROR` (#23365) + +`@objectstack/hono`'s `createHonoApp` has two catch arms: the auth mount's, and +the `${prefix}/*` catch-all's around `HttpDispatcher.dispatch()`. Both answered +an escaped throw from its `statusCode` alone, so a producer that declared +`status` and a registered `code` lost both on this host. A `GET`, `PATCH` or +`DELETE` of `/data/:object/:id` for an id that names no row answered +`500 INTERNAL_ERROR` where the standalone REST server answers +`404 RECORD_NOT_FOUND`. The `/data` version refusals answered `500` too (a stale +`If-Match`'s `409 CONCURRENT_UPDATE`, and `If-Match: ""`'s +`400 VALIDATION_FAILED`), and so did an authorization-store outage +(`AuthzStoreUnavailableError`, `503 SERVICE_UNAVAILABLE`). + +**What changed.** Both arms now answer a throw that carries **both** a declared +ADR-0112 status (a key of `HttpStatusErrorCodeMap`) **and** a code registered in +`ErrorCode` as that envelope. The recognition is `declaredEnvelopeForThrow`, the +rule `@objectstack/plugin-hono-server`'s direct-mount routes have used since +#16545, so the two Hono doors answer one throw alike by construction: the same +two conditions, the same 5xx disclosure filter (`looksLikeInternalErrorLeak`), +and `details` and `userMessage` forwarded only when the producer declared them. + +The rule's other limbs come with it, as they did on the direct-mount door. A +validation-shaped throw answers `400 VALIDATION_FAILED` with its `fields[]`. A +driver fault the engine has already classified keeps its `500` and now carries +the code it declared instead of `INTERNAL_ERROR`. Measured: `GET /data/:object` +over an object whose table is missing answers `500 DATABASE_ERROR`, with the +same message as before. + +**What did NOT change**, pinned in the same PR through a real `createHonoApp` +over the real dispatcher and protocol (`packages/qa/http-conformance`): + +- a throw that is **not** such an envelope answers what it answered before. A + bare `Error` is still `500 INTERNAL_ERROR` carrying `code` and `message`, with + no `cause` in the body, and a throw that declares `statusCode` with no + registered code keeps that status; +- ⛔ no error code is minted and no ledger row is added. + +`@objectstack/plugin-hono-server` gains one export, `declaredEnvelopeForThrow`, +which was module-private. `createHonoApp` imports it rather than restating the +rule. Its behaviour is unchanged. + +Earlier entries say a `createHonoApp` host still renders the `/data` version +refusals as `500`. This change ends that: they are answered `409` and `400` as +declared. + +⚠️ **Known gap, stated rather than left silent.** The `409 CONCURRENT_UPDATE` +this host now answers carries `code` and `message`, but not the +`currentVersion` and `currentRecord` that the standalone REST server's `409` +carries. The recogniser forwards only the `details` a producer declares, and +those two fields are own properties of the error. That gap is outside this +change. + +Clause-②: yes (widening) diff --git a/packages/adapters/hono/src/index.ts b/packages/adapters/hono/src/index.ts index 4089af0a973..3529a20128d 100644 --- a/packages/adapters/hono/src/index.ts +++ b/packages/adapters/hono/src/index.ts @@ -44,6 +44,7 @@ import { hasWildcardPattern, DEFAULT_CORS_ALLOW_HEADERS, DEFAULT_CORS_EXPOSE_HEADERS, + declaredEnvelopeForThrow, } from '@objectstack/plugin-hono-server'; export interface ObjectStackHonoCorsOptions { @@ -508,6 +509,33 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { return c.json({ success: false, error: { code, message } }, status); }; + /** + * [#23365] What BOTH catch arms below answer for a throw that escaped them. + * + * A throw that carries a declared ADR-0112 `status` and a registered `code` + * is answered as that envelope. Both arms used to read `statusCode` alone, so + * a declared `status` and `code` were lost: a missing record's + * `404 RECORD_NOT_FOUND`, the `/data` version refusals' `409` and `400`, and + * an authorization-store outage's `503` all left `dispatch()` and answered + * `500 INTERNAL_ERROR` on this host only. + * + * The recognition is `declaredEnvelopeForThrow`, the ONE rule #16545 landed + * for `@objectstack/plugin-hono-server`'s direct-mount routes, imported and + * ⛔ never restated here, so the two Hono doors answer one throw alike: the + * same two conditions, the same 5xx disclosure filter, and the same body + * (`details` and `userMessage` only when the producer declared them). + * + * A throw the recogniser does not accept takes the arm's answer exactly as + * before: a bare `Error` is still `500` carrying `code` and `message`, with + * no `cause`, and a throw declaring `statusCode` with no registered code + * keeps that status. + */ + const thrownJson = (c: any, err: any) => { + const declared = declaredEnvelopeForThrow(err); + if (declared) return c.json(declared.body, declared.status); + return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500); + }; + const toResponse = (c: any, result: HttpDispatcherResult) => { if (result.handled) { if (result.response) { @@ -779,7 +807,11 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { if (!result.handled) return yieldUnowned(c, next, () => toResponse(c, result)); return toResponse(c, result); } catch (err: any) { - return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500); + // [#23365] The same rendering as the catch-all below (`thrownJson`). No + // in-repo producer was found that reaches this arm with a declared + // envelope (better-auth answers its own failures as a `Response`), so it + // takes the rule for uniformity: one rule for both arms, not two. + return thrownJson(c, err); } }); @@ -828,7 +860,9 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono { const result = await dispatcher.dispatch(method, subPath, body, queryParams, { request: c.req.raw }, prefix); return toResponse(c, result); } catch (err: any) { - return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500); + // [#23365] A declared ADR-0112 envelope that left `dispatch()` (its foot + // catch re-raises everything but a denial) is answered as declared. + return thrownJson(c, err); } }); diff --git a/packages/plugins/plugin-hono-server/src/adapter.ts b/packages/plugins/plugin-hono-server/src/adapter.ts index 9b1e38dc720..2c18b8fca1b 100644 --- a/packages/plugins/plugin-hono-server/src/adapter.ts +++ b/packages/plugins/plugin-hono-server/src/adapter.ts @@ -280,8 +280,19 @@ function toLoggableError(thrown: unknown): Error { * producer's spelling IS the registered member sitting in `code`, so * `demotedDeclaredCode` returns `undefined` by construction — forwarding it * would put two spellings of one fact on every envelope this seam renders. + * + * ## Exported for the other Hono door (#23365) + * + * `@objectstack/hono`'s `createHonoApp` is a second Hono door: it builds its own + * app, and its two catch arms (the auth mount's and the `dispatch()` + * catch-all's) read `statusCode` alone, so a declared `status` and `code` were + * lost there and the answer was `500`. Those arms now ask THIS function, so the + * two doors answer one throw alike. It is exported so that they can, and ⛔ it + * is not restated there: a second recogniser is how the doors would start + * disagreeing about what counts as declared. Each caller keeps its own + * fallback arm for the `undefined` answer. */ -function declaredEnvelopeForThrow(thrown: unknown): { +export function declaredEnvelopeForThrow(thrown: unknown): { status: number; body: { success: false; error: Record }; } | undefined { diff --git a/packages/qa/http-conformance/src/hono-declared-envelope.conformance.test.ts b/packages/qa/http-conformance/src/hono-declared-envelope.conformance.test.ts new file mode 100644 index 00000000000..9eefba47517 --- /dev/null +++ b/packages/qa/http-conformance/src/hono-declared-envelope.conformance.test.ts @@ -0,0 +1,263 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#23365] Through a REAL `createHonoApp` host, an escaped throw that carries a + * declared ADR-0112 envelope is answered as that envelope by BOTH of the + * adapter's catch arms — the composed-host pin. + * + * ## What this pins + * + * `createHonoApp` has two catch arms: the `${authMount}/*` mount's and the + * `${prefix}/*` catch-all's around `HttpDispatcher.dispatch()`. Both used to + * answer `errorJson(c, err.message || …, err.statusCode || 500)`, reading + * `statusCode` only, so a throw that declared `status` and a registered `code` + * lost both and answered `500 INTERNAL_ERROR`. Measured on this host before the + * change, every declared row below answered `500 INTERNAL_ERROR`. + * + * Both arms now ask the ONE recogniser #16545 landed for the direct-mount + * adapter (`declaredEnvelopeForThrow`, `@objectstack/plugin-hono-server`), so a + * `createHonoApp` host and a `HonoHttpServer` host answer the same throw alike. + * A throw the recogniser does not accept takes the arm's unchanged answer. + * + * Each declared row's producer is the REAL one, reached the way a caller + * reaches it: + * + * - a missing record — `recordNotFoundError` (`@objectstack/core`), raised by + * the real protocol's `getData` / `updateData` / `deleteData`; + * - a stale `If-Match` — the protocol's `ConcurrentUpdateError` (409); + * - `If-Match: ""` — the protocol's `MalformedVersionTokenError` (400); + * - an authorization-store outage — `AuthzStoreUnavailableError` (503), + * raised by the dispatcher's own ADR-0069 gate when an active gate's session + * read fails (`enforceAuthGate`, which sits above `dispatch()`'s `try`). + * + * Each row reads the answer on the wire (status, `success`, nested + * `error.code`), and the write rows also read the STORE afterwards. + * + * ## The other direction + * + * A throw that is NOT a declared envelope keeps the arm's answer: a bare + * `Error` is still `500 INTERNAL_ERROR` carrying `code` and `message` only, with + * no `cause` in the body; a throw that declares `statusCode` and no registered + * code (the shape `callData`'s exposure gate throws) keeps the status it + * declared. The declared 503 also carries no `cause`: the session read's own + * failure stays off the wire. + * + * ## The auth-mount arm + * + * No in-repo producer was found that leaves `authService.handleRequest` with a + * declared envelope: better-auth's router answers its own failures as a + * `Response`. The arm takes the same rendering for uniformity — one rule, not + * two — and its rows use a host-supplied auth service whose `handleRequest` + * throws, which is the shape a host's own `IAuthService` could take. + * + * ## ⚠️ Why this lives HERE and not in `packages/adapters/hono` + * + * `packages/adapters/hono/vitest.config.ts` aliases `@objectstack/runtime` to a + * stub, so no test there reaches the real dispatcher. Here `@objectstack/hono` + * is aliased to this checkout's adapter SOURCE (the subject), and + * `@objectstack/runtime` resolves through its `dist/` (a ledgered pair in + * `scripts/check-test-source-alias.mjs`), as in the sibling + * `hono-*.conformance.test.ts` files. `@objectstack/plugin-hono-server`, where + * the recogniser lives, also resolves through its `dist/`, so a change there + * reaches these rows once that package is rebuilt, which is how CI builds the + * closure. + * + * ## The one seam that is stubbed + * + * WHO the caller is comes from the dispatcher's identity step + * (`timedResolveExecutionContext`), replaced on the prototype with one member, + * as in the sibling files. The protocol is the real one: `ObjectQLPlugin` + * registers `ObjectStackProtocolImplementation` over an in-memory SQLite + * (pure-JS WASM) driver. The two fallback controls on the catch-all replace one + * protocol call with a throw of the shape under test. + */ + +import { describe, it, expect, vi, afterEach } from 'vitest'; +import { LiteKernel, AuthzStoreUnavailableError } from '@objectstack/core'; +import { ObjectQLPlugin } from '@objectstack/objectql'; +import type { ObjectQL } from '@objectstack/objectql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { HttpDispatcher } from '@objectstack/runtime'; +import { createHonoApp } from '@objectstack/hono'; + +const PREFIX = '/api/v1'; +const MEMBER = { userId: 'u_member', isSystem: false, systemPermissions: [] as string[] }; +const MISSING_ID = 'no_such_row'; +/** A version token that names a real instant, and never the stored one. */ +const STALE_VERSION = '"2001-01-01T00:00:00.000Z"'; +/** A cause's text, so a row can assert it never reaches the wire. */ +const CAUSE_TEXT = 'SQLITE_BUSY: database is locked while reading sys_session'; + +const TASK = { + name: 'task', + label: 'Task', + fields: { + name: { name: 'name', type: 'text' as const, label: 'Name' }, + }, +}; + +const liveKernels: LiteKernel[] = []; +afterEach(async () => { + vi.restoreAllMocks(); + while (liveKernels.length) { + const kernel = liveKernels.pop(); + await Promise.race([ + kernel?.shutdown(), + new Promise((resolve) => setTimeout(resolve, 10_000)), + ]); + } +}); + +interface Answer { status: number; body: any } + +/** + * The real store, the real protocol over it, and the REAL `createHonoApp` over + * both. `auth`, when given, is registered as the kernel's `auth` service. + */ +async function boot(auth?: Record) { + const driver = new SqliteWasmDriver({ filename: ':memory:' }); + await driver.connect(); + + const kernel = new LiteKernel(); + liveKernels.push(kernel); + kernel.use({ + name: 'test-declared-envelope-services', + version: '1.0.0', + init: (c: any) => { + c.registerService('driver.memory', driver); + if (auth) c.registerService('auth', auth); + }, + } as any); + kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + + const ql = kernel.getService('objectql'); + ql.registerObject(TASK as any); + await ql.syncObjectSchema('task'); + + vi.spyOn(HttpDispatcher.prototype as any, 'timedResolveExecutionContext') + .mockImplementation(async () => ({ ...MEMBER })); + + const app = createHonoApp({ kernel: kernel as any, prefix: PREFIX, cors: false }); + + const send = async (method: string, path: string, opts: { headers?: Record; body?: unknown } = {}): Promise => { + const res = await app.request(`http://localhost${PREFIX}${path}`, { + method, + headers: opts.body === undefined ? (opts.headers ?? {}) : { 'content-type': 'application/json', ...opts.headers }, + ...(opts.body === undefined ? {} : { body: JSON.stringify(opts.body) }), + }); + const text = await res.text(); + return { status: res.status, body: text === '' ? undefined : JSON.parse(text) }; + }; + + /** A stored task, written through the engine, read back with its version. */ + const seed = async (name: string): Promise => { + const created: any = await ql.insert('task', { name }); + return String(created.id); + }; + /** The stored names, by id — the store, not the door's word for it. */ + const stored = async (id: string): Promise => { + const rows: any[] = await ql.find('task', { where: { id } }) ?? []; + return rows[0]?.name; + }; + + return { send, seed, stored, protocol: kernel.getService('protocol') }; +} + +/** The ADR-0112 envelope's three load-bearing slots. */ +const envelope = (a: Answer) => ({ status: a.status, success: a.body?.success, code: a.body?.error?.code }); + +describe('[#23365] a real createHonoApp host: the dispatch() catch-all renders a declared envelope', () => { + it('control: an existing record is served, and the rig answers 200', async () => { + const host = await boot(); + const id = await host.seed('present'); + const answer = await host.send('GET', `/data/task/${id}`); + expect(answer.status, JSON.stringify(answer.body)).toBe(200); + }, 60_000); + + it.each(['GET', 'PATCH', 'DELETE'])('%s of a missing record: 404 RECORD_NOT_FOUND', async (method) => { + const host = await boot(); + const answer = await host.send(method, `/data/task/${MISSING_ID}`, method === 'PATCH' ? { body: { name: 'x' } } : {}); + expect(envelope(answer), JSON.stringify(answer.body)).toEqual({ status: 404, success: false, code: 'RECORD_NOT_FOUND' }); + }, 60_000); + + it('PATCH with a stale If-Match: 409 CONCURRENT_UPDATE, and the stored row is unchanged', async () => { + const host = await boot(); + const id = await host.seed('original'); + const answer = await host.send('PATCH', `/data/task/${id}`, { headers: { 'If-Match': STALE_VERSION }, body: { name: 'lost update' } }); + expect(envelope(answer), JSON.stringify(answer.body)).toEqual({ status: 409, success: false, code: 'CONCURRENT_UPDATE' }); + expect(await host.stored(id)).toBe('original'); + }, 60_000); + + it('DELETE with a stale If-Match: 409 CONCURRENT_UPDATE, and the row is still stored', async () => { + const host = await boot(); + const id = await host.seed('kept'); + const answer = await host.send('DELETE', `/data/task/${id}`, { headers: { 'If-Match': STALE_VERSION } }); + expect(envelope(answer), JSON.stringify(answer.body)).toEqual({ status: 409, success: false, code: 'CONCURRENT_UPDATE' }); + expect(await host.stored(id)).toBe('kept'); + }, 60_000); + + it.each(['PATCH', 'DELETE'])('%s with If-Match: "" (the empty entity-tag): 400 VALIDATION_FAILED, the row untouched', async (method) => { + const host = await boot(); + const id = await host.seed('untouched'); + const answer = await host.send(method, `/data/task/${id}`, { + headers: { 'If-Match': '""' }, + ...(method === 'PATCH' ? { body: { name: 'unguarded write' } } : {}), + }); + expect(envelope(answer), JSON.stringify(answer.body)).toEqual({ status: 400, success: false, code: 'VALIDATION_FAILED' }); + expect(await host.stored(id)).toBe('untouched'); + }, 60_000); + + it('an authorization-store outage (the ADR-0069 gate\'s session read fails): 503 SERVICE_UNAVAILABLE, no cause in the body', async () => { + const host = await boot({ + isAuthGateActive: () => true, + api: { getSession: async () => { throw new Error(CAUSE_TEXT); } }, + }); + const answer = await host.send('GET', `/data/task/${MISSING_ID}`); + expect(envelope(answer), JSON.stringify(answer.body)).toEqual({ status: 503, success: false, code: 'SERVICE_UNAVAILABLE' }); + expect(answer.body.error).not.toHaveProperty('cause'); + expect(JSON.stringify(answer.body)).not.toContain(CAUSE_TEXT); + }, 60_000); + + it('control: a bare Error is still 500 INTERNAL_ERROR, carrying code and message only, with no cause in the body', async () => { + const host = await boot(); + const id = await host.seed('present'); + vi.spyOn(host.protocol, 'getData').mockRejectedValueOnce(new Error('handler fault', { cause: new Error(CAUSE_TEXT) })); + const answer = await host.send('GET', `/data/task/${id}`); + expect(envelope(answer), JSON.stringify(answer.body)).toEqual({ status: 500, success: false, code: 'INTERNAL_ERROR' }); + expect(Object.keys(answer.body.error).sort()).toEqual(['code', 'message']); + expect(JSON.stringify(answer.body)).not.toContain(CAUSE_TEXT); + }, 60_000); + + it('control: a throw declaring `statusCode` and no registered code keeps the status it declared', async () => { + const host = await boot(); + const id = await host.seed('present'); + // The shape `callData` throws when the data service is missing. + vi.spyOn(host.protocol, 'getData').mockRejectedValueOnce({ statusCode: 503, message: 'Data service not available' }); + const answer = await host.send('GET', `/data/task/${id}`); + expect(answer.status, JSON.stringify(answer.body)).toBe(503); + expect(answer.body).toEqual({ success: false, error: { code: 'SERVICE_UNAVAILABLE', message: 'Data service not available' } }); + }, 60_000); +}); + +describe('[#23365] a real createHonoApp host: the auth-mount arm renders a declared envelope', () => { + it('an auth service that throws a declared envelope: that envelope, with no cause in the body', async () => { + const host = await boot({ + handleRequest: async () => { throw new AuthzStoreUnavailableError('sys_session', new Error(CAUSE_TEXT)); }, + }); + const answer = await host.send('GET', '/auth/get-session'); + expect(envelope(answer), JSON.stringify(answer.body)).toEqual({ status: 503, success: false, code: 'SERVICE_UNAVAILABLE' }); + expect(answer.body.error).not.toHaveProperty('cause'); + expect(JSON.stringify(answer.body)).not.toContain(CAUSE_TEXT); + }, 60_000); + + it('control: an auth service that throws a bare Error is still 500 INTERNAL_ERROR, code and message only', async () => { + const host = await boot({ + handleRequest: async () => { throw new Error('auth handler fault', { cause: new Error(CAUSE_TEXT) }); }, + }); + const answer = await host.send('GET', '/auth/get-session'); + expect(envelope(answer), JSON.stringify(answer.body)).toEqual({ status: 500, success: false, code: 'INTERNAL_ERROR' }); + expect(Object.keys(answer.body.error).sort()).toEqual(['code', 'message']); + expect(JSON.stringify(answer.body)).not.toContain(CAUSE_TEXT); + }, 60_000); +});