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(MockHttpSocket): handle response stream errors #548

Merged
Merged
Show file tree
Hide file tree
Changes from 19 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
20 changes: 16 additions & 4 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "@mswjs/interceptors",
"description": "Low-level HTTP/HTTPS/XHR/fetch request interception library.",
"version": "0.28.0",
"version": "0.28.2",
"main": "./lib/node/index.js",
"module": "./lib/node/index.mjs",
"types": "./lib/node/index.d.ts",
Expand All @@ -19,8 +19,12 @@
"default": "./lib/node/index.js"
},
"./ClientRequest": {
"browser": null,
"types": "./lib/node/interceptors/ClientRequest/index.d.ts",
"node": {
"require": "./lib/node/interceptors/ClientRequest/index.js",
"import": "./lib/node/interceptors/ClientRequest/index.mjs"
},
"browser": null,
"require": "./lib/node/interceptors/ClientRequest/index.js",
"import": "./lib/node/interceptors/ClientRequest/index.mjs",
"default": "./lib/node/interceptors/ClientRequest/index.js"
Expand Down Expand Up @@ -56,15 +60,23 @@
"default": "./lib/browser/interceptors/WebSocket/index.js"
},
"./RemoteHttpInterceptor": {
"browser": null,
"types": "./lib/node/RemoteHttpInterceptor.d.ts",
"node": {
"require": "./lib/node/RemoteHttpInterceptor.js",
"import": "./lib/node/RemoteHttpInterceptor.mjs"
},
"browser": null,
"require": "./lib/node/RemoteHttpInterceptor.js",
"import": "./lib/node/RemoteHttpInterceptor.mjs",
"default": "./lib/node/RemoteHttpInterceptor.js"
},
"./presets/node": {
"browser": null,
"types": "./lib/node/presets/node.d.ts",
"node": {
"require": "./lib/node/presets/node.js",
"import": "./lib/node/presets/node.mjs"
},
"browser": null,
"require": "./lib/node/presets/node.js",
"import": "./lib/node/presets/node.mjs",
"default": "./lib/node/presets/node.js"
Expand Down
52 changes: 31 additions & 21 deletions src/interceptors/ClientRequest/MockHttpSocket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,10 @@ import { isPropertyAccessible } from '../../utils/isPropertyAccessible'
import { baseUrlFromConnectionOptions } from '../Socket/utils/baseUrlFromConnectionOptions'
import { parseRawHeaders } from '../Socket/utils/parseRawHeaders'
import { getRawFetchHeaders } from '../../utils/getRawFetchHeaders'
import { RESPONSE_STATUS_CODES_WITHOUT_BODY } from '../../utils/responseUtils'
import {
createServerErrorResponse,
RESPONSE_STATUS_CODES_WITHOUT_BODY,
} from '../../utils/responseUtils'
import { createRequestId } from '../../createRequestId'

type HttpConnectionOptions = any
Expand Down Expand Up @@ -248,34 +251,41 @@ export class MockHttpSocket extends MockSocket {
}

if (response.body) {
const reader = response.body.getReader()

while (true) {
const { done, value } = await reader.read()

if (done) {
break
}

// Flush the headers upon the first chunk in the stream.
// This ensures the consumer will start receiving the response
// as it streams in (subsequent chunks are pushed).
if (httpHeaders.length > 0) {
flushHeaders(value)
continue
try {
const reader = response.body.getReader()

while (true) {
const { done, value } = await reader.read()

if (done) {
break
}

// Flush the headers upon the first chunk in the stream.
// This ensures the consumer will start receiving the response
// as it streams in (subsequent chunks are pushed).
if (httpHeaders.length > 0) {
flushHeaders(value)
continue
}

// Subsequent body chukns are push to the stream.
this.push(value)
}
} catch (error) {
// Coerce response stream errors to 500 responses.
// Don't flush the original response headers because
// unhandled errors translate to 500 error responses forcefully.
this.respondWith(createServerErrorResponse(error))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Treating all the response stream errors as 500 error responses! Consistency.


// Subsequent body chukns are push to the stream.
this.push(value)
return
}
}

// If the headers were not flushed up to this point,
// this means the response either had no body or had
// an empty body stream. Flush the headers.
if (httpHeaders.length > 0) {
flushHeaders()
}
flushHeaders()
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's safe to call flushHeaders() because it will do nothing if the headers have already been flushed.


// Close the socket if the connection wasn't marked as keep-alive.
if (!this.shouldKeepAlive) {
Expand Down
23 changes: 22 additions & 1 deletion src/interceptors/ClientRequest/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import { MockAgent, MockHttpsAgent } from './agents'
import { emitAsync } from '../../utils/emitAsync'
import { toInteractiveRequest } from '../../utils/toInteractiveRequest'
import { normalizeClientRequestArgs } from './utils/normalizeClientRequestArgs'
import { isNodeLikeError } from '../../utils/isNodeLikeError'
import { createServerErrorResponse } from '../../utils/responseUtils'

export class ClientRequestInterceptor extends Interceptor<HttpRequestEventMap> {
static symbol = Symbol('client-request-interceptor')
Expand Down Expand Up @@ -144,13 +146,32 @@ export class ClientRequestInterceptor extends Interceptor<HttpRequestEventMap> {
})

if (listenerResult.error) {
socket.errorWith(listenerResult.error)
// Treat thrown Responses as mocked responses.
if (listenerResult.error instanceof Response) {
socket.respondWith(listenerResult.error)
return
}

// Allow mocking Node-like errors.
if (isNodeLikeError(listenerResult.error)) {
socket.errorWith(listenerResult.error)
return
}

// Unhandled exceptions in the request listeners are
// synonymous to unhandled exceptions on the server.
// Those are represented as 500 error responses.
socket.respondWith(createServerErrorResponse(listenerResult.error))
return
}

const mockedResponse = listenerResult.data

if (mockedResponse) {
/**
* @note The `.respondWith()` method will handle "Response.error()".
* Maybe we should make all interceptors do that?
*/
socket.respondWith(mockedResponse)
return
}
Expand Down
32 changes: 10 additions & 22 deletions src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { XMLHttpRequestEmitter } from '.'
import { toInteractiveRequest } from '../../utils/toInteractiveRequest'
import { emitAsync } from '../../utils/emitAsync'
import { XMLHttpRequestController } from './XMLHttpRequestController'
import { createServerErrorResponse } from '../../utils/responseUtils'

export interface XMLHttpRequestProxyOptions {
emitter: XMLHttpRequestEmitter
Expand Down Expand Up @@ -94,31 +95,18 @@ export function createXMLHttpRequestProxy({
resolverResult.error
)

// Treat unhandled exceptions in the "request" listener
// as 500 server errors.
// Treat thrown Responses as mocked responses.
if (resolverResult.error instanceof Response) {
this.respondWith(resolverResult.error)
return
}
// Unhandled exceptions in the request listeners are
// synonymous to unhandled exceptions on the server.
// Those are represented as 500 error responses.
xhrRequestController.respondWith(
new Response(
JSON.stringify({
name: resolverResult.error.name,
message: resolverResult.error.message,
stack: resolverResult.error.stack,
}),
{
status: 500,
statusText: 'Unhandled Exception',
headers: {
'Content-Type': 'application/json',
},
}
)
createServerErrorResponse(resolverResult.error)
)

/**
* @todo Consider forwarding this error to the stderr as well
* since not all consumers are expecting to handle errors.
* If they don't, this error will be swallowed.
*/
// xhrRequestController.errorWith(resolverResult.error)
return
}

Expand Down
107 changes: 53 additions & 54 deletions src/interceptors/fetch/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { emitAsync } from '../../utils/emitAsync'
import { isPropertyAccessible } from '../../utils/isPropertyAccessible'
import { canParseUrl } from '../../utils/canParseUrl'
import { createRequestId } from '../../createRequestId'
import { createServerErrorResponse } from '../../utils/responseUtils'

export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
static symbol = Symbol('fetch')
Expand Down Expand Up @@ -85,51 +86,69 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
)
}

const resolverResult = await until(async () => {
const listenersFinished = emitAsync(this.emitter, 'request', {
const respondWith = (response: Response): Response => {
// Clone the mocked response for the "response" event listener.
// This way, the listener can read the response and not lock its body
// for the actual fetch consumer.
const responseClone = response.clone()

this.emitter.emit('response', {
response: responseClone,
isMockedResponse: true,
request: interactiveRequest,
requestId,
})

await Promise.race([
requestAborted,
// Put the listeners invocation Promise in the same race condition
// with the request abort Promise because otherwise awaiting the listeners
// would always yield some response (or undefined).
listenersFinished,
requestController.responsePromise,
])
// Set the "response.url" property to equal the intercepted request URL.
Object.defineProperty(response, 'url', {
writable: false,
enumerable: true,
configurable: false,
value: request.url,
})

return response
}

this.logger.info('all request listeners have been resolved!')
const resolverResult = await until<unknown, Response | undefined>(
async () => {
const listenersFinished = emitAsync(this.emitter, 'request', {
request: interactiveRequest,
requestId,
})

const mockedResponse = await requestController.responsePromise
this.logger.info('event.respondWith called with:', mockedResponse)
await Promise.race([
requestAborted,
// Put the listeners invocation Promise in the same race condition
// with the request abort Promise because otherwise awaiting the listeners
// would always yield some response (or undefined).
listenersFinished,
requestController.responsePromise,
])

return mockedResponse
})
this.logger.info('all request listeners have been resolved!')

const mockedResponse = await requestController.responsePromise
this.logger.info('event.respondWith called with:', mockedResponse)

return mockedResponse
}
)

if (requestAborted.state === 'rejected') {
return Promise.reject(requestAborted.rejectionReason)
}

if (resolverResult.error) {
// Treat unhandled exceptions from the "request" listeners
// as 500 errors from the server. Fetch API doesn't respect
// Node.js internal errors so no special treatment for those.
return new Response(
JSON.stringify({
name: resolverResult.error.name,
message: resolverResult.error.message,
stack: resolverResult.error.stack,
}),
{
status: 500,
statusText: 'Unhandled Exception',
headers: {
'Content-Type': 'application/json',
},
}
)
// Treat thrown Responses as mocked responses.
if (resolverResult.error instanceof Response) {
return respondWith(resolverResult.error)
}

// Unhandled exceptions in the request listeners are
// synonymous to unhandled exceptions on the server.
// Those are represented as 500 error responses.
return createServerErrorResponse(resolverResult.error)
}

const mockedResponse = resolverResult.data
Expand All @@ -148,7 +167,7 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {

/**
* Set the cause of the request promise rejection to the
* network error Response instance. This different from Undici.
* network error Response instance. This differs from Undici.
* Undici will forward the "response.error" custom property
* as the rejection reason but for "Response.error()" static method
* "response.error" will equal to undefined, making "cause" an empty Error.
Expand All @@ -157,27 +176,7 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
return Promise.reject(createNetworkError(mockedResponse))
}

// Clone the mocked response for the "response" event listener.
// This way, the listener can read the response and not lock its body
// for the actual fetch consumer.
const responseClone = mockedResponse.clone()

this.emitter.emit('response', {
response: responseClone,
isMockedResponse: true,
request: interactiveRequest,
requestId,
})

// Set the "response.url" property to equal the intercepted request URL.
Object.defineProperty(mockedResponse, 'url', {
writable: false,
enumerable: true,
configurable: false,
value: request.url,
})

return mockedResponse
return respondWith(mockedResponse)
}

this.logger.info('no mocked response received!')
Expand Down
24 changes: 24 additions & 0 deletions src/utils/responseUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,27 @@ export const RESPONSE_STATUS_CODES_WITHOUT_BODY = new Set([
export function isResponseWithoutBody(status: number): boolean {
return RESPONSE_STATUS_CODES_WITHOUT_BODY.has(status)
}

/**
* Creates a generic 500 Unhandled Exception response.
*/
export function createServerErrorResponse(body: unknown): Response {
return new Response(
JSON.stringify(
body instanceof Error
? {
name: body.name,
message: body.message,
stack: body.stack,
}
: body
),
{
status: 500,
statusText: 'Unhandled Exception',
headers: {
'Content-Type': 'application/json',
},
}
)
}
Loading
Loading