Skip to content
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.

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

fix(ClientRequest): ignore message body for responses without body #436

Merged
merged 3 commits into from
Sep 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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: 23 additions & 0 deletions src/interceptors/ClientRequest/utils/createResponse.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { it, expect } from 'vitest'
import { Socket } from 'net'
import * as http from 'http'
import { createResponse } from './createResponse'
import { responseStatusCodesWithoutBody } from '../../../utils/responseUtils'

it('creates a fetch api response from http incoming message', async () => {
const message = new http.IncomingMessage(new Socket())
Expand All @@ -20,3 +21,25 @@ it('creates a fetch api response from http incoming message', async () => {
expect(response.headers.get('content-type')).toBe('application/json')
expect(await response.json()).toEqual({ firstName: 'John' })
})

it.each(responseStatusCodesWithoutBody)(
'ignores message body for %i response status',
(responseStatus) => {
const message = new http.IncomingMessage(new Socket())
message.statusCode = responseStatus

const response = createResponse(message)

// These chunks will be ignored: this response
// cannot have body. We don't forward this error to
// the consumer because it's us who converts the
// internal stream to a Fetch API Response instance.
// Consumers will rely on the Response API when constructing
// mocked responses.
message.emit('data', Buffer.from('hello'))
message.emit('end')

expect(response.status).toBe(responseStatus)
expect(response.body).toBe(null)
}
)
31 changes: 18 additions & 13 deletions src/interceptors/ClientRequest/utils/createResponse.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,29 @@
import type { IncomingHttpHeaders, IncomingMessage } from 'http'
import { responseStatusCodesWithoutBody } from '../../../utils/responseUtils'

/**
* Creates a Fetch API `Response` instance from the given
* `http.IncomingMessage` instance.
*/
export function createResponse(message: IncomingMessage): Response {
const readable = new ReadableStream({
start(controller) {
message.on('data', (chunk) => controller.enqueue(chunk))
message.on('end', () => controller.close())

/**
* @todo Should also listen to the "error" on the message
* and forward it to the controller. Otherwise the stream
* will pend indefinitely.
*/
},
})
const responseBodyOrNull = responseStatusCodesWithoutBody.includes(
message.statusCode || 200
)
? null
: new ReadableStream({
start(controller) {
message.on('data', (chunk) => controller.enqueue(chunk))
message.on('end', () => controller.close())

/**
* @todo Should also listen to the "error" on the message
* and forward it to the controller. Otherwise the stream
* will pend indefinitely.
*/
},
})

return new Response(readable, {
return new Response(responseBodyOrNull, {
status: message.statusCode,
statusText: message.statusMessage,
headers: createHeadersFromIncomingHttpHeaders(message.headers),
Expand Down
6 changes: 4 additions & 2 deletions src/interceptors/XMLHttpRequest/utils/createResponse.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const statusCodesWithoutBody = [204, 205, 304]
import { responseStatusCodesWithoutBody } from '../../../utils/responseUtils'

/**
* Creates a Fetch API `Response` instance from the given
Expand All @@ -16,7 +16,9 @@ export function createResponse(
* when constructing a Response instance.
* @see https://github.com/mswjs/interceptors/issues/379
*/
const responseBodyOrNull = statusCodesWithoutBody.includes(request.status)
const responseBodyOrNull = responseStatusCodesWithoutBody.includes(
request.status
)
? null
: body

Expand Down
5 changes: 5 additions & 0 deletions src/utils/responseUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
/**
* Response status codes for responses that cannot have body.
* @see https://fetch.spec.whatwg.org/#statuses
*/
export const responseStatusCodesWithoutBody = [204, 205, 304]
2 changes: 1 addition & 1 deletion test/features/events/response.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ it('XMLHttpRequest: emits the "response" event upon the original response', asyn
* passthrough to the ClientRequest, it will perform an "OPTIONS" request first,
* thus two request/response events emitted.
*/
expect(responseListener).toHaveBeenCalledTimes(1)
expect(responseListener).toHaveBeenCalledTimes(2)

// Lookup the correct response listener call.
const [{ response, request, isMockedResponse }] =
Expand Down