Skip to content

Commit 2b995cb

Browse files
committed
fix(plugin-hono-server): render an escaped throw's declared ADR-0112 envelope
`HonoHttpServer.wrap()` is the seam every direct-mount route passes, and it answered every escaped throw as `500 { code: 'INTERNAL_ERROR', message: 'No response from handler' }` with the thrown value discarded. A producer that had declared its refusal lost both halves of the declaration on the way out. An escaped throw carrying BOTH a declared ADR-0112 status (a key of `HttpStatusErrorCodeMap`) AND a code registered in `ErrorCode` is now answered as that envelope, with `details` and `userMessage` forwarded. Status and code are read through `resolveThrownHttpError` -- the rule the REST registrar and the dispatcher already share -- so the doors agree by construction, not by a second ladder. The 5xx disclosure filter (`looksLikeInternalErrorLeak`) applies from this seam's first day. Everything else is unchanged and pinned: a non-envelope throw, a partial declaration, an unregistered code, an undeclared status, a handler that wrote nothing, and a handler that wrote then threw. The `notFound` fallback seam still answers `Fallback handler failed`. No error code is minted and no ledger row is added. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DapQyvYrFb1MxSYe7BL2nt
1 parent e1eee43 commit 2b995cb

4 files changed

Lines changed: 851 additions & 16 deletions

File tree

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
---
2+
'@objectstack/plugin-hono-server': minor
3+
---
4+
5+
fix(plugin-hono-server): an escaped throw that declares an ADR-0112 envelope is answered as that envelope, not as a bare `500 INTERNAL_ERROR "No response from handler"` (#16545)
6+
7+
`HonoHttpServer.wrap()` is the seam **every direct-mount route passes**`get` /
8+
`post` / `put` / `delete` / `patch` each register `this.wrap(handler)`, and
9+
`IHttpServer` is how `service-datasource`, `packages/rest` and the dispatcher
10+
bridge all mount. Until now a throw that escaped a route handler was answered
11+
there as `500 { code: 'INTERNAL_ERROR', message: 'No response from handler' }`,
12+
with the thrown value discarded — so a producer that had *declared* its refusal
13+
lost both halves of the declaration on the way to the caller.
14+
15+
The measured case: `service-datasource`'s `requireDatasourceAdmin` re-raises
16+
`AuthzStoreUnavailableError` (declared `status: 503`, declared `code:
17+
SERVICE_UNAVAILABLE`) when the authorization store cannot be read — deliberately,
18+
per the #13279 ruling that an unreadable store licenses no verdict. The operator's
19+
outage reached the caller as a generic fault naming the wrong component: the
20+
declared code never arrived, and the message said "No response from handler".
21+
22+
**What changed.** An escaped throw carrying **both** a declared ADR-0112 status
23+
(a key of `HttpStatusErrorCodeMap`) **and** a code registered in `ErrorCode`
24+
(`StandardErrorCode``ERROR_CODE_LEDGER`) is now rendered as that envelope,
25+
with the producer's `details` and `userMessage` channels forwarded. The status
26+
and code are read through `resolveThrownHttpError` — the one rule the REST
27+
registrar and the dispatcher already share — so this seam agrees with the other
28+
doors by construction rather than by a second ladder.
29+
30+
**What did NOT change**, pinned in the same PR:
31+
32+
- an escaped throw that is **not** such an envelope answers exactly the bytes it
33+
answered before — 500, no cause in the body. A partial declaration (status but
34+
no code, code but no status), an unregistered code, and a status ADR-0112 does
35+
not declare all take that arm;
36+
- a handler that simply wrote nothing is untouched;
37+
- a handler that **wrote and then threw** keeps what it wrote;
38+
- the `notFound` fallback seam still answers `Fallback handler failed` — a
39+
fallback that threw is a broken consumer, not a refusal it declared;
40+
- ⛔ no error code is minted and no ledger row is added. A code on this path that
41+
is not registered is a ledger gap under the #16404 ruling, and takes the
42+
unchanged 500 arm rather than being registered in passing.
43+
44+
The 5xx disclosure filter every door emitting a thrown message already runs
45+
(`looksLikeInternalErrorLeak`, #3867 / #8086) is applied here from this seam's
46+
first day: a driver dump on a declared 5xx is withheld, where the old bare 500
47+
disclosed nothing at all. The escaped-throw diagnosis (#5848) still fires exactly
48+
once at `error`, and now names the answer that was really sent instead of
49+
claiming an opaque 500.
50+
51+
⚠️ **Known-unreached door, stated rather than left silent.** A route mounted
52+
through `getRawApp()` funnels through neither `wrap()` nor any registrar wrapper,
53+
so it is **not** repaired by this change and still answers a non-envelope
54+
`text/plain` 500. That is out of this card's scope by the `domain:cli` seat's
55+
ruling and is filed separately.

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

Lines changed: 202 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,21 @@ import { routePath } from 'hono/route';
2727
import { serve } from '@hono/node-server';
2828
import { serveStatic } from '@hono/node-server/serve-static';
2929
import { matchesRoutePattern } from './route-pattern';
30+
// The ADR-0112 wire vocabulary, read as DATA rather than restated: `ErrorCode`
31+
// is the closed union (`StandardErrorCode` ∪ `ERROR_CODE_LEDGER`) a registered
32+
// code must be a member of, and `HttpStatusErrorCodeMap` IS the set of statuses
33+
// ADR-0112 declares — the same table `standardErrorCodeForHttpStatus` derives
34+
// from, so "a declared ADR-0112 status" needs no second list here.
35+
import { ErrorCode, HttpStatusErrorCodeMap } from '@objectstack/spec/api';
36+
// The ONE rule for "what HTTP answer does a THROWN error declare?" (#8016), and
37+
// the 5xx disclosure filter every door that emits a thrown message already runs
38+
// (#3867 / #8086). Both are CALLED, never restated — a second ladder here is
39+
// how the `/api/v1/packages` two-door divergence arose in the first place.
40+
import {
41+
resolveThrownHttpError,
42+
looksLikeInternalErrorLeak,
43+
INTERNAL_ERROR_MESSAGE,
44+
} from '@objectstack/types';
3045

3146
/**
3247
* Request headers allowed on preflight, by default.
@@ -163,6 +178,116 @@ function toLoggableError(thrown: unknown): Error {
163178
return new Error(`Non-Error value thrown: ${described}`);
164179
}
165180

181+
/**
182+
* The declared ADR-0112 envelope an escaped throw CARRIES, or `undefined` when
183+
* it carries none (#16545, the `domain:cli` half of the #15999 ruling).
184+
*
185+
* ## What the ruling asked for
186+
*
187+
* > **Shared half** (`domain:cli`, hono adapter / registrar wrapper): an
188+
* > escaped throw carrying a declared ADR-0112 `status` + registered `code` is
189+
* > rendered by them, not as a bare `500 INTERNAL_ERROR "No response from
190+
* > handler"`. This changes what an escaped throw means for every direct-mount
191+
* > route; the PR pins that an escaped **non**-envelope throw still answers 500
192+
* > with no cause in the body.
193+
*
194+
* The measured motivating path: `service-datasource`'s `requireDatasourceAdmin`
195+
* re-raises `AuthzStoreUnavailableError` (declared `status: 503` / `code:
196+
* SERVICE_UNAVAILABLE`) on an unreadable authorization store, deliberately and
197+
* per the #13279 ruling — and the caller was told `500 INTERNAL_ERROR "No
198+
* response from handler"`. The declared code never reached the caller and the
199+
* message named the wrong component. Only the RENDERING moves here; #13279's
200+
* discipline (an unreadable authz store licenses no verdict) is untouched.
201+
*
202+
* ## Why this is a GATE and not `sendThrownError`
203+
*
204+
* `packages/rest`'s `sendThrownError` maps EVERY throw through
205+
* `resolveThrownHttpError`, so an undeclared fault arrives as `500
206+
* INTERNAL_ERROR` carrying the thrown message. That is right for a REST
207+
* registrar, whose bodies are parsed against `BaseResponseSchema` by its own
208+
* conformance suite. It is NOT what this seam may do: the ruling pins that a
209+
* non-envelope throw keeps today's behaviour EXACTLY — 500, and no cause in the
210+
* body — so the fallback arm must stay byte-identical rather than gain the
211+
* thrown message. Hence a gate that answers `undefined` for everything the
212+
* ruling did not name, and `wrap`'s existing literal for that arm.
213+
*
214+
* ## The two conditions, both read off the ONE rule
215+
*
216+
* `resolveThrownHttpError` reports the DECLARATION it read — `declaredStatus`
217+
* is absent exactly when the throw declared no status (its docblock states the
218+
* distinction and why `status` cannot answer it), and `declaredCode` is the
219+
* producer's own spelling. So neither condition re-spells that function's
220+
* precedence chain here; a second chain is the divergence #8016 removed.
221+
*
222+
* 1. **a declared ADR-0112 status** — a key of `HttpStatusErrorCodeMap`. That
223+
* table is ADR-0112's own status list, so the vocabulary has one home. A
224+
* producer that declares `418` or `599` is NOT naming an ADR-0112 status
225+
* and takes the fallback arm.
226+
* 2. **a registered code** — a member of `ErrorCode`, i.e. `StandardErrorCode`
227+
* ∪ `ERROR_CODE_LEDGER`. ⛔ No code is minted here and no ledger row is
228+
* added; a code this path carries that is NOT registered is a ledger gap
229+
* under the #16404 ruling and takes the fallback arm rather than being
230+
* registered in passing.
231+
*
232+
* ⚠️ **Blast radius, stated because it is wider than the motivating path.**
233+
* `resolveThrownHttpError` treats the validation SHAPE as a declaration too
234+
* (`err.name === 'ValidationError'` ⇒ `400` / `VALIDATION_FAILED`), so a bare
235+
* `ValidationError` escaping a direct-mount handler now answers `400
236+
* VALIDATION_FAILED` with its `fields[]` instead of a bare 500. That is the one
237+
* rule's own semantics, and second-guessing one of its limbs at this door is
238+
* precisely how two doors start disagreeing — so it is accepted and recorded,
239+
* not carved out.
240+
*
241+
* ⛔ `declaredCode` is deliberately NOT forwarded. Under this gate the
242+
* producer's spelling IS the registered member sitting in `code`, so
243+
* `demotedDeclaredCode` returns `undefined` by construction — forwarding it
244+
* would put two spellings of one fact on every envelope this seam renders.
245+
*/
246+
function declaredEnvelopeForThrow(thrown: unknown): {
247+
status: number;
248+
body: { success: false; error: Record<string, unknown> };
249+
} | undefined {
250+
const resolved = resolveThrownHttpError(thrown);
251+
252+
// Condition 1 — the throw DECLARED a status, and it is one ADR-0112 names.
253+
if (resolved.declaredStatus === undefined) return undefined;
254+
if (!Object.prototype.hasOwnProperty.call(HttpStatusErrorCodeMap, resolved.declaredStatus)) {
255+
return undefined;
256+
}
257+
// Condition 2 — the producer's OWN code is a member of the closed union.
258+
if (resolved.declaredCode === undefined) return undefined;
259+
if (!ErrorCode.safeParse(resolved.declaredCode).success) return undefined;
260+
261+
// The 5xx disclosure filter every door emitting a thrown message runs
262+
// (`HttpDispatcher.error` since #3867, `packages/rest`'s registrars since
263+
// #8086). This seam becomes such a door with this change, so it owes the
264+
// rule from its first day: without it a driver dump reaching a declared
265+
// 5xx would newly travel to the client, where the old bare 500 disclosed
266+
// nothing. Scoped to 5xx, like the twins: a 4xx message is a
267+
// caller-facing answer by design.
268+
const message = resolved.status >= 500 && looksLikeInternalErrorLeak(resolved.message)
269+
? INTERNAL_ERROR_MESSAGE
270+
: resolved.message;
271+
272+
return {
273+
status: resolved.status,
274+
body: {
275+
success: false,
276+
error: {
277+
code: resolved.code,
278+
message,
279+
// The producer's structured context and its END-USER-addressed
280+
// refusal text (#9934), forwarded exactly as the REST twin
281+
// forwards them. Both are absent unless the producer declared
282+
// them, so a throw that carried neither renders the same two
283+
// keys it always did.
284+
...(resolved.details ? { details: resolved.details } : {}),
285+
...(resolved.userMessage !== undefined ? { userMessage: resolved.userMessage } : {}),
286+
},
287+
},
288+
};
289+
}
290+
166291
/**
167292
* The matched route's path parameters, or `{}` when there is no matched route.
168293
*
@@ -296,7 +421,18 @@ export class HonoHttpServer implements IHttpServer {
296421
// internal helper to convert standard handler to Hono handler
297422
private wrap(handler: RouteHandler) {
298423
return async (c: any) => {
299-
const { response } = await this.runHandler(c, handler);
424+
// `renderDeclaredEnvelope` is the #16545 opt-in, and it is opt-IN
425+
// rather than the default because the OTHER caller of `runHandler`
426+
// — the `notFound` seam — must keep answering `Fallback handler
427+
// failed`: a fallback that threw is a broken consumer, not a
428+
// refusal the consumer declared. The ruling names direct-mount
429+
// ROUTES, which is exactly this call site.
430+
const { response } = await this.runHandler(c, handler, {
431+
renderDeclaredEnvelope: true,
432+
});
433+
// Unchanged, and pinned byte-for-byte: a throw that declared no
434+
// ADR-0112 envelope, and a handler that simply wrote nothing, both
435+
// still answer 500 with no cause in the body.
300436
return response ?? c.json(
301437
{
302438
success: false,
@@ -332,6 +468,15 @@ export class HonoHttpServer implements IHttpServer {
332468
private async runHandler(
333469
c: any,
334470
handler: RouteHandler,
471+
opts: {
472+
/**
473+
* Render an escaped throw that carries a declared ADR-0112 status
474+
* and a registered code as THAT envelope (#16545). Off by default
475+
* — see {@link declaredEnvelopeForThrow} for the rule and
476+
* {@link wrap} for why only the route caller opts in.
477+
*/
478+
renderDeclaredEnvelope?: boolean;
479+
} = {},
335480
): Promise<{ response: Response | null; failed: boolean }> {
336481
let body: any = {};
337482

@@ -465,7 +610,7 @@ export class HonoHttpServer implements IHttpServer {
465610

466611
// Create a streaming response wrapper — if handler calls res.write(),
467612
// we return a ReadableStream; otherwise fall back to capturedResponse.
468-
const streamPromise = new Promise<{ response: Response | null; failed: boolean }>((resolve) => {
613+
const streamPromise = new Promise<{ response: Response | null; failed: boolean; thrown?: unknown }>((resolve) => {
469614
const stream = new ReadableStream({
470615
start(controller) {
471616
streamController = controller;
@@ -506,21 +651,37 @@ export class HonoHttpServer implements IHttpServer {
506651
}).catch((err) => {
507652
_endHandler?.();
508653
closeStream();
509-
// The ONE place an escaping throw is reported (#5848). Both
510-
// callers turn `failed: true` into a 500 that says nothing
511-
// about the cause — `wrap`'s `No response from handler` and
512-
// the `notFound` seam's `Fallback handler failed` — so if the
513-
// diagnosis is not emitted here it does not exist anywhere.
514-
this.reportHandlerFailure(c, err);
515-
resolve({ response: null, failed: true });
654+
// [#16545] The throw is CARRIED OUT rather than reported here.
655+
// Reporting moved below so the diagnosis can name the answer
656+
// that was actually sent: since this seam may now render a
657+
// declared envelope, a line hard-coding "answered 500 with no
658+
// cause" would be false for exactly the requests the render
659+
// exists to fix. Still reported exactly once per escaped
660+
// throw, and still the ONLY place it is reported (#5848).
661+
resolve({ response: null, failed: true, thrown: err });
516662
});
517663
});
518664

519665
const outcome = await streamPromise;
520-
return {
521-
response: outcome.response ?? capturedResponse ?? null,
522-
failed: outcome.failed,
523-
};
666+
// A handler that WROTE and then threw keeps what it wrote — unchanged,
667+
// and the reason the render decision is taken here rather than in the
668+
// `catch`: `capturedResponse` is not visible from inside the executor's
669+
// rejection path, so deciding there would have let a declared envelope
670+
// overwrite a response the handler had already produced.
671+
let response = outcome.response ?? capturedResponse ?? null;
672+
let rendered: { status: number; code: unknown } | undefined;
673+
674+
if (outcome.failed && response === null && opts.renderDeclaredEnvelope) {
675+
const envelope = declaredEnvelopeForThrow(outcome.thrown);
676+
if (envelope) {
677+
response = c.json(envelope.body, envelope.status);
678+
rendered = { status: envelope.status, code: envelope.body.error.code };
679+
}
680+
}
681+
682+
if (outcome.failed) this.reportHandlerFailure(c, outcome.thrown, rendered);
683+
684+
return { response, failed: outcome.failed };
524685
}
525686

526687
/**
@@ -567,14 +728,39 @@ export class HonoHttpServer implements IHttpServer {
567728
* likely place for credentials and PII to sit, and `message` + `stack`
568729
* already locate the failure in the code.
569730
*/
570-
private reportHandlerFailure(c: any, thrown: unknown): void {
731+
private reportHandlerFailure(
732+
c: any,
733+
thrown: unknown,
734+
/**
735+
* [#16545] What the caller actually answered, when the throw carried a
736+
* declared ADR-0112 envelope and this seam rendered it. Absent for
737+
* every throw that took the unchanged bare-500 arm.
738+
*
739+
* The log line branches on it because the old sentence is a factual
740+
* CLAIM about the response — "answered 500 with no cause in the body"
741+
* — and it stops being true for precisely the requests this card
742+
* repairs. An operator reading `503 SERVICE_UNAVAILABLE` on the wire
743+
* beside a log line insisting the caller got an opaque 500 would be
744+
* debugging the seam instead of the outage.
745+
*/
746+
rendered?: { status: number; code: unknown },
747+
): void {
571748
try {
572749
const method = typeof c?.req?.method === 'string' ? c.req.method : undefined;
573750
const path = typeof c?.req?.path === 'string' ? c.req.path : undefined;
751+
// Still `error`, in BOTH arms. A rendered envelope makes the answer
752+
// honest; it does not make the escape intentional — a handler that
753+
// throws its refusal past its own `catch` is still a server-side
754+
// defect, and the AGENTS.md "handed to the CALLER" exemption does
755+
// not apply to a throw nobody caught.
574756
this.logger.error(
575-
'[hono] route handler threw — request answered 500 with no cause in the body',
757+
rendered
758+
? '[hono] route handler threw — request answered with the throw\'s declared ADR-0112 envelope'
759+
: '[hono] route handler threw — request answered 500 with no cause in the body',
576760
toLoggableError(thrown),
577-
{ method, path },
761+
rendered
762+
? { method, path, status: rendered.status, code: rendered.code }
763+
: { method, path },
578764
);
579765
} catch {
580766
// Reporting the failure must never become a second failure: a

0 commit comments

Comments
 (0)