From 94b1ba7a45c899557b0e928ababcd8f8dfd93d6d Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sun, 11 May 2025 16:10:03 +0900 Subject: [PATCH 1/9] fix: tweaks interface of `getInternalBody()` --- src/response.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/response.ts b/src/response.ts index 5a50a73..8218913 100644 --- a/src/response.ts +++ b/src/response.ts @@ -19,7 +19,7 @@ export class Response { #body?: BodyInit | null #init?: ResponseInit; - [getResponseCache](): typeof GlobalResponse { + [getResponseCache](): globalThis.Response { delete (this as any)[cacheKey] return ((this as any)[responseCache] ||= new GlobalResponse(this.#body, this.#init)) } @@ -89,7 +89,7 @@ if (!stateKey) { } export function getInternalBody( - response: Response | typeof GlobalResponse + response: Response | globalThis.Response ): InternalBody | undefined { if (!stateKey) { return From fd86f02ce00529927619f6fec208d4b20b9ea59a Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 May 2025 07:58:14 +0900 Subject: [PATCH 2/9] perf: keep using lightweight Response object when retrieving headers --- src/listener.ts | 5 ++++- src/response.ts | 19 ++++++++++++------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/src/listener.ts b/src/listener.ts index 8e92635..f0433c1 100644 --- a/src/listener.ts +++ b/src/listener.ts @@ -49,7 +49,10 @@ const responseViaCache = ( outgoing: ServerResponse | Http2ServerResponse ): undefined | Promise => { // eslint-disable-next-line @typescript-eslint/no-explicit-any - const [status, body, header] = (res as any)[cacheKey] + let [status, body, header] = (res as any)[cacheKey] + if (header instanceof Headers) { + header = buildOutgoingHttpHeaders(header) + } if (typeof body === 'string') { header['Content-Length'] = Buffer.byteLength(body) outgoing.writeHead(status, header) diff --git a/src/response.ts b/src/response.ts index 8218913..8be09dc 100644 --- a/src/response.ts +++ b/src/response.ts @@ -2,7 +2,6 @@ // Define lightweight pseudo Response class and replace global.Response with it. import type { OutgoingHttpHeaders } from 'node:http' -import { buildOutgoingHttpHeaders } from './utils' interface InternalBody { source: string | Uint8Array | FormData | Blob | null @@ -41,22 +40,28 @@ export class Response { } if (typeof body === 'string' || typeof (body as ReadableStream)?.getReader !== 'undefined') { - let headers = (init?.headers || { 'content-type': 'text/plain; charset=UTF-8' }) as + const headers = (init?.headers || { 'content-type': 'text/plain; charset=UTF-8' }) as | Record | Headers | OutgoingHttpHeaders - if (headers instanceof Headers) { - headers = buildOutgoingHttpHeaders(headers) - } - ;(this as any)[cacheKey] = [init?.status || 200, body, headers] } } + + get headers(): Headers { + const cache = (this as any)[cacheKey] + if (cache) { + if (!(cache[2] instanceof Headers)) { + cache[2] = new Headers(cache[2] as HeadersInit) + } + return cache[2] + } + return this[getResponseCache]().headers + } } ;[ 'body', 'bodyUsed', - 'headers', 'ok', 'redirected', 'status', From 5ad89afd156c315f2ae9d8d86ea5b9243b84262b Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 May 2025 08:12:40 +0900 Subject: [PATCH 3/9] refactor: organize internal cache data type. --- src/listener.ts | 6 +++--- src/response.ts | 16 +++++++++++++--- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/src/listener.ts b/src/listener.ts index f0433c1..7f6e4c4 100644 --- a/src/listener.ts +++ b/src/listener.ts @@ -7,6 +7,7 @@ import { toRequestError, } from './request' import { cacheKey, getInternalBody, Response as LightweightResponse } from './response' +import type { InternalCache } from './response' import type { CustomErrorHandler, FetchCallback, HttpBindings } from './types' import { writeFromReadableStream, buildOutgoingHttpHeaders } from './utils' import { X_ALREADY_SENT } from './utils/response/constants' @@ -49,7 +50,7 @@ const responseViaCache = ( outgoing: ServerResponse | Http2ServerResponse ): undefined | Promise => { // eslint-disable-next-line @typescript-eslint/no-explicit-any - let [status, body, header] = (res as any)[cacheKey] + let [status, body, header] = (res as any)[cacheKey] as InternalCache if (header instanceof Headers) { header = buildOutgoingHttpHeaders(header) } @@ -92,8 +93,7 @@ const responseViaResponseObject = async ( const resHeaderRecord: OutgoingHttpHeaders = buildOutgoingHttpHeaders(res.headers) - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const internalBody = getInternalBody(res as any) + const internalBody = getInternalBody(res as Response) if (internalBody) { const { length, source, stream } = internalBody if (source instanceof Uint8Array && source.byteLength !== length) { diff --git a/src/response.ts b/src/response.ts index 8be09dc..ec889e4 100644 --- a/src/response.ts +++ b/src/response.ts @@ -13,14 +13,24 @@ const responseCache = Symbol('responseCache') const getResponseCache = Symbol('getResponseCache') export const cacheKey = Symbol('cache') +export type InternalCache = [ + number, + string | ReadableStream, + Record | Headers | OutgoingHttpHeaders, +] +interface LiteResponse { + [responseCache]?: globalThis.Response + [cacheKey]?: InternalCache +} + export const GlobalResponse = global.Response export class Response { #body?: BodyInit | null #init?: ResponseInit; [getResponseCache](): globalThis.Response { - delete (this as any)[cacheKey] - return ((this as any)[responseCache] ||= new GlobalResponse(this.#body, this.#init)) + delete (this as LiteResponse)[cacheKey] + return ((this as LiteResponse)[responseCache] ||= new GlobalResponse(this.#body, this.#init)) } constructor(body?: BodyInit | null, init?: ResponseInit) { @@ -49,7 +59,7 @@ export class Response { } get headers(): Headers { - const cache = (this as any)[cacheKey] + const cache = (this as LiteResponse)[cacheKey] as InternalCache if (cache) { if (!(cache[2] instanceof Headers)) { cache[2] = new Headers(cache[2] as HeadersInit) From 1187be3313460870551e5db86accc25d56a973fb Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 May 2025 08:28:57 +0900 Subject: [PATCH 4/9] perf: status and ok should also be completed in cache only, if possible, to improve performance --- src/response.ts | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/src/response.ts b/src/response.ts index ec889e4..5a697e3 100644 --- a/src/response.ts +++ b/src/response.ts @@ -68,18 +68,20 @@ export class Response { } return this[getResponseCache]().headers } + + get status() { + return ( + ((this as LiteResponse)[cacheKey] as InternalCache | undefined)?.[0] ?? + this[getResponseCache]().status + ) + } + + get ok() { + const status = this.status + return status >= 200 && status < 300 + } } -;[ - 'body', - 'bodyUsed', - 'ok', - 'redirected', - 'status', - 'statusText', - 'trailers', - 'type', - 'url', -].forEach((k) => { +;['body', 'bodyUsed', 'redirected', 'statusText', 'trailers', 'type', 'url'].forEach((k) => { Object.defineProperty(Response.prototype, k, { get() { return this[getResponseCache]()[k] From 860de9f0040af819e698f5825ec24147f01efce9 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 May 2025 10:20:45 +0900 Subject: [PATCH 5/9] fix: clone headers to avoid sharing the same object between parent and child --- src/response.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/response.ts b/src/response.ts index 5a697e3..9da966d 100644 --- a/src/response.ts +++ b/src/response.ts @@ -34,6 +34,7 @@ export class Response { } constructor(body?: BodyInit | null, init?: ResponseInit) { + let headers: HeadersInit this.#body = body if (init instanceof Response) { const cachedGlobalResponse = (init as any)[responseCache] @@ -44,16 +45,15 @@ export class Response { return } else { this.#init = init.#init + // clone headers to avoid sharing the same object between parent and child + headers = new Headers((init.#init as ResponseInit).headers) } } else { this.#init = init } if (typeof body === 'string' || typeof (body as ReadableStream)?.getReader !== 'undefined') { - const headers = (init?.headers || { 'content-type': 'text/plain; charset=UTF-8' }) as - | Record - | Headers - | OutgoingHttpHeaders + headers ||= init?.headers || { 'content-type': 'text/plain; charset=UTF-8' } ;(this as any)[cacheKey] = [init?.status || 200, body, headers] } } From f657caec74e1f078346f2cf259098898412820c0 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 May 2025 10:40:41 +0900 Subject: [PATCH 6/9] feat: remove `getInternalBody` function --- src/listener.ts | 24 +----------------------- src/response.ts | 29 ----------------------------- test/server.test.ts | 6 +++--- 3 files changed, 4 insertions(+), 55 deletions(-) diff --git a/src/listener.ts b/src/listener.ts index 7f6e4c4..89ba8ff 100644 --- a/src/listener.ts +++ b/src/listener.ts @@ -6,7 +6,7 @@ import { Request as LightweightRequest, toRequestError, } from './request' -import { cacheKey, getInternalBody, Response as LightweightResponse } from './response' +import { cacheKey, Response as LightweightResponse } from './response' import type { InternalCache } from './response' import type { CustomErrorHandler, FetchCallback, HttpBindings } from './types' import { writeFromReadableStream, buildOutgoingHttpHeaders } from './utils' @@ -93,28 +93,6 @@ const responseViaResponseObject = async ( const resHeaderRecord: OutgoingHttpHeaders = buildOutgoingHttpHeaders(res.headers) - const internalBody = getInternalBody(res as Response) - if (internalBody) { - const { length, source, stream } = internalBody - if (source instanceof Uint8Array && source.byteLength !== length) { - // maybe `source` is detached, so we should send via res.body - } else { - // send via internal raw data - if (length) { - resHeaderRecord['content-length'] = length - } - outgoing.writeHead(res.status, resHeaderRecord) - if (typeof source === 'string' || source instanceof Uint8Array) { - outgoing.end(source) - } else if (source instanceof Blob) { - outgoing.end(new Uint8Array(await source.arrayBuffer())) - } else { - await writeFromReadableStream(stream, outgoing) - } - return - } - } - if (res.body) { /** * If content-encoding is set, we assume that the response should be not decoded. diff --git a/src/response.ts b/src/response.ts index 9da966d..b3437fa 100644 --- a/src/response.ts +++ b/src/response.ts @@ -3,12 +3,6 @@ import type { OutgoingHttpHeaders } from 'node:http' -interface InternalBody { - source: string | Uint8Array | FormData | Blob | null - stream: ReadableStream - length: number | null -} - const responseCache = Symbol('responseCache') const getResponseCache = Symbol('getResponseCache') export const cacheKey = Symbol('cache') @@ -97,26 +91,3 @@ export class Response { }) Object.setPrototypeOf(Response, GlobalResponse) Object.setPrototypeOf(Response.prototype, GlobalResponse.prototype) - -const stateKey = Reflect.ownKeys(new GlobalResponse()).find( - (k) => typeof k === 'symbol' && k.toString() === 'Symbol(state)' -) as symbol | undefined -if (!stateKey) { - console.warn('Failed to find Response internal state key') -} - -export function getInternalBody( - response: Response | globalThis.Response -): InternalBody | undefined { - if (!stateKey) { - return - } - - if (response instanceof Response) { - response = (response as any)[getResponseCache]() - } - - const state = (response as any)[stateKey] as { body?: InternalBody } | undefined - - return (state && state.body) || undefined -} diff --git a/test/server.test.ts b/test/server.test.ts index 2cb36b3..b79f2a0 100644 --- a/test/server.test.ts +++ b/test/server.test.ts @@ -107,7 +107,7 @@ describe('Basic', () => { }) }) -describe('via internal body', () => { +describe('via response body', () => { const app = new Hono() app.use('*', async (c, next) => { await next() @@ -173,7 +173,7 @@ describe('via internal body', () => { const res = await request(server).get('/uint8array') expect(res.status).toBe(200) expect(res.headers['content-type']).toMatch('application/octet-stream') - expect(res.headers['content-length']).toMatch('3') + expect(res.headers['content-length']).toBeUndefined() expect(res.body).toEqual(Buffer.from([1, 2, 3])) }) @@ -181,7 +181,7 @@ describe('via internal body', () => { const res = await request(server).get('/blob') expect(res.status).toBe(200) expect(res.headers['content-type']).toMatch('application/octet-stream') - expect(res.headers['content-length']).toMatch('3') + expect(res.headers['content-length']).toBeUndefined() expect(res.body).toEqual(Buffer.from([1, 2, 3])) }) From d72de8d50df24a1b9c3a72543ef5029f326b453c Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 May 2025 17:53:55 +0900 Subject: [PATCH 7/9] feat: support more response body types for caching --- src/listener.ts | 17 +++++++++++++---- src/response.ts | 7 ++++++- test/server.test.ts | 6 +++--- 3 files changed, 22 insertions(+), 8 deletions(-) diff --git a/src/listener.ts b/src/listener.ts index 89ba8ff..504c528 100644 --- a/src/listener.ts +++ b/src/listener.ts @@ -45,21 +45,30 @@ const handleResponseError = (e: unknown, outgoing: ServerResponse | Http2ServerR } } -const responseViaCache = ( +const responseViaCache = async ( res: Response, outgoing: ServerResponse | Http2ServerResponse -): undefined | Promise => { +): Promise => { // eslint-disable-next-line @typescript-eslint/no-explicit-any let [status, body, header] = (res as any)[cacheKey] as InternalCache if (header instanceof Headers) { header = buildOutgoingHttpHeaders(header) } + if (typeof body === 'string') { header['Content-Length'] = Buffer.byteLength(body) - outgoing.writeHead(status, header) + } else if (body instanceof Uint8Array) { + header['Content-Length'] = body.byteLength + } else if (body instanceof Blob) { + header['Content-Length'] = body.size + } + + outgoing.writeHead(status, header) + if (typeof body === 'string' || body instanceof Uint8Array) { outgoing.end(body) + } else if (body instanceof Blob) { + outgoing.end(new Uint8Array(await body.arrayBuffer())) } else { - outgoing.writeHead(status, header) return writeFromReadableStream(body, outgoing)?.catch( (e) => handleResponseError(e, outgoing) as undefined ) diff --git a/src/response.ts b/src/response.ts index b3437fa..be9d0ef 100644 --- a/src/response.ts +++ b/src/response.ts @@ -46,7 +46,12 @@ export class Response { this.#init = init } - if (typeof body === 'string' || typeof (body as ReadableStream)?.getReader !== 'undefined') { + if ( + typeof body === 'string' || + typeof (body as ReadableStream)?.getReader !== 'undefined' || + body instanceof Blob || + body instanceof Uint8Array + ) { headers ||= init?.headers || { 'content-type': 'text/plain; charset=UTF-8' } ;(this as any)[cacheKey] = [init?.status || 200, body, headers] } diff --git a/test/server.test.ts b/test/server.test.ts index b79f2a0..745efc9 100644 --- a/test/server.test.ts +++ b/test/server.test.ts @@ -107,7 +107,7 @@ describe('Basic', () => { }) }) -describe('via response body', () => { +describe('various response body types', () => { const app = new Hono() app.use('*', async (c, next) => { await next() @@ -173,7 +173,7 @@ describe('via response body', () => { const res = await request(server).get('/uint8array') expect(res.status).toBe(200) expect(res.headers['content-type']).toMatch('application/octet-stream') - expect(res.headers['content-length']).toBeUndefined() + expect(res.headers['content-length']).toMatch('3') expect(res.body).toEqual(Buffer.from([1, 2, 3])) }) @@ -181,7 +181,7 @@ describe('via response body', () => { const res = await request(server).get('/blob') expect(res.status).toBe(200) expect(res.headers['content-type']).toMatch('application/octet-stream') - expect(res.headers['content-length']).toBeUndefined() + expect(res.headers['content-length']).toMatch('3') expect(res.body).toEqual(Buffer.from([1, 2, 3])) }) From 73a8a1a9c1247eff2c63172499fa801e81ca3ece Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 May 2025 18:22:26 +0900 Subject: [PATCH 8/9] test: add test for fallback to GlobalResponse object --- test/response.test.ts | 21 ++++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/test/response.test.ts b/test/response.test.ts index 1716527..e999e2f 100644 --- a/test/response.test.ts +++ b/test/response.test.ts @@ -1,7 +1,7 @@ import { createServer } from 'node:http' import type { Server } from 'node:http' import type { AddressInfo } from 'node:net' -import { GlobalResponse, Response as LightweightResponse } from '../src/response' +import { GlobalResponse, Response as LightweightResponse, cacheKey } from '../src/response' Object.defineProperty(global, 'Response', { value: LightweightResponse, @@ -99,4 +99,23 @@ describe('Response', () => { ) expect(await childResponse.text()).toEqual('HONO') }) + + describe('Fallback to GlobalResponse object', () => { + it('Should return value from internal cache', () => { + const res = new Response('Hello! Node!') + res.headers.set('x-test', 'test') + expect(res.headers.get('x-test')).toEqual('test') + expect(res.status).toEqual(200) + expect(res.ok).toEqual(true) + expect(cacheKey in res).toBe(true) + }) + + it('Should return value from generated GlobalResponse object', () => { + const res = new Response('Hello! Node!', { + statusText: 'OK', + }) + expect(res.statusText).toEqual('OK') + expect(cacheKey in res).toBe(false) + }) + }) }) From 33c06dd327ca73fc0a7eeb511e5fb53edc4e2f5b Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Mon, 19 May 2025 18:23:57 +0900 Subject: [PATCH 9/9] refactor: change to an appropriate name from LiteResponse to LightResponse --- src/response.ts | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/response.ts b/src/response.ts index be9d0ef..914f05f 100644 --- a/src/response.ts +++ b/src/response.ts @@ -12,7 +12,7 @@ export type InternalCache = [ string | ReadableStream, Record | Headers | OutgoingHttpHeaders, ] -interface LiteResponse { +interface LightResponse { [responseCache]?: globalThis.Response [cacheKey]?: InternalCache } @@ -23,8 +23,8 @@ export class Response { #init?: ResponseInit; [getResponseCache](): globalThis.Response { - delete (this as LiteResponse)[cacheKey] - return ((this as LiteResponse)[responseCache] ||= new GlobalResponse(this.#body, this.#init)) + delete (this as LightResponse)[cacheKey] + return ((this as LightResponse)[responseCache] ||= new GlobalResponse(this.#body, this.#init)) } constructor(body?: BodyInit | null, init?: ResponseInit) { @@ -58,7 +58,7 @@ export class Response { } get headers(): Headers { - const cache = (this as LiteResponse)[cacheKey] as InternalCache + const cache = (this as LightResponse)[cacheKey] as InternalCache if (cache) { if (!(cache[2] instanceof Headers)) { cache[2] = new Headers(cache[2] as HeadersInit) @@ -70,7 +70,7 @@ export class Response { get status() { return ( - ((this as LiteResponse)[cacheKey] as InternalCache | undefined)?.[0] ?? + ((this as LightResponse)[cacheKey] as InternalCache | undefined)?.[0] ?? this[getResponseCache]().status ) }