Skip to content

Commit e1cefa5

Browse files
fix(hono): createHonoApp's two catch arms answer a declared ADR-0112 envelope as declared, not as 500 (#23365) (#23530)
Fixes #23365 Clause-②: yes (widening): one added export of `@objectstack/plugin-hono-server`, `declaredEnvelopeForThrow`, which was module-private ## What this does `createHonoApp` (`@objectstack/hono`) has two catch arms: the `${authMount}/*` mount's and the `${prefix}/*` catch-all's around `HttpDispatcher.dispatch()`. Both answered `errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500)`, reading `statusCode` alone. A throw that declared `status` and a registered `code` therefore lost both on this host and answered `500 INTERNAL_ERROR`. Both arms now go through one helper, `thrownJson`. It asks `declaredEnvelopeForThrow`, the recogniser #16545 landed for `@objectstack/plugin-hono-server`'s direct-mount routes, and renders the envelope it returns. When the recogniser answers `undefined`, the arm's old line runs unchanged. No second recogniser is written. The recogniser was module-private, so it is now exported from `@objectstack/plugin-hono-server` (its `src/index.ts` already does `export * from './adapter'`, so `adapter.ts` is the only file that changes there). That export is why line 2 reads `Clause-②: yes`, against the claim's `no`; the dispatch order names this exception. **The seat re-grades.** ## Readings on the wire (real `createHonoApp` over the real dispatcher and protocol, in-memory SQLite) Taken at `167cebe824` (fix) against `84672ff876` (pins only, adapter unchanged). Both are on this branch, and both sit on `origin/main` `869a381dc9`. | Row (producer) | Before | After | |---|---|---| | `GET` / `PATCH` / `DELETE /data/task/no_such_row` (core `recordNotFoundError`, via the protocol) | `500 INTERNAL_ERROR` "Record no_such_row not found in task" | `404 RECORD_NOT_FOUND` | | `PATCH` with a stale `If-Match` (protocol `ConcurrentUpdateError`) | `500 INTERNAL_ERROR` | `409 CONCURRENT_UPDATE`, row unchanged | | `DELETE` with a stale `If-Match` | `500 INTERNAL_ERROR` | `409 CONCURRENT_UPDATE`, row kept | | `PATCH` / `DELETE` with `If-Match: ""` (protocol `MalformedVersionTokenError`) | `500 INTERNAL_ERROR` | `400 VALIDATION_FAILED`, row untouched | | The ADR-0069 gate's session read fails (`AuthzStoreUnavailableError('auth_gate')`, raised by `enforceAuthGate` above `dispatch()`'s `try`) | `500 INTERNAL_ERROR` | `503 SERVICE_UNAVAILABLE`, no cause in the body | | Auth mount: the auth service's `handleRequest` throws `AuthzStoreUnavailableError` | `500 INTERNAL_ERROR` | `503 SERVICE_UNAVAILABLE`, no cause in the body | | Control: a bare `Error` (with a `cause`) on either arm | `500 INTERNAL_ERROR`, keys `code` and `message` | unchanged | | Control: a throw that declares `statusCode: 503` and no code (`callData`'s shape) | `503 SERVICE_UNAVAILABLE` | unchanged | ## Mechanism hypotheses, as measured - **(a) The two arms.** Confirmed on `869a381dc9`. Both lines were `index.ts:782` (auth) and `:831` (catch-all) and read `statusCode` only. - **(b) The recogniser.** `declaredEnvelopeForThrow` reads `resolveThrownHttpError`. Its `declaredStatus` comes from `.status`, then `.statusCode`, then the 400 that a validation-shaped throw declares. It requires that status to be a key of `HttpStatusErrorCodeMap`, and the producer's own `.code` to parse as `ErrorCode`. It renders `{ success: false, error: { code, message, details?, userMessage? } }`, and on 5xx it applies `looksLikeInternalErrorLeak` to the message. `@objectstack/hono` could not reach it through any existing export, so the export was added. - **(c) Declared throws.** All five measured; see the table. - **(d) The 409 body.** It does **not** carry `currentVersion` or `currentRecord`. The after-fix body is `{"success":false,"error":{"code":"CONCURRENT_UPDATE","message":"Record task/ID was modified by another user (current version T1, expected T0)"}}`. The recogniser forwards only the `details` that a producer declares (non-string `code`, `issues`, validation `fields`). `ConcurrentUpdateError` keeps the two fields as own properties, and only `RestServer`'s classification arm reads them (`packages/types/src/data-error-classification.ts`, the `CONCURRENT_UPDATE` arm). Per the ruling, the recogniser is not widened here. This is reported to the seat as a finding. - **(e) The auth-mount arm.** No in-repo producer was found that reaches it with a declared envelope. better-call 1.4.0's router (the one better-auth's handler runs) turns a handler throw into a `Response`: an `APIError` becomes its own `Response`, and anything else becomes a bare `500`. The dispatcher's `handleAuthRequest` catches the auth service's throw itself. The arm takes the same rendering for uniformity: one rule, not two. Its pin uses a host-supplied auth service whose `handleRequest` throws. ## "No cause in the body" Read as: the body of a non-envelope throw gains nothing. It gets no `cause`, no `details` and no `userMessage`; it carries `code` and `message` only. Each control row asserts exactly that, with a cause whose text must not appear in the body. The old fallback line is unchanged byte for byte, so a bare `Error`'s own message is still in its `500` body, as it was before. `packages/adapters/hono/src/hono.test.ts` already pins that (`returns error on dispatch exception`), and this PR leaves that pin alone. Withholding the message of an undeclared 500 on this host would be a separate behaviour change; see the acceptance notes. ## Tests - New: `packages/qa/http-conformance/src/hono-declared-envelope.conformance.test.ts`, 13 rows. Before the fix: 9 failed, 4 passed (the four controls). After: 13 passed. - `@objectstack/http-conformance`: 13 files, 137 tests passed. `typecheck` (with `check:test-typecheck`) is green, and `tsc --listFiles -p tsconfig.test.json` counts 13 of 13 test files, the new one included. - `@objectstack/hono`: 7 files, 154 tests passed; `typecheck` green. - `@objectstack/plugin-hono-server`: 29 files, 336 tests passed; `typecheck` green. ## Ablation (one leg per arm, `scripts/ablation-replace.mjs` in wrap mode plus an outer `trap … EXIT INT TERM` restore) In http-conformance's `vitest.config.ts`, the subject `@objectstack/hono` is aliased to `packages/adapters/hono/src/index.ts`, so a mutation of the source reaches the run without a build. - **Catch-all arm** reverted to the old line. The anchor hit once, x1 to x0; the blob went `3529a20128d4` to `51939c434b24`. Result: **8 failed, 5 passed**. All 8 catch-all declared pins went red. The controls and the two auth-arm rows stayed green. - **Auth arm** reverted to the old line. The anchor hit once; the blob went `3529a20128d4` to `4314a9ba898d`. Result: **1 failed, 12 passed**. Only the auth-arm declared pin went red, and its bare-`Error` control stayed green. - Restore was proven for each leg: the blob equals HEAD (`3529a20128d4`) and `git diff HEAD` is empty. ## Gates - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack` derived 66 commands at `287fd99bda`. 65 exited 0. `pnpm check:dual-build-cjs-loads` exited 3 (PREREQUISITE NOT MET: it reads the whole tree's `dist/`), which counts as NOT MEASURED locally; CI runs it. `--ran` reports 66 derived, 65 run, 1 NOT-MEASURED, 0 UNRUN. - Lint, narrowed: `eslint --no-inline-config` over the diff's three `.ts` files exited 0, linting 3 files with 0 errors and 0 warnings. Positive control: a stdin file in `packages/adapters/hono/src/` importing a root namespace from `@objectstack/spec` exits 1 (`no-restricted-imports`). Repo-wide `pnpm lint` is CI's run. ## Changeset `.changeset/23365-hono-declared-envelope.md`, `minor` for both packages. `@objectstack/plugin-hono-server` is `minor` because `Clause-②: yes` (an added export) takes at least `minor`. `@objectstack/hono` is `minor` to match #16545's level: that change applied the same rule to the direct-mount door and shipped as `minor` (`cefe068702`). The wording follows that changeset's sections. ## Acceptance notes - On this host, an undeclared 500 still carries the thrown message verbatim, with no `looksLikeInternalErrorLeak` filter. It was already the case before this change, and it is unchanged here. Noted only; not filed. - For a declared 5xx, the recogniser keeps the message unless `looksLikeInternalErrorLeak` fires. The dispatcher's own exit withholds the message of every declared 5xx (commit 0783d7b). The two Hono doors agree with each other by construction, but not with that exit. Noted only. - `main` gained `989aa483fd` and `0be162c75d` after the base. Neither touches `packages/adapters/hono`, `plugin-hono-server`, `http-conformance`, the dispatcher or `@objectstack/types`, and `git merge-tree` is clean, so the branch was not merged; the merge queue rebuilds on `main`. --- _Generated by [Claude Code](https://claude.ai/code/session_01Q9PZ5Pzqcus4woVvTZwWNU)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent b1cfa3d commit e1cefa5

4 files changed

Lines changed: 369 additions & 3 deletions

File tree

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
---
2+
'@objectstack/hono': minor
3+
'@objectstack/plugin-hono-server': minor
4+
---
5+
6+
fix(hono): `createHonoApp` answers an escaped throw that declares an ADR-0112 envelope as that envelope, not as `500 INTERNAL_ERROR` (#23365)
7+
8+
`@objectstack/hono`'s `createHonoApp` has two catch arms: the auth mount's, and
9+
the `${prefix}/*` catch-all's around `HttpDispatcher.dispatch()`. Both answered
10+
an escaped throw from its `statusCode` alone, so a producer that declared
11+
`status` and a registered `code` lost both on this host. A `GET`, `PATCH` or
12+
`DELETE` of `/data/:object/:id` for an id that names no row answered
13+
`500 INTERNAL_ERROR` where the standalone REST server answers
14+
`404 RECORD_NOT_FOUND`. The `/data` version refusals answered `500` too (a stale
15+
`If-Match`'s `409 CONCURRENT_UPDATE`, and `If-Match: ""`'s
16+
`400 VALIDATION_FAILED`), and so did an authorization-store outage
17+
(`AuthzStoreUnavailableError`, `503 SERVICE_UNAVAILABLE`).
18+
19+
**What changed.** Both arms now answer a throw that carries **both** a declared
20+
ADR-0112 status (a key of `HttpStatusErrorCodeMap`) **and** a code registered in
21+
`ErrorCode` as that envelope. The recognition is `declaredEnvelopeForThrow`, the
22+
rule `@objectstack/plugin-hono-server`'s direct-mount routes have used since
23+
#16545, so the two Hono doors answer one throw alike by construction: the same
24+
two conditions, the same 5xx disclosure filter (`looksLikeInternalErrorLeak`),
25+
and `details` and `userMessage` forwarded only when the producer declared them.
26+
27+
The rule's other limbs come with it, as they did on the direct-mount door. A
28+
validation-shaped throw answers `400 VALIDATION_FAILED` with its `fields[]`. A
29+
driver fault the engine has already classified keeps its `500` and now carries
30+
the code it declared instead of `INTERNAL_ERROR`. Measured: `GET /data/:object`
31+
over an object whose table is missing answers `500 DATABASE_ERROR`, with the
32+
same message as before.
33+
34+
**What did NOT change**, pinned in the same PR through a real `createHonoApp`
35+
over the real dispatcher and protocol (`packages/qa/http-conformance`):
36+
37+
- a throw that is **not** such an envelope answers what it answered before. A
38+
bare `Error` is still `500 INTERNAL_ERROR` carrying `code` and `message`, with
39+
no `cause` in the body, and a throw that declares `statusCode` with no
40+
registered code keeps that status;
41+
- ⛔ no error code is minted and no ledger row is added.
42+
43+
`@objectstack/plugin-hono-server` gains one export, `declaredEnvelopeForThrow`,
44+
which was module-private. `createHonoApp` imports it rather than restating the
45+
rule. Its behaviour is unchanged.
46+
47+
Earlier entries say a `createHonoApp` host still renders the `/data` version
48+
refusals as `500`. This change ends that: they are answered `409` and `400` as
49+
declared.
50+
51+
⚠️ **Known gap, stated rather than left silent.** The `409 CONCURRENT_UPDATE`
52+
this host now answers carries `code` and `message`, but not the
53+
`currentVersion` and `currentRecord` that the standalone REST server's `409`
54+
carries. The recogniser forwards only the `details` a producer declares, and
55+
those two fields are own properties of the error. That gap is outside this
56+
change.
57+
58+
Clause-②: yes (widening)

‎packages/adapters/hono/src/index.ts‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ import {
4444
hasWildcardPattern,
4545
DEFAULT_CORS_ALLOW_HEADERS,
4646
DEFAULT_CORS_EXPOSE_HEADERS,
47+
declaredEnvelopeForThrow,
4748
} from '@objectstack/plugin-hono-server';
4849

4950
export interface ObjectStackHonoCorsOptions {
@@ -508,6 +509,33 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
508509
return c.json({ success: false, error: { code, message } }, status);
509510
};
510511

512+
/**
513+
* [#23365] What BOTH catch arms below answer for a throw that escaped them.
514+
*
515+
* A throw that carries a declared ADR-0112 `status` and a registered `code`
516+
* is answered as that envelope. Both arms used to read `statusCode` alone, so
517+
* a declared `status` and `code` were lost: a missing record's
518+
* `404 RECORD_NOT_FOUND`, the `/data` version refusals' `409` and `400`, and
519+
* an authorization-store outage's `503` all left `dispatch()` and answered
520+
* `500 INTERNAL_ERROR` on this host only.
521+
*
522+
* The recognition is `declaredEnvelopeForThrow`, the ONE rule #16545 landed
523+
* for `@objectstack/plugin-hono-server`'s direct-mount routes, imported and
524+
* ⛔ never restated here, so the two Hono doors answer one throw alike: the
525+
* same two conditions, the same 5xx disclosure filter, and the same body
526+
* (`details` and `userMessage` only when the producer declared them).
527+
*
528+
* A throw the recogniser does not accept takes the arm's answer exactly as
529+
* before: a bare `Error` is still `500` carrying `code` and `message`, with
530+
* no `cause`, and a throw declaring `statusCode` with no registered code
531+
* keeps that status.
532+
*/
533+
const thrownJson = (c: any, err: any) => {
534+
const declared = declaredEnvelopeForThrow(err);
535+
if (declared) return c.json(declared.body, declared.status);
536+
return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500);
537+
};
538+
511539
const toResponse = (c: any, result: HttpDispatcherResult) => {
512540
if (result.handled) {
513541
if (result.response) {
@@ -779,7 +807,11 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
779807
if (!result.handled) return yieldUnowned(c, next, () => toResponse(c, result));
780808
return toResponse(c, result);
781809
} catch (err: any) {
782-
return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500);
810+
// [#23365] The same rendering as the catch-all below (`thrownJson`). No
811+
// in-repo producer was found that reaches this arm with a declared
812+
// envelope (better-auth answers its own failures as a `Response`), so it
813+
// takes the rule for uniformity: one rule for both arms, not two.
814+
return thrownJson(c, err);
783815
}
784816
});
785817

@@ -828,7 +860,9 @@ export function createHonoApp(options: ObjectStackHonoOptions): Hono {
828860
const result = await dispatcher.dispatch(method, subPath, body, queryParams, { request: c.req.raw }, prefix);
829861
return toResponse(c, result);
830862
} catch (err: any) {
831-
return errorJson(c, err.message || 'Internal Server Error', err.statusCode || 500);
863+
// [#23365] A declared ADR-0112 envelope that left `dispatch()` (its foot
864+
// catch re-raises everything but a denial) is answered as declared.
865+
return thrownJson(c, err);
832866
}
833867
});
834868

‎packages/plugins/plugin-hono-server/src/adapter.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -280,8 +280,19 @@ function toLoggableError(thrown: unknown): Error {
280280
* producer's spelling IS the registered member sitting in `code`, so
281281
* `demotedDeclaredCode` returns `undefined` by construction — forwarding it
282282
* would put two spellings of one fact on every envelope this seam renders.
283+
*
284+
* ## Exported for the other Hono door (#23365)
285+
*
286+
* `@objectstack/hono`'s `createHonoApp` is a second Hono door: it builds its own
287+
* app, and its two catch arms (the auth mount's and the `dispatch()`
288+
* catch-all's) read `statusCode` alone, so a declared `status` and `code` were
289+
* lost there and the answer was `500`. Those arms now ask THIS function, so the
290+
* two doors answer one throw alike. It is exported so that they can, and ⛔ it
291+
* is not restated there: a second recogniser is how the doors would start
292+
* disagreeing about what counts as declared. Each caller keeps its own
293+
* fallback arm for the `undefined` answer.
283294
*/
284-
function declaredEnvelopeForThrow(thrown: unknown): {
295+
export function declaredEnvelopeForThrow(thrown: unknown): {
285296
status: number;
286297
body: { success: false; error: Record<string, unknown> };
287298
} | undefined {

0 commit comments

Comments
 (0)