diff --git a/src/RemoteHttpInterceptor.ts b/src/RemoteHttpInterceptor.ts index 0c1f996b8..784608529 100644 --- a/src/RemoteHttpInterceptor.ts +++ b/src/RemoteHttpInterceptor.ts @@ -8,6 +8,7 @@ import { FetchInterceptor } from './interceptors/fetch' import { handleRequest } from './utils/handleRequest' import { RequestController } from './RequestController' import { FetchResponse } from './utils/fetchUtils' +import { isResponseError } from './utils/responseUtils' export interface SerializedRequest { id: string @@ -178,13 +179,14 @@ export class RemoteHttpResolver extends Interceptor { body: requestJson.body, }) - const controller = new RequestController(request) - await handleRequest({ - request, - requestId: requestJson.id, - controller, - emitter: this.emitter, - onResponse: async (response) => { + const controller = new RequestController(request, { + passthrough: () => {}, + respondWith: async (response) => { + if (isResponseError(response)) { + this.logger.info('received a network error!', { response }) + throw new Error('Not implemented') + } + this.logger.info('received mocked response!', { response }) const responseClone = response.clone() @@ -221,15 +223,18 @@ export class RemoteHttpResolver extends Interceptor { serializedResponse ) }, - onRequestError: (response) => { - this.logger.info('received a network error!', { response }) - throw new Error('Not implemented') - }, - onError: (error) => { - this.logger.info('request has errored!', { error }) + errorWith: (reason) => { + this.logger.info('request has errored!', { error: reason }) throw new Error('Not implemented') }, }) + + await handleRequest({ + request, + requestId: requestJson.id, + controller, + emitter: this.emitter, + }) } this.subscriptions.push(() => { diff --git a/src/RequestController.test.ts b/src/RequestController.test.ts index a7cd495ca..eeed75f0a 100644 --- a/src/RequestController.test.ts +++ b/src/RequestController.test.ts @@ -1,57 +1,104 @@ -import { it, expect } from 'vitest' -import { kResponsePromise, RequestController } from './RequestController' +import { vi, it, expect } from 'vitest' +import { + RequestController, + type RequestControllerSource, +} from './RequestController' +import { InterceptorError } from './InterceptorError' -it('creates a pending response promise on construction', () => { - const controller = new RequestController(new Request('http://localhost')) - expect(controller[kResponsePromise]).toBeInstanceOf(Promise) - expect(controller[kResponsePromise].state).toBe('pending') +const defaultSource = { + passthrough() {}, + respondWith() {}, + errorWith() {}, +} satisfies RequestControllerSource + +it('has a pending state upon construction', () => { + const controller = new RequestController( + new Request('http://localhost'), + defaultSource + ) + + expect(controller.handled).toBeInstanceOf(Promise) + expect(controller.readyState).toBe(RequestController.PENDING) }) -it('resolves the response promise with the response provided to "respondWith"', async () => { - const controller = new RequestController(new Request('http://localhost')) - controller.respondWith(new Response('hello world')) +it('handles a request when calling ".respondWith()" with a mocked response', async () => { + const respondWith = vi.fn() + const controller = new RequestController(new Request('http://localhost'), { + ...defaultSource, + respondWith, + }) + + await controller.respondWith(new Response('hello world')) + + expect(controller.readyState).toBe(RequestController.RESPONSE) + await expect(controller.handled).resolves.toBeUndefined() - const response = (await controller[kResponsePromise]) as Response + expect(respondWith).toHaveBeenCalledOnce() + const [response] = respondWith.mock.calls[0] expect(response).toBeInstanceOf(Response) expect(response.status).toBe(200) - expect(await response.text()).toBe('hello world') + await expect(response.text()).resolves.toBe('hello world') }) -it('resolves the response promise with the error provided to "errorWith"', async () => { - const controller = new RequestController(new Request('http://localhost')) +it('handles the request when calling ".errorWith()" with an error', async () => { + const errorWith = vi.fn() + const controller = new RequestController(new Request('http://localhost'), { + ...defaultSource, + errorWith, + }) + const error = new Error('Oops!') - controller.errorWith(error) + await controller.errorWith(error) + + expect(controller.readyState).toBe(RequestController.ERROR) + await expect(controller.handled).resolves.toBeUndefined() - await expect(controller[kResponsePromise]).resolves.toEqual(error) + expect(errorWith).toHaveBeenCalledOnce() + expect(errorWith).toHaveBeenCalledWith(error) }) -it('resolves the response promise with an arbitrary object provided to "errorWith"', async () => { - const controller = new RequestController(new Request('http://localhost')) +it('handles the request when calling ".errorWith()" with an arbitrary object', async () => { + const errorWith = vi.fn() + const controller = new RequestController(new Request('http://localhost'), { + ...defaultSource, + errorWith, + }) + const error = { message: 'Oops!' } - controller.errorWith(error) + await controller.errorWith(error) - await expect(controller[kResponsePromise]).resolves.toEqual(error) + expect(controller.readyState).toBe(RequestController.ERROR) + await expect(controller.handled).resolves.toBeUndefined() + + expect(errorWith).toHaveBeenCalledOnce() + expect(errorWith).toHaveBeenCalledWith(error) }) -it('throws when calling "respondWith" multiple times', () => { - const controller = new RequestController(new Request('http://localhost')) +it('throws when calling "respondWith" multiple times', async () => { + const controller = new RequestController( + new Request('http://localhost'), + defaultSource + ) controller.respondWith(new Response('hello world')) - expect(() => { - controller.respondWith(new Response('second response')) - }).toThrow( - 'Failed to respond to the "GET http://localhost/" request: the "request" event has already been handled.' + expect(() => controller.respondWith(new Response('second response'))).toThrow( + new InterceptorError( + 'Failed to respond to the "GET http://localhost/" request with "200 OK": the request has already been handled (2)' + ) ) }) -it('throws when calling "errorWith" multiple times', () => { - const controller = new RequestController(new Request('http://localhost')) +it('throws when calling "errorWith" multiple times', async () => { + const controller = new RequestController( + new Request('http://localhost'), + defaultSource + ) controller.errorWith(new Error('Oops!')) - expect(() => { - controller.errorWith(new Error('second error')) - }).toThrow( - 'Failed to error the "GET http://localhost/" request: the "request" event has already been handled.' + expect(() => controller.errorWith(new Error('second error'))).toThrow( + new InterceptorError( + 'Failed to error the "GET http://localhost/" request with "Error: second error": the request has already been handled (3)' + ) ) }) diff --git a/src/RequestController.ts b/src/RequestController.ts index 1d47f9f65..57486d848 100644 --- a/src/RequestController.ts +++ b/src/RequestController.ts @@ -1,35 +1,59 @@ -import { invariant } from 'outvariant' import { DeferredPromise } from '@open-draft/deferred-promise' +import { invariant } from 'outvariant' import { InterceptorError } from './InterceptorError' -const kRequestHandled = Symbol('kRequestHandled') -export const kResponsePromise = Symbol('kResponsePromise') +export interface RequestControllerSource { + passthrough(): void + respondWith(response: Response): void + errorWith(reason?: unknown): void +} export class RequestController { + static PENDING = 0 as const + static PASSTHROUGH = 1 as const + static RESPONSE = 2 as const + static ERROR = 3 as const + + public readyState: number + /** - * Internal response promise. - * Available only for the library internals to grab the - * response instance provided by the developer. - * @note This promise cannot be rejected. It's either infinitely - * pending or resolved with whichever Response was passed to `respondWith()`. + * A Promise that resolves when this controller handles a request. + * See `controller.readyState` for more information on the handling result. */ - [kResponsePromise]: DeferredPromise< - Response | Record | undefined - >; + public handled: Promise + + constructor( + protected readonly request: Request, + protected readonly source: RequestControllerSource + ) { + this.readyState = RequestController.PENDING + this.handled = new DeferredPromise() + } + + get #handled() { + return this.handled as DeferredPromise + } /** - * Internal flag indicating if this request has been handled. - * @note The response promise becomes "fulfilled" on the next tick. + * Perform this request as-is. */ - [kRequestHandled]: boolean + public async passthrough(): Promise { + invariant.as( + InterceptorError, + this.readyState === RequestController.PENDING, + 'Failed to passthrough the "%s %s" request: the request has already been handled', + this.request.method, + this.request.url + ) - constructor(private request: Request) { - this[kRequestHandled] = false - this[kResponsePromise] = new DeferredPromise() + this.readyState = RequestController.PASSTHROUGH + await this.source.passthrough() + this.#handled.resolve() } /** * Respond to this request with the given `Response` instance. + * * @example * controller.respondWith(new Response()) * controller.respondWith(Response.json({ id })) @@ -38,22 +62,25 @@ export class RequestController { public respondWith(response: Response): void { invariant.as( InterceptorError, - !this[kRequestHandled], - 'Failed to respond to the "%s %s" request: the "request" event has already been handled.', + this.readyState === RequestController.PENDING, + 'Failed to respond to the "%s %s" request with "%d %s": the request has already been handled (%d)', this.request.method, - this.request.url + this.request.url, + response.status, + response.statusText || 'OK', + this.readyState ) - this[kRequestHandled] = true - this[kResponsePromise].resolve(response) + this.readyState = RequestController.RESPONSE + this.#handled.resolve() /** - * @note The request controller doesn't do anything - * apart from letting the interceptor await the response - * provided by the developer through the response promise. - * Each interceptor implements the actual respondWith/errorWith - * logic based on that interceptor's needs. + * @note Although `source.respondWith()` is potentially asynchronous, + * do NOT await it for backward-compatibility. Awaiting it will short-circuit + * the request listener invocation as soon as a listener responds to a request. + * Ideally, that's what we want, but that's not what we promise the user. */ + this.source.respondWith(response) } /** @@ -64,22 +91,19 @@ export class RequestController { * controller.errorWith(new Error('Oops!')) * controller.errorWith({ message: 'Oops!'}) */ - public errorWith(reason?: Error | Record): void { + public errorWith(reason?: unknown): void { invariant.as( InterceptorError, - !this[kRequestHandled], - 'Failed to error the "%s %s" request: the "request" event has already been handled.', + this.readyState === RequestController.PENDING, + 'Failed to error the "%s %s" request with "%s": the request has already been handled (%d)', this.request.method, - this.request.url + this.request.url, + reason?.toString(), + this.readyState ) - this[kRequestHandled] = true - - /** - * @note Resolve the response promise, not reject. - * This helps us differentiate between unhandled exceptions - * and intended errors ("errorWith") while waiting for the response. - */ - this[kResponsePromise].resolve(reason) + this.readyState = RequestController.ERROR + this.source.errorWith(reason) + this.#handled.resolve() } } diff --git a/src/index.ts b/src/index.ts index 8e334b58f..151f28007 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,6 +1,10 @@ export * from './glossary' export * from './Interceptor' export * from './BatchInterceptor' +export { + RequestController, + type RequestControllerSource, +} from './RequestController' /* Utils */ export { createRequestId } from './createRequestId' diff --git a/src/interceptors/ClientRequest/MockHttpSocket.ts b/src/interceptors/ClientRequest/MockHttpSocket.ts index c4fe832c4..9a9f4d497 100644 --- a/src/interceptors/ClientRequest/MockHttpSocket.ts +++ b/src/interceptors/ClientRequest/MockHttpSocket.ts @@ -311,6 +311,16 @@ export class MockHttpSocket extends MockSocket { return } + // Prevent recursive calls. + invariant( + this.socketState !== 'mock', + '[MockHttpSocket] Failed to respond to the "%s %s" request with "%s %s": the request has already been handled', + this.request?.method, + this.request?.url, + response.status, + response.statusText + ) + // Handle "type: error" responses. if (isPropertyAccessible(response, 'type') && response.type === 'error') { this.errorWith(new TypeError('Network error')) @@ -393,9 +403,18 @@ export class MockHttpSocket extends MockSocket { serverResponse.write(value) } } catch (error) { - // Coerce response stream errors to 500 responses. - this.respondWith(createServerErrorResponse(error)) - return + if (error instanceof Error) { + serverResponse.destroy() + /** + * @note Destroy the request socket gracefully. + * Response stream errors do NOT produce request errors. + */ + this.destroy() + return + } + + serverResponse.destroy() + throw error } } else { serverResponse.end() diff --git a/src/interceptors/ClientRequest/index.ts b/src/interceptors/ClientRequest/index.ts index 24a2d1d29..471233614 100644 --- a/src/interceptors/ClientRequest/index.ts +++ b/src/interceptors/ClientRequest/index.ts @@ -153,30 +153,26 @@ export class ClientRequestInterceptor extends Interceptor { request, socket, }) => { - const requestId = Reflect.get(request, kRequestId) - const controller = new RequestController(request) - - const isRequestHandled = await handleRequest({ - request, - requestId, - controller, - emitter: this.emitter, - onResponse: (response) => { - socket.respondWith(response) + const controller = new RequestController(request, { + passthrough() { + socket.passthrough() }, - onRequestError: (response) => { - socket.respondWith(response) + async respondWith(response) { + await socket.respondWith(response) }, - onError: (error) => { - if (error instanceof Error) { - socket.errorWith(error) + errorWith(reason) { + if (reason instanceof Error) { + socket.errorWith(reason) } }, }) - if (!isRequestHandled) { - return socket.passthrough() - } + await handleRequest({ + request, + requestId: Reflect.get(request, kRequestId), + controller, + emitter: this.emitter, + }) } public onResponse: MockHttpSocketResponseCallback = async ({ diff --git a/src/interceptors/XMLHttpRequest/XMLHttpRequestController.ts b/src/interceptors/XMLHttpRequest/XMLHttpRequestController.ts index 37aa3141c..b9b1fa04f 100644 --- a/src/interceptors/XMLHttpRequest/XMLHttpRequestController.ts +++ b/src/interceptors/XMLHttpRequest/XMLHttpRequestController.ts @@ -57,7 +57,10 @@ export class XMLHttpRequestController { Array > - constructor(readonly initialRequest: XMLHttpRequest, public logger: Logger) { + constructor( + readonly initialRequest: XMLHttpRequest, + public logger: Logger + ) { this[kIsRequestHandled] = false this.events = new Map() @@ -111,7 +114,7 @@ export class XMLHttpRequestController { case 'addEventListener': { const [eventName, listener] = args as [ keyof XMLHttpRequestEventTargetEventMap, - Function + Function, ] this.registerEvent(eventName, listener) @@ -131,7 +134,7 @@ export class XMLHttpRequestController { case 'send': { const [body] = args as [ - body?: XMLHttpRequestBodyInit | Document | null + body?: XMLHttpRequestBodyInit | Document | null, ] this.request.addEventListener('load', () => { @@ -166,38 +169,44 @@ export class XMLHttpRequestController { const fetchRequest = this.toFetchApiRequest(requestBody) this[kFetchRequest] = fetchRequest.clone() - const onceRequestSettled = - this.onRequest?.call(this, { - request: fetchRequest, - requestId: this.requestId!, - }) || Promise.resolve() - - onceRequestSettled.finally(() => { - // If the consumer didn't handle the request (called `.respondWith()`) perform it as-is. - if (!this[kIsRequestHandled]) { - this.logger.info( - 'request callback settled but request has not been handled (readystate %d), performing as-is...', - this.request.readyState - ) - - /** - * @note Set the intercepted request ID on the original request in Node.js - * so that if it triggers any other interceptors, they don't attempt - * to process it once again. - * - * For instance, XMLHttpRequest is often implemented via "http.ClientRequest" - * and we don't want for both XHR and ClientRequest interceptors to - * handle the same request at the same time (e.g. emit the "response" event twice). - */ - if (IS_NODE) { - this.request.setRequestHeader( - INTERNAL_REQUEST_ID_HEADER_NAME, - this.requestId! + /** + * @note Start request handling on the next tick so that the user + * could add event listeners for "loadend" before the interceptor fires it. + */ + queueMicrotask(() => { + const onceRequestSettled = + this.onRequest?.call(this, { + request: fetchRequest, + requestId: this.requestId!, + }) || Promise.resolve() + + onceRequestSettled.finally(() => { + // If the consumer didn't handle the request (called `.respondWith()`) perform it as-is. + if (!this[kIsRequestHandled]) { + this.logger.info( + 'request callback settled but request has not been handled (readystate %d), performing as-is...', + this.request.readyState ) - } - return invoke() - } + /** + * @note Set the intercepted request ID on the original request in Node.js + * so that if it triggers any other interceptors, they don't attempt + * to process it once again. + * + * For instance, XMLHttpRequest is often implemented via "http.ClientRequest" + * and we don't want for both XHR and ClientRequest interceptors to + * handle the same request at the same time (e.g. emit the "response" event twice). + */ + if (IS_NODE) { + this.request.setRequestHeader( + INTERNAL_REQUEST_ID_HEADER_NAME, + this.requestId! + ) + } + + return invoke() + } + }) }) break @@ -241,7 +250,7 @@ export class XMLHttpRequestController { case 'addEventListener': { const [eventName, listener] = args as [ keyof XMLHttpRequestEventTargetEventMap, - Function + Function, ] this.registerUploadEvent(eventName, listener) this.logger.info('upload.addEventListener', eventName, listener) @@ -312,6 +321,7 @@ export class XMLHttpRequestController { loaded: totalRequestBodyLength, total: totalRequestBodyLength, }) + this.trigger('loadend', this.request.upload, { loaded: totalRequestBodyLength, total: totalRequestBodyLength, @@ -614,7 +624,7 @@ export class XMLHttpRequestController { private trigger< EventName extends keyof (XMLHttpRequestEventTargetEventMap & { readystatechange: ProgressEvent - }) + }), >( eventName: EventName, target: XMLHttpRequest | XMLHttpRequestUpload, diff --git a/src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts b/src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts index 997f06fa0..4cfe8b89b 100644 --- a/src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts +++ b/src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts @@ -3,6 +3,7 @@ import { XMLHttpRequestEmitter } from '.' import { RequestController } from '../../RequestController' import { XMLHttpRequestController } from './XMLHttpRequestController' import { handleRequest } from '../../utils/handleRequest' +import { isResponseError } from '../../utils/responseUtils' export interface XMLHttpRequestProxyOptions { emitter: XMLHttpRequestEmitter @@ -52,7 +53,28 @@ export function createXMLHttpRequestProxy({ ) xhrRequestController.onRequest = async function ({ request, requestId }) { - const controller = new RequestController(request) + const controller = new RequestController(request, { + passthrough: () => { + this.logger.info( + 'no mocked response received, performing request as-is...' + ) + }, + respondWith: async (response) => { + if (isResponseError(response)) { + this.errorWith(new TypeError('Network error')) + return + } + + await this.respondWith(response) + }, + errorWith: (reason) => { + this.logger.info('request errored!', { error: reason }) + + if (reason instanceof Error) { + this.errorWith(reason) + } + }, + }) this.logger.info('awaiting mocked response...') @@ -61,31 +83,12 @@ export function createXMLHttpRequestProxy({ emitter.listenerCount('request') ) - const isRequestHandled = await handleRequest({ + await handleRequest({ request, requestId, controller, emitter, - onResponse: async (response) => { - await this.respondWith(response) - }, - onRequestError: () => { - this.errorWith(new TypeError('Network error')) - }, - onError: (error) => { - this.logger.info('request errored!', { error }) - - if (error instanceof Error) { - this.errorWith(error) - } - }, }) - - if (!isRequestHandled) { - this.logger.info( - 'no mocked response received, performing request as-is...' - ) - } } xhrRequestController.onResponse = async function ({ diff --git a/src/interceptors/fetch/index.ts b/src/interceptors/fetch/index.ts index f2d82aecb..9eabba4b9 100644 --- a/src/interceptors/fetch/index.ts +++ b/src/interceptors/fetch/index.ts @@ -1,4 +1,5 @@ import { invariant } from 'outvariant' +import { until } from '@open-draft/until' import { DeferredPromise } from '@open-draft/deferred-promise' import { HttpRequestEventMap, IS_PATCHED_MODULE } from '../../glossary' import { Interceptor } from '../../Interceptor' @@ -13,6 +14,7 @@ import { decompressResponse } from './utils/decompression' import { hasConfigurableGlobal } from '../../utils/hasConfigurableGlobal' import { FetchResponse } from '../../utils/fetchUtils' import { setRawRequest } from '../../getRawRequest' +import { isResponseError } from '../../utils/responseUtils' export class FetchInterceptor extends Interceptor { static symbol = Symbol('fetch') @@ -59,22 +61,54 @@ export class FetchInterceptor extends Interceptor { } const responsePromise = new DeferredPromise() - const controller = new RequestController(request) - this.logger.info('[%s] %s', request.method, request.url) - this.logger.info('awaiting for the mocked response...') + const controller = new RequestController(request, { + passthrough: async () => { + this.logger.info('request has not been handled, passthrough...') - this.logger.info( - 'emitting the "request" event for %s listener(s)...', - this.emitter.listenerCount('request') - ) + /** + * @note Clone the request instance right before performing it. + * This preserves any modifications made to the intercepted request + * in the "request" listener. This also allows the user to read the + * request body in the "response" listener (otherwise "unusable"). + */ + const requestCloneForResponseEvent = request.clone() + + // Perform the intercepted request as-is. + const { error: responseError, data: originalResponse } = await until( + () => pureFetch(request) + ) + + if (responseError) { + return responsePromise.reject(responseError) + } + + this.logger.info('original fetch performed', originalResponse) + + if (this.emitter.listenerCount('response') > 0) { + this.logger.info('emitting the "response" event...') + + const responseClone = originalResponse.clone() + await emitAsync(this.emitter, 'response', { + response: responseClone, + isMockedResponse: false, + request: requestCloneForResponseEvent, + requestId, + }) + } + + // Resolve the response promise with the original response + // since the `fetch()` return this internal promise. + responsePromise.resolve(originalResponse) + }, + respondWith: async (rawResponse) => { + // Handle mocked `Response.error()` (i.e. request errors). + if (isResponseError(rawResponse)) { + this.logger.info('request has errored!', { response: rawResponse }) + responsePromise.reject(createNetworkError(rawResponse)) + return + } - const isRequestHandled = await handleRequest({ - request, - requestId, - emitter: this.emitter, - controller, - onResponse: async (rawResponse) => { this.logger.info('received mocked response!', { rawResponse, }) @@ -134,51 +168,28 @@ export class FetchInterceptor extends Interceptor { responsePromise.resolve(response) }, - onRequestError: (response) => { - this.logger.info('request has errored!', { response }) - responsePromise.reject(createNetworkError(response)) - }, - onError: (error) => { - this.logger.info('request has been aborted!', { error }) - responsePromise.reject(error) + errorWith: (reason) => { + this.logger.info('request has been aborted!', { reason }) + responsePromise.reject(reason) }, }) - if (isRequestHandled) { - this.logger.info('request has been handled, returning mock promise...') - return responsePromise - } + this.logger.info('[%s] %s', request.method, request.url) + this.logger.info('awaiting for the mocked response...') this.logger.info( - 'no mocked response received, performing request as-is...' + 'emitting the "request" event for %s listener(s)...', + this.emitter.listenerCount('request') ) - /** - * @note Clone the request instance right before performing it. - * This preserves any modifications made to the intercepted request - * in the "request" listener. This also allows the user to read the - * request body in the "response" listener (otherwise "unusable"). - */ - const requestCloneForResponseEvent = request.clone() - - return pureFetch(request).then(async (response) => { - this.logger.info('original fetch performed', response) - - if (this.emitter.listenerCount('response') > 0) { - this.logger.info('emitting the "response" event...') - - const responseClone = response.clone() - - await emitAsync(this.emitter, 'response', { - response: responseClone, - isMockedResponse: false, - request: requestCloneForResponseEvent, - requestId, - }) - } - - return response + await handleRequest({ + request, + requestId, + emitter: this.emitter, + controller, }) + + return responsePromise } Object.defineProperty(globalThis.fetch, IS_PATCHED_MODULE, { diff --git a/src/utils/RequestController.ts b/src/utils/RequestController.ts deleted file mode 100644 index d93e70b3d..000000000 --- a/src/utils/RequestController.ts +++ /dev/null @@ -1,21 +0,0 @@ -import { invariant } from 'outvariant' -import { DeferredPromise } from '@open-draft/deferred-promise' - -export class RequestController { - public responsePromise: DeferredPromise - - constructor(protected request: Request) { - this.responsePromise = new DeferredPromise() - } - - public respondWith(response?: Response): void { - invariant( - this.responsePromise.state === 'pending', - 'Failed to respond to "%s %s" request: the "request" event has already been responded to.', - this.request.method, - this.request.url - ) - - this.responsePromise.resolve(response) - } -} diff --git a/src/utils/handleRequest.ts b/src/utils/handleRequest.ts index d2f1ba97e..4ef6fcd5a 100644 --- a/src/utils/handleRequest.ts +++ b/src/utils/handleRequest.ts @@ -3,12 +3,11 @@ import { DeferredPromise } from '@open-draft/deferred-promise' import { until } from '@open-draft/until' import type { HttpRequestEventMap } from '../glossary' import { emitAsync } from './emitAsync' -import { kResponsePromise, RequestController } from '../RequestController' +import { RequestController } from '../RequestController' import { createServerErrorResponse, isResponseError, isResponseLike, - ResponseError, } from './responseUtils' import { InterceptorError } from '../InterceptorError' import { isNodeLikeError } from './isNodeLikeError' @@ -19,43 +18,22 @@ interface HandleRequestOptions { request: Request emitter: Emitter controller: RequestController - - /** - * Called when the request has been handled - * with the given `Response` instance. - */ - onResponse: (response: Response) => void | Promise - - /** - * Called when the request has been handled - * with the given `Response.error()` instance. - */ - onRequestError: (response: ResponseError) => void - - /** - * Called when an unhandled error happens during the - * request handling. This is never a thrown error/response. - */ - onError: (error: unknown) => void } -/** - * @returns {Promise} Indicates whether the request has been handled. - */ export async function handleRequest( options: HandleRequestOptions -): Promise { +): Promise { const handleResponse = async ( response: Response | Error | Record ) => { if (response instanceof Error) { - options.onError(response) + await options.controller.errorWith(response) return true } // Handle "Response.error()" instances. if (isResponseError(response)) { - options.onRequestError(response) + await options.controller.respondWith(response) return true } @@ -65,13 +43,13 @@ export async function handleRequest( * since Response instances are, in fact, objects. */ if (isResponseLike(response)) { - await options.onResponse(response) + await options.controller.respondWith(response) return true } // Handle arbitrary objects provided to `.errorWith(reason)`. if (isObject(response)) { - options.onError(response) + await options.controller.errorWith(response) return true } @@ -87,7 +65,7 @@ export async function handleRequest( // Support mocking Node.js-like errors. if (isNodeLikeError(error)) { - options.onError(error) + await options.controller.errorWith(error) return true } @@ -102,15 +80,14 @@ export async function handleRequest( // Add the last "request" listener to check if the request // has been handled in any way. If it hasn't, resolve the // response promise with undefined. - options.emitter.once('request', ({ requestId: pendingRequestId }) => { - if (pendingRequestId !== options.requestId) { - return - } - - if (options.controller[kResponsePromise].state === 'pending') { - options.controller[kResponsePromise].resolve(undefined) - } - }) + // options.emitter.once('request', async ({ requestId: pendingRequestId }) => { + // if ( + // pendingRequestId === options.requestId && + // options.controller.readyState === RequestController.PENDING + // ) { + // await options.controller.passthrough() + // } + // }) const requestAbortPromise = new DeferredPromise() @@ -119,16 +96,17 @@ export async function handleRequest( */ if (options.request.signal) { if (options.request.signal.aborted) { - requestAbortPromise.reject(options.request.signal.reason) - } else { - options.request.signal.addEventListener( - 'abort', - () => { - requestAbortPromise.reject(options.request.signal.reason) - }, - { once: true } - ) + await options.controller.errorWith(options.request.signal.reason) + return } + + options.request.signal.addEventListener( + 'abort', + () => { + requestAbortPromise.reject(options.request.signal.reason) + }, + { once: true } + ) } const result = await until(async () => { @@ -146,25 +124,21 @@ export async function handleRequest( // Short-circuit the request handling promise if the request gets aborted. requestAbortPromise, requestListenersPromise, - options.controller[kResponsePromise], + options.controller.handled, ]) - - // The response promise will settle immediately once - // the developer calls either "respondWith" or "errorWith". - return await options.controller[kResponsePromise] }) // Handle the request being aborted while waiting for the request listeners. if (requestAbortPromise.state === 'rejected') { - options.onError(requestAbortPromise.rejectionReason) - return true + await options.controller.errorWith(requestAbortPromise.rejectionReason) + return } if (result.error) { // Handle the error during the request listener execution. // These can be thrown responses or request errors. if (await handleResponseError(result.error)) { - return true + return } // If the developer has added "unhandledException" listeners, @@ -175,7 +149,28 @@ export async function handleRequest( // This is needed because the original controller might have been already // interacted with (e.g. "respondWith" or "errorWith" called on it). const unhandledExceptionController = new RequestController( - options.request + options.request, + { + /** + * @note Intentionally empty passthrough handle. + * This controller is created within another controller and we only need + * to know if `unhandledException` listeners handled the request. + */ + passthrough() {}, + async respondWith(response) { + await handleResponse(response) + }, + async errorWith(reason) { + /** + * @note Handle the result of the unhandled controller + * in the same way as the original request controller. + * The exception here is that thrown errors within the + * "unhandledException" event do NOT result in another + * emit of the same event. They are forwarded as-is. + */ + await options.controller.errorWith(reason) + }, + } ) await emitAsync(options.emitter, 'unhandledException', { @@ -183,53 +178,28 @@ export async function handleRequest( request: options.request, requestId: options.requestId, controller: unhandledExceptionController, - }).then(() => { - // If all the "unhandledException" listeners have finished - // but have not handled the response in any way, preemptively - // resolve the pending response promise from the new controller. - // This prevents it from hanging forever. - if ( - unhandledExceptionController[kResponsePromise].state === 'pending' - ) { - unhandledExceptionController[kResponsePromise].resolve(undefined) - } }) - const nextResult = await until( - () => unhandledExceptionController[kResponsePromise] - ) - - /** - * @note Handle the result of the unhandled controller - * in the same way as the original request controller. - * The exception here is that thrown errors within the - * "unhandledException" event do NOT result in another - * emit of the same event. They are forwarded as-is. - */ - if (nextResult.error) { - return handleResponseError(nextResult.error) - } - - if (nextResult.data) { - return handleResponse(nextResult.data) + // If all the "unhandledException" listeners have finished + // but have not handled the request in any way, passthrough. + if ( + unhandledExceptionController.readyState !== RequestController.PENDING + ) { + return } } // Otherwise, coerce unhandled exceptions to a 500 Internal Server Error response. - options.onResponse(createServerErrorResponse(result.error)) - return true + await options.controller.respondWith( + createServerErrorResponse(result.error) + ) + return } - /** - * Handle a mocked Response instance. - * @note That this can also be an Error in case - * the developer called "errorWith". This differentiates - * unhandled exceptions from intended errors. - */ - if (result.data) { - return handleResponse(result.data) + // If the request hasn't been handled by this point, passthrough. + if (options.controller.readyState === RequestController.PENDING) { + return await options.controller.passthrough() } - // In all other cases, consider the request unhandled. - return false + return options.controller.handled } diff --git a/test/features/get-client-request-body-stream.test.ts b/test/features/get-client-request-body-stream.test.ts index 7f575a065..2e0e7db17 100644 --- a/test/features/get-client-request-body-stream.test.ts +++ b/test/features/get-client-request-body-stream.test.ts @@ -29,7 +29,7 @@ afterAll(() => { it('returns the underlying request body stream for http.ClientRequest', async () => { const requestBodyStreamPromise = new DeferredPromise() - interceptor.on('request', async ({ request, controller }) => { + interceptor.on('request', ({ request, controller }) => { try { const requestBodyStream = getClientRequestBodyStream(request) requestBodyStreamPromise.resolve(requestBodyStream) @@ -52,10 +52,10 @@ it('returns the underlying request body stream for http.ClientRequest', async () request.write('hello world') request.end() + await waitForClientRequest(request) const requestBodyStream = await requestBodyStreamPromise expect(requestBodyStream).toBeInstanceOf(Readable) - await waitForClientRequest(request) await expect(text(requestBodyStream)).resolves.toBe('hello world') }) diff --git a/test/features/presets/node-preset.test.ts b/test/features/presets/node-preset.test.ts index 6ca0ac6cf..d78e7c956 100644 --- a/test/features/presets/node-preset.test.ts +++ b/test/features/presets/node-preset.test.ts @@ -1,6 +1,6 @@ // @vitest-environment jsdom import { vi, it, expect, beforeAll, afterEach, afterAll } from 'vitest' -import http from 'http' +import http from 'node:http' import { BatchInterceptor } from '../../../lib/node' import nodeInterceptors from '../../../lib/node/presets/node' import { createXMLHttpRequest, waitForClientRequest } from '../../helpers' @@ -42,7 +42,7 @@ it('intercepts and mocks a ClientRequest', async () => { // The listener must send back a mocked response. expect(res.statusCode).toBe(200) - expect(await text()).toBe('mocked') + await expect(text()).resolves.toBe('mocked') }) it('intercepts and mocks an XMLHttpRequest (jsdom)', async () => { @@ -73,5 +73,5 @@ it('intercepts and mocks a fetch request', async () => { ) expect(response.status).toBe(200) - expect(await response.text()).toBe('mocked') + await expect(response.text()).resolves.toBe('mocked') }) diff --git a/test/helpers.ts b/test/helpers.ts index 3c2be0dec..cd4ba87e5 100644 --- a/test/helpers.ts +++ b/test/helpers.ts @@ -6,6 +6,7 @@ import { Page } from '@playwright/test' import { getIncomingMessageBody } from '../src/interceptors/ClientRequest/utils/getIncomingMessageBody' import { SerializedRequest } from '../src/RemoteHttpInterceptor' import { RequestHandler } from 'express' +import { DeferredPromise } from '@open-draft/deferred-promise' export const REQUEST_ID_REGEXP = /^\w{9,}$/ @@ -234,7 +235,7 @@ export function createRawBrowserXMLHttpRequest(page: Page) { Record | undefined, string | undefined, boolean | undefined, - boolean + boolean, ] >( (args) => { diff --git a/test/modules/XMLHttpRequest/features/events.test.ts b/test/modules/XMLHttpRequest/features/events.test.ts index 11fca22c5..e068ceaf0 100644 --- a/test/modules/XMLHttpRequest/features/events.test.ts +++ b/test/modules/XMLHttpRequest/features/events.test.ts @@ -1,6 +1,4 @@ -/** - * @vitest-environment jsdom - */ +// @vitest-environment jsdom import { vi, it, expect, beforeAll, afterAll } from 'vitest' import { HttpServer } from '@open-draft/test-server/http' import { XMLHttpRequestInterceptor } from '../../../../src/interceptors/XMLHttpRequest' diff --git a/test/modules/fetch/compliance/abort-conrtoller.test.ts b/test/modules/fetch/compliance/abort-conrtoller.test.ts index 54fa0c21a..ed0d9e5e2 100644 --- a/test/modules/fetch/compliance/abort-conrtoller.test.ts +++ b/test/modules/fetch/compliance/abort-conrtoller.test.ts @@ -48,8 +48,8 @@ it('aborts unsent request when the original request is aborted', async () => { controller.abort() const abortError = await abortErrorPromise - expect(abortError.name).toBe('AbortError') - expect(abortError.message).toBe('This operation was aborted') + expect.soft(abortError.name).toBe('AbortError') + expect.soft(abortError.message).toBe('This operation was aborted') }) it('aborts a pending request when the original request is aborted', async () => { @@ -141,6 +141,6 @@ it('respects requests aborted before they are dispatched', async () => { (error) => error ) - expect(abortError.name).toBe('AbortError') - expect(abortError.message).toBe('This operation was aborted') + expect.soft(abortError.name).toBe('AbortError') + expect.soft(abortError.message).toBe('This operation was aborted') }) diff --git a/test/modules/fetch/fetch-request-controller.test.ts b/test/modules/fetch/fetch-request-controller.test.ts index 62a50f527..fd6d4f4ac 100644 --- a/test/modules/fetch/fetch-request-controller.test.ts +++ b/test/modules/fetch/fetch-request-controller.test.ts @@ -56,7 +56,7 @@ it('throws if "respondWith" is called multiple times within the same listener', expect(error).toHaveProperty('name', 'InterceptorError') expect(error).toHaveProperty( 'message', - `Failed to respond to the "GET http://localhost/resource" request: the "request" event has already been handled.` + `Failed to respond to the "GET http://localhost/resource" request with "200 OK": the request has already been handled (2)` ) // Must respond to the request using the first mocked response. @@ -93,7 +93,7 @@ it('throws if "respondWith" is called multiple times across different listeners' expect(error).toHaveProperty('name', 'InterceptorError') expect(error).toHaveProperty( 'message', - `Failed to respond to the "GET http://localhost/resource" request: the "request" event has already been handled.` + `Failed to respond to the "GET http://localhost/resource" request with "200 OK": the request has already been handled (2)` ) // Must respond to the request using the first mocked response. @@ -160,7 +160,7 @@ it('throws if "errorWith" is called multiple times within the same listener', as expect(error).toHaveProperty('name', 'InterceptorError') expect(error).toHaveProperty( 'message', - `Failed to error the "GET http://localhost/resource" request: the "request" event has already been handled.` + `Failed to error the "GET http://localhost/resource" request with "Error: two": the request has already been handled (3)` ) // Must reject the response promise with the given error. @@ -204,7 +204,7 @@ it('throws if "errorWith" is called multiple times across different listeners', expect(error).toHaveProperty('name', 'InterceptorError') expect(error).toHaveProperty( 'message', - `Failed to error the "GET http://localhost/resource" request: the "request" event has already been handled.` + `Failed to error the "GET http://localhost/resource" request with "Error: two": the request has already been handled (3)` ) // Must reject the response promise with the given error. @@ -246,7 +246,7 @@ it('throws if "respondWith" is called after "errorWith" was called', async () => expect(error).toHaveProperty('name', 'InterceptorError') expect(error).toHaveProperty( 'message', - `Failed to respond to the "GET http://localhost/resource" request: the "request" event has already been handled.` + `Failed to respond to the "GET http://localhost/resource" request with "200 OK": the request has already been handled (3)` ) // Must reject the response promise with the given error. @@ -282,10 +282,10 @@ it('throws if "errorWith" is called after "respondWith" was called', async () => expect(error).toHaveProperty('name', 'InterceptorError') expect(error).toHaveProperty( 'message', - `Failed to error the "GET http://localhost/resource" request: the "request" event has already been handled.` + `Failed to error the "GET http://localhost/resource" request with "Error: one": the request has already been handled (2)` ) // Must resolve the response promise with the given mocked response. - expect(response.status).toBe(200) - await expect(response.text()).resolves.toBe('mock') + expect.soft(response.status).toBe(200) + await expect.soft(response.text()).resolves.toBe('mock') }) diff --git a/test/modules/http/compliance/http-unhandled-exception.test.ts b/test/modules/http/compliance/http-unhandled-exception.test.ts index 9dc75a55b..2adf5a81a 100644 --- a/test/modules/http/compliance/http-unhandled-exception.test.ts +++ b/test/modules/http/compliance/http-unhandled-exception.test.ts @@ -1,6 +1,4 @@ -/** - * @vitest-environment node - */ +// @vitest-environment node import { vi, it, expect, beforeAll, afterEach, afterAll } from 'vitest' import http from 'node:http' import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' diff --git a/test/modules/http/response/http-response-error.test.ts b/test/modules/http/response/http-response-error.test.ts index 21fa65abe..7f657ee3d 100644 --- a/test/modules/http/response/http-response-error.test.ts +++ b/test/modules/http/response/http-response-error.test.ts @@ -1,3 +1,4 @@ +// @vitest-environment node import { vi, it, expect, beforeAll, afterEach, afterAll } from 'vitest' import http from 'node:http' import { ClientRequestInterceptor } from '../../../../src/interceptors/ClientRequest' @@ -28,13 +29,11 @@ it('treats "Response.error()" as a network error', async () => { const request = http.get('http://localhost:3001/resource') request.on('error', requestErrorListener) - // Must handle Response.error() as a network error. - await vi.waitFor(() => { - expect(requestErrorListener).toHaveBeenNthCalledWith( - 1, - new TypeError('Network error') - ) - }) + await expect + .poll(() => requestErrorListener, { + message: 'Handles Response.erorr() as a request error', + }) + .toHaveBeenNthCalledWith(1, new TypeError('Network error')) expect(responseListener).not.toHaveBeenCalled() }) @@ -51,13 +50,12 @@ it('treats a thrown Response.error() as a network error', async () => { const request = http.get('http://localhost:3001/resource') request.on('error', requestErrorListener) - // Must handle Response.error() as a request error. - await vi.waitFor(() => { - expect(requestErrorListener).toHaveBeenNthCalledWith( - 1, - new TypeError('Network error') - ) - }) + await expect + .poll(() => requestErrorListener, { + message: 'Handles Response.error() as a request error', + }) + .toHaveBeenCalledWith(new TypeError('Network error')) - expect(responseListener).not.toHaveBeenCalled() + expect.soft(requestErrorListener).toHaveBeenCalledOnce() + expect.soft(responseListener).not.toHaveBeenCalled() }) diff --git a/test/modules/http/response/http-response-readable-stream.test.ts b/test/modules/http/response/http-response-readable-stream.test.ts index 1a40db0a2..7b20f018b 100644 --- a/test/modules/http/response/http-response-readable-stream.test.ts +++ b/test/modules/http/response/http-response-readable-stream.test.ts @@ -105,17 +105,16 @@ it('supports delays when enqueuing chunks', async () => { expect(chunkTimings[2] - chunkTimings[1]).toBeGreaterThanOrEqual(150) }) -it('forwards ReadableStream errors to the request', async () => { +it('handles immediate response stream errors as request errors', async () => { const requestErrorListener = vi.fn() const responseErrorListener = vi.fn() + const streamError = new Error('stream error') interceptor.once('request', ({ controller }) => { const stream = new ReadableStream({ - start(controller) { + async start(controller) { controller.enqueue(new TextEncoder().encode('original')) - queueMicrotask(() => { - controller.error(new Error('stream error')) - }) + controller.error(streamError) }, }) controller.respondWith(new Response(stream)) @@ -123,23 +122,109 @@ it('forwards ReadableStream errors to the request', async () => { const request = http.get('http://localhost/resource') request.on('error', requestErrorListener) - request.on('response', (response) => { - response.on('error', responseErrorListener) + + await vi.waitFor(() => { + return new Promise((resolve, reject) => { + request.on('error', () => resolve()) + request.on('response', () => { + reject('Must not emit response') + }) + }) + }) + + expect.soft(request.destroyed).toBe(true) + expect.soft(requestErrorListener).toHaveBeenCalledOnce() + expect.soft(requestErrorListener).toHaveBeenCalledWith( + expect.objectContaining({ + code: 'ECONNRESET', + message: 'socket hang up', + }) + ) + expect.soft(responseErrorListener).not.toHaveBeenCalled() +}) + +it('handles delayed response stream errors as IncomingMessage errors', async () => { + const requestErrorListener = vi.fn() + const responseErrorListener = vi.fn() + + const streamError = new Error('stream error') + interceptor.once('request', ({ controller }) => { + const stream = new ReadableStream({ + async start(controller) { + controller.enqueue(new TextEncoder().encode('original')) + /** + * @note Pause is important here so that Node.js flushes the response headers + * before the stream errors. If the error happens immediately, Node.js will + * optimize for that and translate it into the request error. + */ + await sleep(250) + controller.error(streamError) + }, + }) + controller.respondWith(new Response(stream)) }) + const request = http.get('http://localhost/resource') + request.on('error', requestErrorListener) + const response = await vi.waitFor(() => { - return new Promise((resolve) => { - request.on('response', resolve) + return new Promise((resolve, reject) => { + request.on('error', () => reject('Must not emit request error')) + request.on('response', (response) => { + response.on('close', () => resolve(response)) + response.on('error', responseErrorListener) + }) + }) + }) + + expect.soft(response.statusCode).toBe(200) + expect.soft(response.statusMessage).toBe('OK') + + expect.soft(request.destroyed).toBe(true) + expect.soft(requestErrorListener).not.toHaveBeenCalled() + expect.soft(responseErrorListener).toHaveBeenCalledOnce() + expect.soft(responseErrorListener).toHaveBeenCalledWith( + expect.objectContaining({ + code: 'ECONNRESET', + message: 'aborted', + }) + ) +}) + +it('treats unhandled exceptions during the response stream as request errors', async () => { + const requestErrorListener = vi.fn() + const responseErrorListener = vi.fn() + + interceptor.once('request', ({ controller }) => { + const stream = new ReadableStream({ + async start(controller) { + await sleep(200) + // Intentionally invalid input. + controller.enqueue({}) + }, }) + controller.respondWith(new Response(stream)) }) - // Response stream errors are translated to unhandled exceptions, - // and then the server decides how to handle them. This is often - // done as returning a 500 response. - expect(response.statusCode).toBe(500) - expect(response.statusMessage).toBe('Unhandled Exception') + const request = http.get('http://localhost/resource') + request.on('error', requestErrorListener) + + await vi.waitFor(() => { + return new Promise((resolve, reject) => { + request.on('error', () => resolve()) + request.on('response', (response) => { + reject('Must not emit response') + }) + }) + }) - // Response stream errors are not request errors. - expect(requestErrorListener).not.toHaveBeenCalled() - expect(request.destroyed).toBe(false) + expect.soft(request.destroyed).toBe(true) + expect.soft(requestErrorListener).toHaveBeenCalledOnce() + expect.soft(requestErrorListener).toHaveBeenCalledWith( + expect.objectContaining({ + code: 'ECONNRESET', + message: 'socket hang up', + }) + ) + expect.soft(responseErrorListener).not.toHaveBeenCalled() })