Skip to content

Commit dfa8bad

Browse files
qq9340100claude
andauthored
fix(plugin-hono-server): 逃出 handler 的抛出不再被静默丢弃 (#5848) (#6052)
runHandler() 的兜底 .catch 此前把 rejection 显式丢弃(参数名就是 `_err`), wrap() 随后回一个不带原因的 500。净效果:任何逃出 handler 的抛出,在以本 适配器为 transport 的 host 上都是裸 500 + 零日志 —— 连 stack 都没有。 #4264 诊断的正是这段代码,但它的修法是给三条 datasource 路由各加 catch, 接缝本身没动;check-route-envelope.mjs 结构上看不到这一类(它审计响应写点, 未捕获的抛出根本不写响应)。 现在该接缝按 Logger 契约打一条 error 记录:Error 走契约的 error 形参槽而不是 结构化 meta(message/stack 是 non-enumerable,进 meta 会序列化成 {},比没有 日志更糟 —— 它会报告成功);跨 realm 的 Error 按 name/message/stack 重建; 非 Error 抛出被描述进 message。meta 只带 method + path,不带请求体。 未接线时默认用 createLogger() 而非静默 —— 直接内嵌 HonoHttpServer 的 serverless 入口正是本问题的生产现场。HonoServerPlugin.init() 用 ctx.logger 替换默认值。新增 HonoHttpServer.setLogger(),不改 IHttpServer 契约。 响应形状一字未改(兜底 body 仍是 {"error":"No response from handler"} + 500), 并加测试钉住 —— 收成声明信封属另一项未裁决的契约决策,不随本次改动附带。 Claude-Session: https://claude.ai/code/session_01Wbxm29qPKnLf44AbSxizqW Co-authored-by: Claude <noreply@anthropic.com>
1 parent ef8b1ff commit dfa8bad

5 files changed

Lines changed: 469 additions & 2 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/plugin-hono-server': patch
3+
---
4+
5+
修复:逃出路由 handler 的抛出不再被静默丢弃 —— 适配器接缝现在有诊断出口
6+
7+
`HonoHttpServer.runHandler()` 的兜底 `.catch` 此前把 rejection 显式丢弃(参数名就是 `_err`),`wrap()` 随后回一个 `{ error: 'No response from handler' }` 的 500。净效果是:**任何**逃出 handler 的抛出,在以本适配器为 transport 的 host 上都表现为一个不带原因的裸 500,而且**任何地方都没有日志** —— 连 stack 都没有。
8+
9+
现在该接缝会按 `Logger` 契约打一条 `error` 记录,带上原始 `message` / `stack` 与定位所需的请求上下文(`method` + `path`)。
10+
11+
- **`Error` 走契约的 `error` 形参槽**,不塞进结构化 meta。`Error``message` / `stack` 是 non-enumerable,直接进 meta 会序列化成 `{}` —— 那比没有日志更糟,因为它会报告成功。跨 realm 的 `Error`(`instanceof` 不成立)会按 `name`/`message`/`stack` 重建;`throw 'boom'` 这类非 `Error` 抛出会被描述进 message 而不是丢掉。
12+
- **请求体不入日志** —— 只有 `method``path`
13+
- **默认就有日志出口。** 未接线时适配器用 `createLogger()`,而不是静默:直接内嵌 `HonoHttpServer` 的 host(serverless 入口)正是本问题的生产现场,静默默认会对它们原样复现该 bug。`HonoServerPlugin.init()` 会用 `ctx.logger` 替换掉默认值;要静默须显式传 `NoopLogger`
14+
15+
新增 `HonoHttpServer.setLogger(logger)`(纯新增,不改 `IHttpServer` 契约)。
16+
17+
⚠️ **响应形状一字未改**:兜底 body 仍是 `{ error: 'No response from handler' }` + 500,已加测试钉住。把它收成声明信封会改变线上响应形状,属另一项尚未裁决的契约决策,不随本次改动附带。

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

Lines changed: 127 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,10 @@ export * from '@objectstack/core';
66
import {
77
IHttpServer,
88
RouteHandler,
9-
Middleware
9+
Middleware,
10+
createLogger,
1011
} from '@objectstack/core';
12+
import type { Logger } from '@objectstack/spec/contracts';
1113
import type { Context } from 'hono';
1214
import { currentPerfTiming } from '@objectstack/observability';
1315
import { Hono } from 'hono';
@@ -99,6 +101,57 @@ function readRemoteAddress(c: any): string | undefined {
99101
}
100102
}
101103

104+
/**
105+
* Any thrown value, as a real `Error` whose `message` and `stack` survive
106+
* structured logging.
107+
*
108+
* ## The trap this exists for
109+
*
110+
* `Error.prototype.message` and `.stack` are **non-enumerable**. A rejection
111+
* handed straight to a structured logger's *meta* slot — `{ err }`,
112+
* `{ ...err }`, `JSON.stringify(err)` — therefore serializes to `{}`: the
113+
* record is emitted, the reader sees a log line, and the one thing they needed
114+
* is not in it. That is strictly worse than no log at all, because it reports
115+
* success. (cloud's `objectos-runtime/src/safe-log.ts` header documents the
116+
* same hazard from the other side of the wire.)
117+
*
118+
* The `Logger` contract's `error(message, error?: Error, meta?)` has a
119+
* dedicated `Error` slot precisely so implementations can lift those two
120+
* fields out by name, and all three in-repo implementations do
121+
* (`ObjectLogger`, `ConsoleLogger`, `JsonLogger`). So the adapter's job is
122+
* only to make sure what reaches that slot really is an `Error`:
123+
*
124+
* - a genuine `Error` passes through untouched — original stack preserved;
125+
* - an **error-like** object that fails `instanceof` (a cross-realm `Error`
126+
* from a `vm` context or a worker, the shape a bare spread would flatten to
127+
* `{}`) is rebuilt, carrying its own `name`/`message`/`stack` across;
128+
* - anything else (`throw 'boom'`, `throw { code: 1 }`, `throw undefined`) is
129+
* described in the message rather than dropped, and labelled as a non-Error
130+
* throw so the synthesized stack is not mistaken for the thrower's.
131+
*/
132+
function toLoggableError(thrown: unknown): Error {
133+
if (thrown instanceof Error) return thrown;
134+
135+
if (thrown !== null && typeof thrown === 'object') {
136+
const like = thrown as { name?: unknown; message?: unknown; stack?: unknown };
137+
if (typeof like.message === 'string') {
138+
const rebuilt = new Error(like.message);
139+
if (typeof like.name === 'string') rebuilt.name = like.name;
140+
if (typeof like.stack === 'string') rebuilt.stack = like.stack;
141+
return rebuilt;
142+
}
143+
}
144+
145+
let described: string;
146+
try {
147+
described = typeof thrown === 'string' ? thrown : JSON.stringify(thrown) ?? String(thrown);
148+
} catch {
149+
// Circular / throwing `toJSON` — `String()` still yields something.
150+
described = String(thrown);
151+
}
152+
return new Error(`Non-Error value thrown: ${described}`);
153+
}
154+
102155
/**
103156
* The matched route's path parameters, or `{}` when there is no matched route.
104157
*
@@ -144,6 +197,11 @@ export class HonoHttpServer implements IHttpServer {
144197
private fallbackHandler: RouteHandler | undefined;
145198
/** Whether the Hono `notFound` hook that runs {@link unmatchedResponse} is mounted. */
146199
private notFoundSeamInstalled = false;
200+
/**
201+
* Where {@link reportHandlerFailure} writes. See {@link setLogger} for why
202+
* the default is a REAL logger and not a no-op.
203+
*/
204+
private logger: Logger = createLogger({ name: 'hono' });
147205

148206
constructor(
149207
private port: number = 3000,
@@ -362,9 +420,15 @@ export class HonoHttpServer implements IHttpServer {
362420
closeStream();
363421
resolve({ response: null, failed: false });
364422
}
365-
}).catch((_err) => {
423+
}).catch((err) => {
366424
_endHandler?.();
367425
closeStream();
426+
// The ONE place an escaping throw is reported (#5848). Both
427+
// callers turn `failed: true` into a 500 that says nothing
428+
// about the cause — `wrap`'s `No response from handler` and
429+
// the `notFound` seam's `Fallback handler failed` — so if the
430+
// diagnosis is not emitted here it does not exist anywhere.
431+
this.reportHandlerFailure(c, err);
368432
resolve({ response: null, failed: true });
369433
});
370434
});
@@ -376,6 +440,67 @@ export class HonoHttpServer implements IHttpServer {
376440
};
377441
}
378442

443+
/**
444+
* Point this adapter's diagnostics at the host's logger. Called by
445+
* `HonoServerPlugin.init()` with `ctx.logger`; a host that embeds
446+
* `HonoHttpServer` directly (cloud's serverless entrypoints, tests) may
447+
* call it itself, at any time.
448+
*
449+
* ## Why the default is a real logger, not a no-op (#5848)
450+
*
451+
* The failure this reports is one nobody can see any other way: a throw
452+
* that escapes a route handler produces a 500 carrying no cause, so a
453+
* silent default reproduces exactly the bug — bare 5xx, zero log — for
454+
* every host that forgets to wire this. That is not hypothetical: the
455+
* production report behind #5848 came from a control plane built on the
456+
* BARE adapter, i.e. the path that never sees `ctx.logger`, and its only
457+
* remedy was to re-wrap every route in its own try/catch (cloud#1144) —
458+
* paying off this seam's debt one route at a time, which is the tax
459+
* #4264 already described and did not remove.
460+
*
461+
* So the default is `createLogger()`: level `info` (an `error` always
462+
* passes), secrets redacted by field name, and `message`/`stack` lifted
463+
* out of the `Error` slot by name. Wiring a host logger REPLACES it;
464+
* silencing is a deliberate act (pass a `NoopLogger`), never the default.
465+
*/
466+
setLogger(logger: Logger): void {
467+
this.logger = logger;
468+
}
469+
470+
/**
471+
* Report a throw that escaped a {@link RouteHandler} — the diagnostic exit
472+
* that did not exist before #5848.
473+
*
474+
* Deliberately at `error`, not `warn`: per AGENTS.md "Degradation log
475+
* levels", the third legal answer — "the failure was handed to the CALLER"
476+
* — does NOT apply here. What the caller gets is a bare 500 whose body
477+
* names no cause, no code and no message; they were told that something
478+
* broke, not what, and nothing downstream can reconstruct it. Nor is this
479+
* a validation path that could fire once per malformed keystroke: an
480+
* unhandled throw out of a handler is a server-side defect, and one
481+
* `error` per occurrence is the correct volume.
482+
*
483+
* Method and path only. The request body is NOT logged — it is the most
484+
* likely place for credentials and PII to sit, and `message` + `stack`
485+
* already locate the failure in the code.
486+
*/
487+
private reportHandlerFailure(c: any, thrown: unknown): void {
488+
try {
489+
const method = typeof c?.req?.method === 'string' ? c.req.method : undefined;
490+
const path = typeof c?.req?.path === 'string' ? c.req.path : undefined;
491+
this.logger.error(
492+
'[hono] route handler threw — request answered 500 with no cause in the body',
493+
toLoggableError(thrown),
494+
{ method, path },
495+
);
496+
} catch {
497+
// Reporting the failure must never become a second failure: a
498+
// host logger that throws (or a partial one missing `error`)
499+
// would otherwise reject `runHandler`'s own promise and turn a
500+
// clean 500 into Hono's opaque error page.
501+
}
502+
}
503+
379504
get(path: string, handler: RouteHandler) {
380505
this.registeredRoutes.push({ method: 'GET', pattern: path });
381506
this.app.get(path, this.wrap(handler));

0 commit comments

Comments
 (0)