From 990fe5c81043ff7653dee58be42c62bebac63a9c Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 Feb 2024 06:08:11 +0900 Subject: [PATCH 1/4] feat: define getInternalBody function to get the internal body of a response This change cherry-picks some of the following commits b5ba8dd7b9260be1c8e95ea860cc0303f1b69924 Thanks to @tangye1234 for the original implementation. --- src/response.ts | 41 ++++++++++++++++++++++++++++++++++++----- 1 file changed, 36 insertions(+), 5 deletions(-) diff --git a/src/response.ts b/src/response.ts index 2c68b8e..a031464 100644 --- a/src/response.ts +++ b/src/response.ts @@ -4,15 +4,22 @@ import type { OutgoingHttpHeaders } from 'node:http' import { buildOutgoingHttpHeaders } from './utils' +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') export const GlobalResponse = global.Response export class Response { #body?: BodyInit | null - #init?: ResponseInit + #init?: ResponseInit; - private get cache(): typeof GlobalResponse { + [getResponseCache](): typeof GlobalResponse { delete (this as any)[cacheKey] return ((this as any)[responseCache] ||= new GlobalResponse(this.#body, this.#init)) } @@ -24,7 +31,7 @@ export class Response { if (cachedGlobalResponse) { this.#init = cachedGlobalResponse // instantiate GlobalResponse cache and this object always returns value from global.Response - this.cache + this[getResponseCache]() return } else { this.#init = init.#init @@ -60,14 +67,14 @@ export class Response { ].forEach((k) => { Object.defineProperty(Response.prototype, k, { get() { - return this.cache[k] + return this[getResponseCache]()[k] }, }) }) ;['arrayBuffer', 'blob', 'clone', 'formData', 'json', 'text'].forEach((k) => { Object.defineProperty(Response.prototype, k, { value: function () { - return this.cache[k]() + return this[getResponseCache]()[k]() }, }) }) @@ -76,3 +83,27 @@ Object.setPrototypeOf(Response.prototype, GlobalResponse.prototype) Object.defineProperty(global, 'Response', { value: Response, }) + +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 | typeof GlobalResponse +): InternalBody | undefined { + if (!stateKey) { + return + } + + if (response instanceof Response) { + response = (response as any)[getResponseCache]() + } + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const state = (response as any)[stateKey] as { body?: InternalBody } | undefined + + return (state && state.body) || undefined +} From 9eefaca3f4e7ea541f0cb43a8020374bafeaef1d Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 Feb 2024 09:14:32 +0900 Subject: [PATCH 2/4] feat: use internal body if available for returning the response in its original form as much as possible --- src/listener.ts | 22 ++++++++++-- test/server.test.ts | 82 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 2 deletions(-) diff --git a/src/listener.ts b/src/listener.ts index cde6cea..386ca5b 100644 --- a/src/listener.ts +++ b/src/listener.ts @@ -1,7 +1,7 @@ import type { IncomingMessage, ServerResponse, OutgoingHttpHeaders } from 'node:http' import type { Http2ServerRequest, Http2ServerResponse } from 'node:http2' import { getAbortController, newRequest } from './request' -import { cacheKey } from './response' +import { cacheKey, getInternalBody } from './response' import type { CustomErrorHandler, FetchCallback, HttpBindings } from './types' import { writeFromReadableStream, buildOutgoingHttpHeaders } from './utils' import './globals' @@ -83,7 +83,25 @@ const responseViaResponseObject = async ( const resHeaderRecord: OutgoingHttpHeaders = buildOutgoingHttpHeaders(res.headers) - if (res.body) { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const internalBody = getInternalBody(res as any) + if (internalBody) { + try { + if (internalBody.length) { + resHeaderRecord['content-length'] = internalBody.length + } + outgoing.writeHead(res.status, resHeaderRecord) + if (typeof internalBody.source === 'string' || internalBody.source instanceof Uint8Array) { + outgoing.end(internalBody.source) + } else if (internalBody.source instanceof Blob) { + outgoing.end(new Uint8Array(await internalBody.source.arrayBuffer())) + } else { + await writeFromReadableStream(internalBody.stream, outgoing) + } + } catch (e: unknown) { + handleResponseError(e, outgoing) + } + } else if (res.body) { try { /** * If content-encoding is set, we assume that the response should be not decoded. diff --git a/test/server.test.ts b/test/server.test.ts index db48372..b3291d2 100644 --- a/test/server.test.ts +++ b/test/server.test.ts @@ -83,6 +83,88 @@ describe('Basic', () => { }) }) +describe('via internal body', () => { + const app = new Hono() + app.use('*', async (c, next) => { + await next() + + // generate internal response object + const status = c.res.status + if (status > 999) { + c.res = new Response('Internal Server Error', { status: 500 }) + } + }) + app.get('/', () => { + const response = new Response('Hello! Node!') + return response + }) + app.get('/uint8array', () => { + const response = new Response(new Uint8Array([1, 2, 3]), { + headers: { 'content-type': 'application/octet-stream' }, + }) + return response + }) + app.get('/blob', () => { + const response = new Response(new Blob([new Uint8Array([1, 2, 3])]), { + headers: { 'content-type': 'application/octet-stream' }, + }) + return response + }) + app.get('/readable-stream', () => { + const stream = new ReadableStream({ + async start(controller) { + controller.enqueue('Hello!') + controller.enqueue(' Node!') + controller.close() + }, + }) + return new Response(stream) + }) + + const server = createAdaptorServer(app) + + it('Should return 200 response - GET /', async () => { + const res = await request(server).get('/') + expect(res.status).toBe(200) + expect(res.headers['content-type']).toMatch('text/plain') + expect(res.headers['content-length']).toMatch('12') + expect(res.text).toBe('Hello! Node!') + }) + + it('Should return 200 response - GET /uint8array', async () => { + 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.body).toEqual(Buffer.from([1, 2, 3])) + }) + + it('Should return 200 response - GET /blob', async () => { + 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.body).toEqual(Buffer.from([1, 2, 3])) + }) + + it('Should return 200 response - GET /readable-stream', async () => { + const expectedChunks = ['Hello!', ' Node!'] + const res = await request(server) + .get('/readable-stream') + .parse((res, fn) => { + res.on('data', (chunk) => { + const str = chunk.toString() + expect(str).toBe(expectedChunks.shift()) + }) + res.on('end', () => fn(null, '')) + }) + expect(res.status).toBe(200) + expect(res.headers['content-type']).toMatch('text/plain; charset=UTF-8') + expect(res.headers['content-length']).toBeUndefined() + expect(expectedChunks.length).toBe(0) // all chunks are received + }) +}) + describe('Routing', () => { describe('Nested Route', () => { const book = new Hono() From 3c64fbaae5f0e171574c30be1315307c09d7bb6a Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 Feb 2024 09:44:53 +0900 Subject: [PATCH 3/4] refactor: simplify try section for handleResponseError() --- src/listener.ts | 110 ++++++++++++++++++++++-------------------------- 1 file changed, 51 insertions(+), 59 deletions(-) diff --git a/src/listener.ts b/src/listener.ts index 386ca5b..840e3c8 100644 --- a/src/listener.ts +++ b/src/listener.ts @@ -72,13 +72,9 @@ const responseViaResponseObject = async ( } } - try { - const isCached = cacheKey in res - if (isCached) { - return responseViaCache(res as Response, outgoing) - } - } catch (e: unknown) { - return handleResponseError(e, outgoing) + const isCached = cacheKey in res + if (isCached) { + return responseViaCache(res as Response, outgoing) } const resHeaderRecord: OutgoingHttpHeaders = buildOutgoingHttpHeaders(res.headers) @@ -86,60 +82,52 @@ const responseViaResponseObject = async ( // eslint-disable-next-line @typescript-eslint/no-explicit-any const internalBody = getInternalBody(res as any) if (internalBody) { - try { - if (internalBody.length) { - resHeaderRecord['content-length'] = internalBody.length - } - outgoing.writeHead(res.status, resHeaderRecord) - if (typeof internalBody.source === 'string' || internalBody.source instanceof Uint8Array) { - outgoing.end(internalBody.source) - } else if (internalBody.source instanceof Blob) { - outgoing.end(new Uint8Array(await internalBody.source.arrayBuffer())) - } else { - await writeFromReadableStream(internalBody.stream, outgoing) - } - } catch (e: unknown) { - handleResponseError(e, outgoing) + if (internalBody.length) { + resHeaderRecord['content-length'] = internalBody.length + } + outgoing.writeHead(res.status, resHeaderRecord) + if (typeof internalBody.source === 'string' || internalBody.source instanceof Uint8Array) { + outgoing.end(internalBody.source) + } else if (internalBody.source instanceof Blob) { + outgoing.end(new Uint8Array(await internalBody.source.arrayBuffer())) + } else { + await writeFromReadableStream(internalBody.stream, outgoing) } } else if (res.body) { - try { - /** - * If content-encoding is set, we assume that the response should be not decoded. - * Else if transfer-encoding is set, we assume that the response should be streamed. - * Else if content-length is set, we assume that the response content has been taken care of. - * Else if x-accel-buffering is set to no, we assume that the response should be streamed. - * Else if content-type is not application/json nor text/* but can be text/event-stream, - * we assume that the response should be streamed. - */ - - const { - 'transfer-encoding': transferEncoding, - 'content-encoding': contentEncoding, - 'content-length': contentLength, - 'x-accel-buffering': accelBuffering, - 'content-type': contentType, - } = resHeaderRecord - - if ( - transferEncoding || - contentEncoding || - contentLength || - // nginx buffering variant - (accelBuffering && regBuffer.test(accelBuffering as string)) || - !regContentType.test(contentType as string) - ) { - outgoing.writeHead(res.status, resHeaderRecord) - - await writeFromReadableStream(res.body, outgoing) - } else { - const buffer = await res.arrayBuffer() - resHeaderRecord['content-length'] = buffer.byteLength + /** + * If content-encoding is set, we assume that the response should be not decoded. + * Else if transfer-encoding is set, we assume that the response should be streamed. + * Else if content-length is set, we assume that the response content has been taken care of. + * Else if x-accel-buffering is set to no, we assume that the response should be streamed. + * Else if content-type is not application/json nor text/* but can be text/event-stream, + * we assume that the response should be streamed. + */ + + const { + 'transfer-encoding': transferEncoding, + 'content-encoding': contentEncoding, + 'content-length': contentLength, + 'x-accel-buffering': accelBuffering, + 'content-type': contentType, + } = resHeaderRecord + + if ( + transferEncoding || + contentEncoding || + contentLength || + // nginx buffering variant + (accelBuffering && regBuffer.test(accelBuffering as string)) || + !regContentType.test(contentType as string) + ) { + outgoing.writeHead(res.status, resHeaderRecord) - outgoing.writeHead(res.status, resHeaderRecord) - outgoing.end(new Uint8Array(buffer)) - } - } catch (e: unknown) { - handleResponseError(e, outgoing) + await writeFromReadableStream(res.body, outgoing) + } else { + const buffer = await res.arrayBuffer() + resHeaderRecord['content-length'] = buffer.byteLength + + outgoing.writeHead(res.status, resHeaderRecord) + outgoing.end(new Uint8Array(buffer)) } } else { outgoing.writeHead(res.status, resHeaderRecord) @@ -191,6 +179,10 @@ export const getRequestListener = ( } } - return responseViaResponseObject(res, outgoing, options) + try { + return responseViaResponseObject(res, outgoing, options) + } catch (e) { + return handleResponseError(e, outgoing) + } } } From a25fa2a2c558f575ef0bf35fcd9faf055c47d2e2 Mon Sep 17 00:00:00 2001 From: Taku Amano Date: Sat, 17 Feb 2024 09:48:16 +0900 Subject: [PATCH 4/4] refactor: remove redundant variable "isCached" --- src/listener.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/listener.ts b/src/listener.ts index 840e3c8..d8af24f 100644 --- a/src/listener.ts +++ b/src/listener.ts @@ -72,8 +72,7 @@ const responseViaResponseObject = async ( } } - const isCached = cacheKey in res - if (isCached) { + if (cacheKey in res) { return responseViaCache(res as Response, outgoing) }