fix: consistent exception handling

mswjs · Apr 17, 2024 · c46fd4d · c46fd4d
1 parent 26ff83c
commit c46fd4d
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 107 deletions.
diff --git a/src/interceptors/ClientRequest/NodeClientRequest.ts b/src/interceptors/ClientRequest/NodeClientRequest.ts
@@ -23,6 +23,7 @@ import { isPropertyAccessible } from '../../utils/isPropertyAccessible'
 import { isNodeLikeError } from '../../utils/isNodeLikeError'
 import { INTERNAL_REQUEST_ID_HEADER_NAME } from '../../Interceptor'
 import { createRequestId } from '../../createRequestId'
+import { createServerErrorResponse } from '../../utils/responseUtils'
 
 export type Protocol = 'http' | 'https'
 
@@ -220,7 +221,7 @@ export class NodeClientRequest extends ClientRequest {
 
     // Execute the resolver Promise like a side-effect.
     // Node.js 16 forces "ClientRequest.end" to be synchronous and return "this".
-    until(async () => {
+    until<unknown, Response | undefined>(async () => {
       // Notify the interceptor about the request.
       // This will call any "request" listeners the users have.
       this.logger.info(
@@ -266,6 +267,7 @@ export class NodeClientRequest extends ClientRequest {
           resolverResult.error
         )
 
+        // Treat thrown Responses as mocked responses.
         if (resolverResult.error instanceof Response) {
           this.respondWith(resolverResult.error)
           return
@@ -278,29 +280,11 @@ export class NodeClientRequest extends ClientRequest {
           return this
         }
 
-        if (resolverResult.error instanceof Error) {
-          // Coerce unhandled exceptions in the "request" listeners
-          // as 500 responses.
-          this.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',
-                },
-              }
-            )
-          )
-          return this
-        }
+        // Unhandled exceptions in the request listeners are
+        // synonymous to unhandled exceptions on the server.
+        // Those are represented as 500 error responses.
+        this.respondWith(createServerErrorResponse(resolverResult.error))
 
-        this.errorWith(resolverResult.error)
         return this
       }
 

diff --git a/src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts b/src/interceptors/XMLHttpRequest/XMLHttpRequestProxy.ts
@@ -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
@@ -94,35 +95,18 @@ export function createXMLHttpRequestProxy({
             resolverResult.error
           )
 
+          // 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(
+            createServerErrorResponse(resolverResult.error)
+          )
 
-          if (resolverResult.error instanceof Error) {
-            // Treat unhandled exceptions in the "request" listener
-            // as 500 server errors.
-            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',
-                  },
-                }
-              )
-            )
-            return
-          }
-
-          // Otherwise, error the request with the thrown data.
-          xhrRequestController.errorWith(resolverResult.error)
           return
         }
 

diff --git a/src/interceptors/fetch/index.ts b/src/interceptors/fetch/index.ts
@@ -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')
@@ -109,28 +110,30 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
         return response
       }
 
-      const resolverResult = await until(async () => {
-        const listenersFinished = emitAsync(this.emitter, 'request', {
-          request: interactiveRequest,
-          requestId,
-        })
+      const resolverResult = await until<unknown, Response | undefined>(
+        async () => {
+          const listenersFinished = emitAsync(this.emitter, 'request', {
+            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,
-        ])
+          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,
+          ])
 
-        this.logger.info('all request listeners have been resolved!')
+          this.logger.info('all request listeners have been resolved!')
 
-        const mockedResponse = await requestController.responsePromise
-        this.logger.info('event.respondWith called with:', mockedResponse)
+          const mockedResponse = await requestController.responsePromise
+          this.logger.info('event.respondWith called with:', mockedResponse)
 
-        return mockedResponse
-      })
+          return mockedResponse
+        }
+      )
 
       if (requestAborted.state === 'rejected') {
         return Promise.reject(requestAborted.rejectionReason)
@@ -142,27 +145,10 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
           return respondWith(resolverResult.error)
         }
 
-        if (resolverResult.error instanceof 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',
-              },
-            }
-          )
-        }
-
-        return Promise.reject(createNetworkError(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

diff --git a/src/utils/responseUtils.ts b/src/utils/responseUtils.ts
@@ -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',
+      },
+    }
+  )
+}
diff --git a/test/modules/XMLHttpRequest/compliance/xhr-middleware-exception.test.ts b/test/modules/XMLHttpRequest/compliance/xhr-middleware-exception.test.ts
@@ -78,9 +78,9 @@ it('treats a thrown Response instance as a mocked response', async () => {
   expect(request.responseText).toBe('hello world')
 })
 
-it('forwards thrown non-errors as request errors', async () => {
-  interceptor.on('request', () => {
-    throw 123
+it('treats Response.error() as a network error', async () => {
+  interceptor.on('request', ({ request }) => {
+    request.respondWith(Response.error())
   })
 
   const requestErrorListener = vi.fn()

diff --git a/test/modules/fetch/fetch-exception.test.ts b/test/modules/fetch/fetch-exception.test.ts
@@ -46,9 +46,9 @@ it('treats a thrown Response as a mocked response', async () => {
   expect(await response.text()).toBe('hello world')
 })
 
-it('treats thrown non-errors a network error', async () => {
-  interceptor.on('request', () => {
-    throw 123
+it('treats Response.error() as a network error', async () => {
+  interceptor.on('request', ({ request }) => {
+    request.respondWith(Response.error())
   })
 
   const requestError = await fetch('http://localhost:3001/resource')
@@ -59,5 +59,5 @@ it('treats thrown non-errors a network error', async () => {
 
   expect(requestError.name).toBe('TypeError')
   expect(requestError.message).toBe('Failed to fetch')
-  expect(requestError.cause).toBe(123)
+  expect(requestError.cause).toBeInstanceOf(Response)
 })
diff --git a/test/modules/http/compliance/http-unhandled-exception.test.ts b/test/modules/http/compliance/http-unhandled-exception.test.ts
@@ -50,17 +50,3 @@ it('treats unhandled interceptor errors as 500 responses', async () => {
     stack: expect.any(String),
   })
 })
-
-it('treats thrown non-errors as request errors', async () => {
-  interceptor.on('request', () => {
-    throw 123
-  })
-
-  const request = http.get('http://localhost/resource')
-  const requestErrorPromise = new DeferredPromise<Error>()
-  request.on('error', (error) => requestErrorPromise.resolve(error))
-
-  const requestError = await requestErrorPromise
-
-  expect(requestError).toBe(123)
-})