Skip to content

Commit

Permalink
feat: use internal body if available for returning the response in it…
Browse files Browse the repository at this point in the history
…s original form as much as possible (#145)

* feat: define getInternalBody function to get the internal body of a response

This change cherry-picks some of the following commits
b5ba8dd

Thanks to @tangye1234 for the original implementation.

* feat: use internal body if available for returning the response in its original form as much as possible

* refactor: simplify try section for handleResponseError()

* refactor: remove redundant variable "isCached"
  • Loading branch information
usualoma authored Feb 17, 2024
1 parent 4c5b0fb commit 12f75e0
Show file tree
Hide file tree
Showing 3 changed files with 174 additions and 52 deletions.
103 changes: 56 additions & 47 deletions src/listener.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -72,56 +72,61 @@ const responseViaResponseObject = async (
}
}

try {
const isCached = cacheKey in res
if (isCached) {
return responseViaCache(res as Response, outgoing)
}
} catch (e: unknown) {
return handleResponseError(e, outgoing)
if (cacheKey in res) {
return responseViaCache(res as Response, outgoing)
}

const resHeaderRecord: OutgoingHttpHeaders = buildOutgoingHttpHeaders(res.headers)

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
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const internalBody = getInternalBody(res as any)
if (internalBody) {
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) {
/**
* 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

outgoing.writeHead(res.status, resHeaderRecord)
outgoing.end(new Uint8Array(buffer))
}
} catch (e: unknown) {
handleResponseError(e, outgoing)
outgoing.writeHead(res.status, resHeaderRecord)
outgoing.end(new Uint8Array(buffer))
}
} else {
outgoing.writeHead(res.status, resHeaderRecord)
Expand Down Expand Up @@ -173,6 +178,10 @@ export const getRequestListener = (
}
}

return responseViaResponseObject(res, outgoing, options)
try {
return responseViaResponseObject(res, outgoing, options)
} catch (e) {
return handleResponseError(e, outgoing)
}
}
}
41 changes: 36 additions & 5 deletions src/response.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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
Expand Down Expand Up @@ -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]()
},
})
})
Expand All @@ -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
}
82 changes: 82 additions & 0 deletions test/server.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down

0 comments on commit 12f75e0

Please sign in to comment.