Skip to content

refactor: improve handling of null body in response #341

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
usualoma wants to merge 5 commits into
base: v2
Choose a base branch
from
Open

refactor: improve handling of null body in response #341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6cf7fe6
refactor: accept default content type in buildOutgoingHttpHeaders
usualoma Apr 4, 2026
ccfe561
refactor: define `defaultContentType` constant in `response.ts`
usualoma Apr 4, 2026
5fe988d
test: add tests for default content-type
usualoma Apr 4, 2026
5cdc481
refactor: improve handling of null body in response
usualoma Apr 4, 2026
a2d2ab0
refactor: `bun run format:fix`
usualoma Apr 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions src/listener.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
wrapBodyStream,
toRequestError,
} from './request'
import { cacheKey, Response as LightweightResponse } from './response'
import { defaultContentType, cacheKey, Response as LightweightResponse } from './response'
import type { InternalCache } from './response'
import type { CustomErrorHandler, FetchCallback, HttpBindings } from './types'
import {
Expand Down Expand Up @@ -87,7 +87,7 @@

const makeCloseHandler =
(
req: any,

Check warning on line 90 in src/listener.ts

View workflow job for this annotation

GitHub Actions / ci (20.x) 

Unexpected any. Specify a different type

Check warning on line 90 in src/listener.ts

View workflow job for this annotation

GitHub Actions / ci (22.x) 

Unexpected any. Specify a different type

Check warning on line 90 in src/listener.ts

View workflow job for this annotation

GitHub Actions / ci (24.x) 

Unexpected any. Specify a different type
incoming: IncomingMessage | Http2ServerRequest,
outgoing: ServerResponse | Http2ServerResponse,
needsBodyCleanup: boolean
Expand Down Expand Up @@ -157,29 +157,29 @@
// Fast path: no custom headers — create the final header object in one shot
// (avoids shape transitions from mutating a single-key object).
if (!header) {
if (body == null) {
outgoing.writeHead(status, {})
if (body === null) {
outgoing.writeHead(status)
outgoing.end()
} else if (typeof body === 'string') {
outgoing.writeHead(status, {
'content-type': 'text/plain; charset=UTF-8',
'Content-Type': defaultContentType,
'Content-Length': Buffer.byteLength(body),
})
outgoing.end(body)
} else if (body instanceof Uint8Array) {
outgoing.writeHead(status, {
'content-type': 'text/plain; charset=UTF-8',
'Content-Type': defaultContentType,
'Content-Length': body.byteLength,
})
outgoing.end(body)
} else if (body instanceof Blob) {
outgoing.writeHead(status, {
'content-type': 'text/plain; charset=UTF-8',
'Content-Type': defaultContentType,
'Content-Length': body.size,
})
outgoing.end(new Uint8Array(await body.arrayBuffer()))
} else {
outgoing.writeHead(status, { 'content-type': 'text/plain; charset=UTF-8' })
outgoing.writeHead(status, { 'Content-Type': defaultContentType })
flushHeaders(outgoing)
await writeFromReadableStream(body, outgoing)?.catch(
(e) => handleResponseError(e, outgoing) as undefined
Expand All @@ -192,11 +192,11 @@
let hasContentLength = false
if (header instanceof Headers) {
hasContentLength = header.has('content-length')
header = buildOutgoingHttpHeaders(header)
header = buildOutgoingHttpHeaders(header, body === null ? undefined : defaultContentType)
} else if (Array.isArray(header)) {
const headerObj = new Headers(header)
hasContentLength = headerObj.has('content-length')
header = buildOutgoingHttpHeaders(headerObj)
header = buildOutgoingHttpHeaders(headerObj, body === null ? undefined : defaultContentType)
} else {
for (const key in header) {
if (key.length === 14 && key.toLowerCase() === 'content-length') {
Expand Down Expand Up @@ -262,7 +262,10 @@
return responseViaCache(res as Response, outgoing)
}

const resHeaderRecord: OutgoingHttpHeaders = buildOutgoingHttpHeaders(res.headers)
const resHeaderRecord: OutgoingHttpHeaders = buildOutgoingHttpHeaders(
res.headers,
res.body === null ? undefined : defaultContentType
)

if (res.body) {
const reader = res.body.getReader()
Expand Down
8 changes: 5 additions & 3 deletions src/response.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

import type { OutgoingHttpHeaders } from 'node:http'

export const defaultContentType = 'text/plain; charset=UTF-8'

const responseCache = Symbol('responseCache')
const getResponseCache = Symbol('getResponseCache')
export const cacheKey = Symbol('cache')
Expand Down Expand Up @@ -47,8 +49,7 @@ export class Response {
}

if (
body === null ||
body === undefined ||
body == null ||
typeof body === 'string' ||
typeof (body as ReadableStream)?.getReader !== 'undefined' ||
body instanceof Blob ||
Expand All @@ -63,7 +64,8 @@ export class Response {
if (cache) {
if (!(cache[2] instanceof Headers)) {
cache[2] = new Headers(
(cache[2] || { 'content-type': 'text/plain; charset=UTF-8' }) as HeadersInit
(cache[2] ||
(cache[1] === null ? undefined : { 'content-type': defaultContentType })) as HeadersInit
)
}
return cache[2]
Expand Down
8 changes: 6 additions & 2 deletions src/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,8 @@ export function writeFromReadableStream(stream: ReadableStream<Uint8Array>, writ
}

export const buildOutgoingHttpHeaders = (
headers: Headers | HeadersInit | null | undefined
headers: Headers | HeadersInit | null | undefined,
defaultContentType: string | undefined
): OutgoingHttpHeaders => {
const res: OutgoingHttpHeaders = {}
if (!(headers instanceof Headers)) {
Expand All @@ -86,7 +87,10 @@ export const buildOutgoingHttpHeaders = (
res[k] = v
}
}
res['content-type'] ??= 'text/plain; charset=UTF-8'

if (defaultContentType) {
res['content-type'] ??= defaultContentType
}

return res
}
14 changes: 14 additions & 0 deletions test/listener.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,20 @@ describe('Invalid request', () => {
})
})

describe('Response headers', () => {
const requestListener = getRequestListener(() => new Response(null), {
hostname: 'example.com',
})
const server = createServer(requestListener)

it('Should not set content-type for a null body response', async () => {
const res = await request(server).get('/').send()
expect(res.status).toBe(200)
expect(res.headers['content-type']).toBeUndefined()
expect(res.text).toBe('')
})
})

describe('Error handling - sync fetchCallback', () => {
const fetchCallback = vi.fn(() => {
throw new Error('thrown error')
Expand Down
43 changes: 29 additions & 14 deletions test/utils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,68 +11,83 @@ describe('buildOutgoingHttpHeaders', () => {
a: 'b',
'content-type': 'text/html; charset=UTF-8',
})
const result = buildOutgoingHttpHeaders(headers)
const result = buildOutgoingHttpHeaders(headers, 'text/plain; charset=UTF-8')
expect(result).toEqual({
a: 'b',
'content-type': 'text/html; charset=UTF-8',
})
})

it('multiple set-cookie', () => {
it('multiple set-cookie with default content-type', () => {
const headers = new Headers()
headers.append('set-cookie', 'a')
headers.append('set-cookie', 'b')
const result = buildOutgoingHttpHeaders(headers)
const result = buildOutgoingHttpHeaders(headers, 'text/plain; charset=UTF-8')
expect(result).toEqual({
'set-cookie': ['a', 'b'],
'content-type': 'text/plain; charset=UTF-8',
})
})

it('Headers', () => {
it('Headers with default content-type', () => {
const headers = new Headers({
a: 'b',
})
const result = buildOutgoingHttpHeaders(headers)
const result = buildOutgoingHttpHeaders(headers, 'text/plain; charset=UTF-8')
expect(result).toEqual({
a: 'b',
'content-type': 'text/plain; charset=UTF-8',
})
})

it('Record<string, string>', () => {
it('Record<string, string> with default content-type', () => {
const headers = {
a: 'b',
'Set-Cookie': 'c', // case-insensitive
}
const result = buildOutgoingHttpHeaders(headers)
const result = buildOutgoingHttpHeaders(headers, 'text/plain; charset=UTF-8')
expect(result).toEqual({
a: 'b',
'set-cookie': ['c'],
'content-type': 'text/plain; charset=UTF-8',
})
})

it('Record<string, string>[]', () => {
it('Record<string, string>[] with default content-type', () => {
const headers: HeadersInit = [['a', 'b']]
const result = buildOutgoingHttpHeaders(headers)
const result = buildOutgoingHttpHeaders(headers, 'text/plain; charset=UTF-8')
expect(result).toEqual({
a: 'b',
'content-type': 'text/plain; charset=UTF-8',
})
})

it('null', () => {
const result = buildOutgoingHttpHeaders(null)
it('null without default content-type', () => {
const result = buildOutgoingHttpHeaders(null, undefined)
expect(result).toEqual({})
})

it('undefined without default content-type', () => {
const result = buildOutgoingHttpHeaders(undefined, undefined)
expect(result).toEqual({})
})

it('null with default content-type', () => {
const result = buildOutgoingHttpHeaders(null, 'text/plain; charset=UTF-8')
expect(result).toEqual({
'content-type': 'text/plain; charset=UTF-8',
})
})

it('undefined', () => {
const result = buildOutgoingHttpHeaders(undefined)
it('does not override existing content-type when default is provided', () => {
const result = buildOutgoingHttpHeaders(
{
'content-type': 'application/json',
},
'text/plain; charset=UTF-8'
)
expect(result).toEqual({
'content-type': 'text/plain; charset=UTF-8',
'content-type': 'application/json',
})
})
})
Expand Down
Loading