diff --git a/.changeset/ctx-res-view.md b/.changeset/ctx-res-view.md new file mode 100644 index 0000000..0d1a3bb --- /dev/null +++ b/.changeset/ctx-res-view.md @@ -0,0 +1,5 @@ +--- +"@chttp/router": minor +--- + +feat(ctx): typed `c.res` response view on both surfaces — `.status`, `.header(name)`, `.headers` (plus `statusCode`/`nodeRes()` escape hatches for the Node surface). `use()` middleware now also wraps the notFound chain, and `c.req.url` reports the absolute URL on `app.fetch` (Hono parity for hono/logger's outbound `-->` line). diff --git a/docs/middleware.md b/docs/middleware.md index 134fbc3..63264c1 100644 --- a/docs/middleware.md +++ b/docs/middleware.md @@ -41,7 +41,10 @@ function preflight(c: Ctx): void { .header("access-control-allow-methods", "GET,POST,PUT,DELETE,OPTIONS") .header("access-control-allow-headers", "content-type,authorization") .header("access-control-max-age", "86400"); - c.res.end(); + const rs = c.res.nodeRes(); // raw ServerResponse (Node surface only) + if (rs !== null) { + rs.end(); + } c.sent = true; // tells dispatch this middleware answered } @@ -74,13 +77,19 @@ app.get("/users", (c: Ctx) => { const inm: string | null = c.req.header("if-none-match"); if (inm !== null && inm === etag) { c.status(304); - c.res.end(); + const rs = c.res.nodeRes(); // raw ServerResponse (Node surface only) + if (rs !== null) { + rs.end(); + } c.sent = true; return; } c.header("etag", etag); - c.res.setHeader("content-type", "application/json; charset=utf-8"); - c.res.end(body); + const rs = c.res.nodeRes(); + if (rs !== null) { + rs.setHeader("content-type", "application/json; charset=utf-8"); + rs.end(body); + } c.sent = true; }); ``` @@ -160,7 +169,7 @@ Fixed windows burst at the seam (up to 2×LIMIT across a boundary). Sliding wind Do not hand-roll a static handler: reading files as utf8 corrupts binaries, and the traversal, caching and `index.html` details are easy to get wrong. Use [`@chttp/static`](static.md), which serves bytes through `c.bytes()`, handles ETag/304, HEAD, precompressed siblings and an opt-in SPA fallback, and compiles into the ELF. -If you do need a hand-rolled variant (single small asset set, no package), know the traversal pitfall before you start: `c.req.param("*")` is the *decoded* path tail, so `%2e%2e` arrives as `..` before your code sees it, so guard on the decoded value, reject any tail containing `..` or a leading `/`, and write the response manually with `c.res.setHeader` + `c.res.end()`, because `c.text()` forces its own `content-type`. +If you do need a hand-rolled variant (single small asset set, no package), know the traversal pitfall before you start: `c.req.param("*")` is the *decoded* path tail, so `%2e%2e` arrives as `..` before your code sees it, so guard on the decoded value, reject any tail containing `..` or a leading `/`, and write the response manually through `c.res.nodeRes()` (the raw `ServerResponse`, null on the `app.fetch` surface) with `rs.setHeader` + `rs.end()`, because `c.text()` forces its own `content-type`. ### Timing header diff --git a/examples/integrations/hono-middleware/test/logger.test.ts b/examples/integrations/hono-middleware/test/logger.test.ts index ad49602..708e1b0 100644 --- a/examples/integrations/hono-middleware/test/logger.test.ts +++ b/examples/integrations/hono-middleware/test/logger.test.ts @@ -32,18 +32,9 @@ function captureStdout(): { read: () => string; restore: () => void } { }; } -// Known limitation: hono/logger reads `c.res.status` after `await next()` -// to compose the outbound log line. chttp's `c.res` returns the -// underlying ServerResponse (Node path) or null (fetch path); neither -// exposes a `.status` property — ServerResponse uses `.statusCode`, and -// the fetch path has no response object. hono/logger therefore throws -// on the outbound line, and only the inbound line (<-- ...) ever -// prints. This is tracked in TODO #__ (file follow-up after this PR): -// either surface a Response-shaped `c.res` on the fetch path that -// scriptc can lower, or report the type issue to scriptc if Hono's -// typing is actually correct. The test below asserts only the inbound -// line; flipping the outbound assertion back on is the close-out for -// that follow-up. +// `c.res` is the typed response view added for Hono compat (issue #57): +// `.status` reads through the sink on both surfaces, so hono/logger's +// outbound `-->` line prints the real status code after `await next()`. test("hono/logger prints the inbound <-- line on a 200 response", async () => { const app = createApp(); @@ -55,14 +46,14 @@ test("hono/logger prints the inbound <-- line on a 200 response", async () => { } finally { cap.restore(); } - const out = cap.read(); + const out = cap.read().replace(/\x1b\[\d+m/g, ""); // The inbound line proves c.req.url is populated and that // app.use('*', mw) is honored by chttp's middleware chain. Both // landed in this PR. assert.match(out, /<-- GET \//, "expected inbound log line, got: " + out); }); -test.skip("hono/logger outbound --> line (blocked on c.res.status on fetch path)", async () => { +test("hono/logger outbound --> line (c.res.status on fetch path)", async () => { const app = createApp(); const cap = captureStdout(); try { @@ -73,10 +64,10 @@ test.skip("hono/logger outbound --> line (blocked on c.res.status on fetch path) cap.restore(); } const out = cap.read(); - assert.match(out, /--> GET \/\s*200\b/, "expected outbound log line with 200, got: " + out); + assert.match(out.replace(/\x1b\[\d+m/g, ""), /--> GET \/\s*200\b/, "expected outbound log line with 200, got: " + out); }); -test.skip("hono/logger outbound --> line for 404 (blocked on c.res.status on fetch path)", async () => { +test("hono/logger outbound --> line for 404 (c.res.status on fetch path)", async () => { const app = createApp(); const cap = captureStdout(); try { @@ -87,5 +78,5 @@ test.skip("hono/logger outbound --> line for 404 (blocked on c.res.status on fet cap.restore(); } const out = cap.read(); - assert.match(out, /--> GET \/missing\s*404\b/, "expected 404 in outbound log, got: " + out); + assert.match(out.replace(/\x1b\[\d+m/g, ""), /--> GET \/missing\s*404\b/, "expected 404 in outbound log, got: " + out); }); \ No newline at end of file diff --git a/examples/integrations/otlp-traces/src/app.ts b/examples/integrations/otlp-traces/src/app.ts index 3025646..9c547b3 100644 --- a/examples/integrations/otlp-traces/src/app.ts +++ b/examples/integrations/otlp-traces/src/app.ts @@ -18,7 +18,7 @@ export function createApp(exporter: Exporter) { name: c.req.method + " " + c.req.path, startMs: startMs, endMs: Date.now(), - httpStatus: c.res === null ? 0 : c.res.statusCode, + httpStatus: c.res.statusCode, }); }); diff --git a/examples/integrations/prometheus-metrics/src/app.ts b/examples/integrations/prometheus-metrics/src/app.ts index ff058d2..a67d060 100644 --- a/examples/integrations/prometheus-metrics/src/app.ts +++ b/examples/integrations/prometheus-metrics/src/app.ts @@ -11,7 +11,7 @@ export function createApp(record: Record) { app.use(async (c: Ctx, next) => { const t0 = Date.now(); await next(); - record(c.req.method, c.res === null ? 0 : c.res.statusCode, (Date.now() - t0) / 1000); + record(c.req.method, c.res.statusCode, (Date.now() - t0) / 1000); }); app.get("/", (c) => c.json({ ok: true })); diff --git a/packages/router/src/index.ts b/packages/router/src/index.ts index 8fc900d..eacb15b 100644 --- a/packages/router/src/index.ts +++ b/packages/router/src/index.ts @@ -151,11 +151,12 @@ export class ReqViewCore { // joined with ", " in copyHeaders). _headersDistinct: Record; method: string; - // Full request URL (path + query, no host). The Node path stores the - // raw `req.url` value; the app.fetch path stores the parsed - // path+query from the Web Request. Hono middlewares (logger, etc.) - // and Web Standards callers read `c.req.url` to recover the request - // line without splitting `path` + `_qs` themselves. + // Full request URL. The Node path stores the raw `req.url` value + // (path + query, no host — IncomingMessage carries no host). The + // app.fetch path stores the Web Request's ABSOLUTE URL, matching + // Hono's c.req.url so middlewares that derive the path themselves + // (hono/logger's url.slice(indexOf("/", 8))) parse it correctly. + // `path`/`_qs` remain the path+query split either way. url: string; path: string; _qs: string; @@ -216,9 +217,11 @@ export class ReqViewCore { // from a Record local in header(). this._headersDistinct = copyHeadersDistinct(raw.headersDistinct); // `url` is the full path+query string the caller saw. On the - // Node path it equals `req.url` (path+query, no host); on the - // app.fetch path it equals the path+query portion of the Web - // Request URL. We accept it as an optional 4th arg so the Node + // Node path it equals `req.url` (path+query, no host). On the + // app.fetch path the constructor arg is the parsed path+query + // used for matching; _dispatchPrim then overwrites `url` with the + // Web Request's absolute URL for Hono parity (see the field's + // comment). We accept it as an optional 4th arg so the Node // dispatch can default to `raw.url` and the fetch path can // pass the already-parsed value (avoids a second split). this.url = url === undefined ? String(raw.url || path) : String(url); @@ -480,9 +483,48 @@ export class ReqViewCore { } } +// CtxResCore is the typed read-back view `c.res` exposes on BOTH surfaces +// (issue #57): `status`, `header(name)`, and `headers` read through the +// sink's uniform accessors, so Hono's outbound logger (`c.res.status` +// after `await next()`) and the compress/etag/prettyJSON middlewares see +// the same shape under app.handler and app.fetch. Read-only — response +// writes still go through c.status()/c.header()/the body helpers. +export class CtxResCore { + _sink: ResponseSink; + + constructor(sink: ResponseSink) { + this._sink = sink; + } + + get status(): number { + return this._sink.statusOf(); + } + + // ServerResponse-shaped alias so Node-shaped callers reading + // `c.res.statusCode` keep working on the Node surface. + get statusCode(): number { + return this._sink.statusOf(); + } + + header(name: string): string | null { + return this._sink.headerOf(String(name)); + } + + get headers(): { [k: string]: string } { + return this._sink.headersMap(); + } + + // Escape hatch to the underlying ServerResponse on the Node surface + // (c.res.end() 304-style paths, res.writeHead); null under app.fetch. + nodeRes(): ServerResponse | null { + return this._sink.nodeRes(); + } +} + export class CtxCore { req: ReqViewCore; sink: ResponseSink; + _resView: CtxResCore; _status: number; // True once a body-carrying helper (_send/redirect) flushed the response. // runOnError reads it: an onError that already answered must not get a @@ -500,11 +542,12 @@ export class CtxCore { // assignment at the constructor branches keeps it typed. _nodeRes: ServerResponse | null; - // Legacy accessor for downstream Node-shaped code (e.g. @chttp/static's - // c.res.end() 304 path, @chttp/stream's res.writeHead). null under the - // Fetch surface — callers must check. - get res(): ServerResponse | null { - return this._nodeRes; + // Typed response view exposed on both surfaces (issue #57). Replaces + // the old `ServerResponse | null` accessor: Node-shaped downstream code + // that needs the raw ServerResponse goes through `c.res.nodeRes()` (or + // the `c._nodeRes` field, used by @chttp/static and @chttp/stream). + get res(): CtxResCore { + return this._resView; } constructor(req: ReqViewCore, sink: ResponseSink) { @@ -520,6 +563,7 @@ export class CtxCore { // generated dispatch) without a cast. this.sink = sink; this._nodeRes = sink.nodeRes(); + this._resView = new CtxResCore(sink); this._status = 200; this.sent = false; this._vars = "{}"; @@ -626,6 +670,8 @@ export interface ReqView

extends ReqViewCore { rawReq(): IncomingMessage | WebReqView; } export const ReqView = ReqViewCore; +export interface CtxRes extends CtxResCore {} +export const CtxRes = CtxResCore; // Param names inferred from a path literal: '/user/:id' -> 'id', // '/a/:x/b/:y' -> 'x' | 'y', '/w/*' -> '*'. Degrades to `string` when the @@ -1536,9 +1582,14 @@ export class App { return next(); }; this._mw.push(wrapped); + // Keep the notFound chain in step: use() middleware wraps 404s too + // (Hono semantics — hono/logger's outbound line must fire on a miss; + // issue #57). Per-route snapshots in _mws are unaffected. + this._nfchain = compose(copyMw(this._mw), this._nf); return this; } this._mw.push(pathOrFn); + this._nfchain = compose(copyMw(this._mw), this._nf); return this; } @@ -1826,6 +1877,14 @@ export class App { // it (Hono parity). The stream is left untouched here — body reading // is declaration-driven below. rv._setWeb(webReq); + // Hono parity: c.req.url is the ABSOLUTE URL on the fetch surface — + // hono/logger derives the path as url.slice(url.indexOf("/", 8)), + // which misreads a path-only value ("g" for "/missing"). The Node + // surface stays path+query-only because IncomingMessage.url carries + // no host (issue #57). + if (webReq !== null) { + rv.url = String(webReq.url); + } // Lazy bodies: multipart routes arrive with their bytes already read // (app.fetch's arrayBuffer for byte-exact parsing); fetchShim seeds a // utf8 string. Everything else defers to the route's bodyFlag: declared diff --git a/packages/router/src/sink.ts b/packages/router/src/sink.ts index 7a71385..32d6bc1 100644 --- a/packages/router/src/sink.ts +++ b/packages/router/src/sink.ts @@ -44,6 +44,19 @@ export class ResponseSink { return false; } destroy(): void {} + // Read-back accessors backing Ctx.res (issue #57): a uniform status / + // header view over both surfaces so `c.res.status` resolves identically + // under app.handler and app.fetch. Base defaults are no-ops like the + // write-side members; concrete sinks override. + statusOf(): number { + return 200; + } + headerOf(_k: string): string | null { + return null; + } + headersMap(): { [k: string]: string } { + return {}; + } // Underlying ServerResponse when the sink wraps one; null otherwise // (FetchResponseSink). A method rather than a cast so callers can // discriminate sink flavors without `instanceof`/`as` — both are @@ -155,6 +168,27 @@ export class NodeResponseSink extends ResponseSink { } } + statusOf(): number { + return this._status; + } + + headerOf(k: string): string | null { + const v = this._set[String(k).toLowerCase()]; + if (v === undefined) { + return null; + } + return v; + } + + headersMap(): { [k: string]: string } { + const out: { [k: string]: string } = {}; + const keys = Object.keys(this._set); + for (let i = 0; i < keys.length; i++) { + out[keys[i]] = this._set[keys[i]]; + } + return out; + } + nodeRes(): ServerResponse | null { return this.res; } @@ -314,6 +348,28 @@ export class FetchResponseSink extends ResponseSink { return this.headersSent; } + statusOf(): number { + return this.status; + } + + headerOf(k: string): string | null { + const kk = String(k).toLowerCase(); + for (let i = 0; i < this.hdrKeys.length; i++) { + if (this.hdrKeys[i].toLowerCase() === kk) { + return this.hdrVals[i]; + } + } + return null; + } + + headersMap(): { [k: string]: string } { + const out: { [k: string]: string } = {}; + for (let i = 0; i < this.hdrKeys.length; i++) { + out[this.hdrKeys[i]] = this.hdrVals[i]; + } + return out; + } + destroy(): void { // Web fetch surface: no socket to destroy. Caller aborts the Request // signal upstream if they need to. diff --git a/packages/router/test/fetch-passthrough.test.js b/packages/router/test/fetch-passthrough.test.js index 0852251..77b01c1 100644 --- a/packages/router/test/fetch-passthrough.test.js +++ b/packages/router/test/fetch-passthrough.test.js @@ -158,7 +158,6 @@ test("405/404 unchanged: method miss is 405 with Allow, miss is bare Not Found", assert.equal(miss404.status, 404); assert.equal(await miss404.text(), "Not Found"); }); - test("empty bodies carry no content-type on the fetch surface (Hono parity)", async () => { const app = createRouter(); app.get("/go", (c) => c.redirect("/dest")); @@ -174,3 +173,36 @@ test("empty bodies carry no content-type on the fetch surface (Hono parity)", as assert.equal(nf.status, 404); assert.equal(nf.headers.get("content-type"), "text/plain; charset=UTF-8"); }); + +// c.res view (issue #57): the typed response view exposes .status, +// .header(name), and .headers on the Web Standards surface, reading the +// FetchResponseSink's captured state — so a middleware that awaits next() +// sees the final status and headers, same as the Node surface. +test("c.res view: status reflects a 404 after next() settles", async () => { + const app = createRouter(); + let seen = null; + app.use("*", async (c, next) => { + await next(); + seen = c.res.status; + }); + app.get("/ok", (c) => c.text("ok")); + const res = await app.fetch(new Request("http://x/missing")); + assert.equal(res.status, 404); + assert.equal(seen, 404); +}); + +test("c.res view: header('content-type') reflects a json() write", async () => { + const app = createRouter(); + let ct = null; + let all = null; + app.use("*", async (c, next) => { + await next(); + ct = c.res.header("content-type"); + all = c.res.headers; + }); + app.get("/json", (c) => c.json({ ok: true })); + const res = await app.fetch(new Request("http://x/json")); + assert.equal(res.status, 200); + assert.equal(ct, "application/json; charset=utf-8"); + assert.equal(all["content-type"], "application/json; charset=utf-8"); +}); diff --git a/packages/router/test/router.test.js b/packages/router/test/router.test.js index 0d491ae..497ca3b 100644 --- a/packages/router/test/router.test.js +++ b/packages/router/test/router.test.js @@ -729,3 +729,34 @@ test("fetchShim drives the fetch dispatch path with primitives", async () => { } assert.ok(allow.length > 0); }); + +// c.res view (issue #57): same typed view on the Node surface, backed by +// NodeResponseSink's tracked status + header table. +test("c.res view: status reflects a 404 after next() settles (node)", async () => { + const app = createRouter(); + let seen = null; + app.use("*", async (c, next) => { + await next(); + seen = c.res.status; + }); + app.get("/ok", (c) => c.text("ok")); + const res = call(app, { url: "/missing" }); + assert.equal(res.statusCode, 404); + // The async middleware's post-next() continuation runs on a microtask + // after the sync call() returns. + await new Promise((r) => setImmediate(r)); + assert.equal(seen, 404); +}); + +test("c.res view: header('content-type') reflects a json() write (node)", async () => { + const app = createRouter(); + let ct = null; + app.use("*", async (c, next) => { + await next(); + ct = c.res.header("content-type"); + }); + app.get("/json", (c) => c.json({ ok: true })); + call(app, { url: "/json" }); + await new Promise((r) => setImmediate(r)); + assert.equal(ct, "application/json; charset=utf-8"); +});