fix: ignore irrelevant request listeners during response patching (#253)

* fix(ClientRequest): await idle of relevant requests

* fix(fetch): await idle of relevant requests

* fix(xhr): await idle of relevant requests

* chore(helpers): reject on missing "resolver" event dispatch

* chore: set browser tests timeout to 15s

* fix(RemoteHttpInterceptor): await idle of relevant requests

* chore: remove logs from the FetchInterceptor

src/BatchInterceptor.test.ts

@@ -80,13 +80,13 @@ it('proxies event listeners to the interceptors', () => {
 })
 
 it('disposes of child interceptors', async () => {
-  class PrimaryInterceptor extends Interceptor<never> {
+  class PrimaryInterceptor extends Interceptor<any> {
     constructor() {
       super(Symbol('primary'))
     }
   }
 
-  class SecondaryInterceptor extends Interceptor<never> {
+  class SecondaryInterceptor extends Interceptor<any> {
     constructor() {
       super(Symbol('secondary'))
     }

src/RemoteHttpInterceptor.ts

@@ -131,7 +131,9 @@ export class RemoteHttpResolver extends Interceptor<HttpRequestEventMap> {
       }
 
       this.emitter.emit('request', interactiveIsomorphicRequest)
-      await this.emitter.untilIdle('request')
+      await this.emitter.untilIdle('request', ({ args: [request] }) => {
+        return request.id === interactiveIsomorphicRequest.id
+      })
       const [mockedResponse] =
         await interactiveIsomorphicRequest.respondWith.invoked()
 

src/interceptors/ClientRequest/NodeClientRequest.ts

@@ -159,8 +159,15 @@ 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 () => {
-      await this.emitter.untilIdle('request')
-      this.log('all request listeners have been resolved!')
+      await this.emitter.untilIdle('request', ({ args: [request] }) => {
+        /**
+         * @note Await only those listeners that are relevant to this request.
+         * This prevents extraneous parallel request from blocking the resolution
+         * of another, unrelated request. For example, during response patching,
+         * when request resolution is nested.
+         */
+        return request.id === interactiveIsomorphicRequest.id
+      })
 
       const [mockedResponse] =
         await interactiveIsomorphicRequest.respondWith.invoked()

src/interceptors/XMLHttpRequest/XMLHttpRequestOverride.ts

@@ -272,7 +272,9 @@ export const createXMLHttpRequestOverride = (
 
       Promise.resolve(
         until(async () => {
-          await emitter.untilIdle('request')
+          await emitter.untilIdle('request', ({ args: [request] }) => {
+            return request.id === interactiveIsomorphicRequest.id
+          })
           this.log('all request listeners have been resolved!')
 
           const [mockedResponse] =

src/interceptors/fetch/index.ts

@@ -33,6 +33,7 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
 
     globalThis.fetch = async (input, init) => {
       const request = new Request(input, init)
+
       const url = typeof input === 'string' ? input : input.url
       const method = request.method
 
@@ -58,7 +59,9 @@ export class FetchInterceptor extends Interceptor<HttpRequestEventMap> {
 
       this.log('awaiting for the mocked response...')
 
-      await this.emitter.untilIdle('request')
+      await this.emitter.untilIdle('request', ({ args: [request] }) => {
+        return request.id === isomorphicRequest.id
+      })
       this.log('all request listeners have been resolved!')
 
       const [mockedResponse] = await isomorphicRequest.respondWith.invoked()

src/utils/AsyncEventEmitter.test.ts

@@ -1,6 +1,10 @@
 import { AsyncEventEmitter } from './AsyncEventEmitter'
 import { sleep } from '../../test/helpers'
 
+afterEach(() => {
+  jest.useRealTimers()
+})
+
 it('emits and listens to events', () => {
   const emitter = new AsyncEventEmitter<{ hello(name: string): void }>()
   const listener = jest.fn()
@@ -35,6 +39,35 @@ it('resolves "untilIdle" when all the event listeners are done', async () => {
   expect(results).toEqual(['first', 'second'])
 })
 
+it('resolves "untilIdle" only for the relevant listeners', async () => {
+  const emitter = new AsyncEventEmitter<{ signal(code: number): void }>()
+
+  const results: number[] = []
+  const listener = jest.fn(async (code: number) => {
+    if (code !== 1) {
+      // Delay listener based on the signal code.
+      await sleep(150)
+    }
+
+    results.push(code)
+  })
+  emitter.on('signal', listener)
+
+  emitter.emit('signal', 1)
+  emitter.emit('signal', 2)
+
+  const resultsAfterIdle = await emitter
+    .untilIdle('signal', ({ args: [code] }) => {
+      return code === 1
+    })
+    .then(() => results)
+
+  await emitter.untilIdle('signal')
+
+  expect(listener).toHaveBeenCalled()
+  expect(resultsAfterIdle).toEqual([1])
+})
+
 it('resolves "untilIdle" immediately if there are no pending listeners', async () => {
   const emitter = new AsyncEventEmitter<{ ping(): void }>()
   emitter.emit('ping')

src/utils/AsyncEventEmitter.ts

@@ -2,7 +2,10 @@ import { Debugger, debug } from 'debug'
 import { StrictEventEmitter, EventMapType } from 'strict-event-emitter'
 import { nextTick } from './nextTick'
 
-export type QueueItem = Promise<void>
+export interface QueueItem<Args extends any[]> {
+  args: Args
+  done: Promise<void>
+}
 
 export enum AsyncEventEmitterReadyState {
   ACTIVE = 'ACTIVE',
@@ -15,7 +18,10 @@ export class AsyncEventEmitter<
   public readyState: AsyncEventEmitterReadyState
 
   private log: Debugger
-  protected queue: Map<keyof EventMap, QueueItem[]>
+  protected queue: Map<
+    keyof EventMap,
+    QueueItem<Parameters<EventMap[keyof EventMap]>>[]
+  >
 
   constructor() {
     super()
@@ -39,16 +45,17 @@ export class AsyncEventEmitter<
       return this
     }
 
-    return super.on(event, (async (...args: unknown[]) => {
+    return super.on(event, (async (...args: Parameters<EventMap[Event]>) => {
       // Event queue is always established when calling ".emit()".
       const queue = this.openListenerQueue(event)
 
       log('awaiting the "%s" listener...', event)
 
       // Whenever a listener is called, create a new Promise
       // that resolves when that listener function completes its execution.
-      queue.push(
-        new Promise<void>(async (resolve, reject) => {
+      queue.push({
+        args,
+        done: new Promise<void>(async (resolve, reject) => {
           try {
             // Treat listeners as potentially asynchronous functions
             // so they could be awaited.
@@ -60,8 +67,8 @@ export class AsyncEventEmitter<
             log('"%s" listener has rejected!', error)
             reject(error)
           }
-        })
-      )
+        }),
+      })
     }) as EventMap[Event])
   }
 
@@ -103,10 +110,15 @@ export class AsyncEventEmitter<
    * If the event has no listeners, resolves immediately.
    */
   public async untilIdle<Event extends keyof EventMap>(
-    event: Event
+    event: Event,
+    filter: (item: QueueItem<Parameters<EventMap[Event]>>) => boolean = () =>
+      true
   ): Promise<void> {
     const listenersQueue = this.queue.get(event) || []
-    await Promise.all(listenersQueue).finally(() => {
+
+    await Promise.all(
+      listenersQueue.filter(filter).map(({ done }) => done)
+    ).finally(() => {
       // Clear the queue one the promise settles
       // so that different events don't share the same queue.
       this.queue.delete(event)
@@ -115,7 +127,7 @@ export class AsyncEventEmitter<
 
   private openListenerQueue<Event extends keyof EventMap>(
     event: Event
-  ): QueueItem[] {
+  ): QueueItem<Parameters<EventMap[Event]>>[] {
     const log = this.log.extend('openListenerQueue')
 
     log('opening "%s" listeners queue...', event)

test/helpers.ts

@@ -205,10 +205,19 @@ export async function extractRequestFromPage(
   page: Page
 ): Promise<IsomorphicRequest> {
   const request = await page.evaluate(() => {
-    return new Promise<StringifiedIsomorphicRequest>((resolve) => {
+    return new Promise<StringifiedIsomorphicRequest>((resolve, reject) => {
+      const timeoutTimer = setTimeout(() => {
+        reject(
+          new Error(
+            'Browser runtime module did not dispatch the custom "resolver" event'
+          )
+        )
+      }, 5000)
+
       window.addEventListener(
         'resolver' as any,
         (event: CustomEvent<string>) => {
+          clearTimeout(timeoutTimer)
           resolve(JSON.parse(event.detail))
         }
       )

test/jest.browser.config.js

@@ -1,5 +1,5 @@
 module.exports = {
-  testTimeout: 60000,
+  testTimeout: 15000,
   testMatch: ['**/*.browser.test.ts'],
   setupFilesAfterEnv: ['./jest.browser.setup.ts'],
   transform: {

test/modules/XMLHttpRequest/response/xhr-response-patching.browser.runtime.js

@@ -0,0 +1,43 @@
+import { XMLHttpRequestInterceptor } from '@mswjs/interceptors/lib/interceptors/XMLHttpRequest'
+
+const interceptor = new XMLHttpRequestInterceptor()
+
+interceptor.on('request', async (request) => {
+  window.dispatchEvent(
+    new CustomEvent('resolver', {
+      detail: JSON.stringify({
+        id: request.id,
+        method: request.method,
+        url: request.url.href,
+        headers: request.headers.all(),
+        credentials: request.credentials,
+        body: request.body,
+      }),
+    })
+  )
+
+  if (request.url.pathname === '/mocked') {
+    await new Promise((resolve) => setTimeout(resolve, 0))
+
+    const req = new XMLHttpRequest()
+    req.open('GET', window.originalUrl, true)
+    req.send()
+    await new Promise((resolve, reject) => {
+      req.addEventListener('loadend', resolve)
+      req.addEventListener('error', reject)
+    })
+
+    request.respondWith({
+      status: req.status,
+      statusText: req.statusText,
+      headers: {
+        'X-Custom-Header': req.getResponseHeader('X-Custom-Header'),
+      },
+      body: `${req.responseText} world`,
+    })
+  }
+})
+
+interceptor.apply()
+
+window.interceptor = interceptor

test/modules/XMLHttpRequest/response/xhr-response-patching.browser.test.ts

@@ -0,0 +1,58 @@
+/**
+ * @jest-environment node
+ */
+import * as path from 'path'
+import { pageWith } from 'page-with'
+import { HttpServer } from '@open-draft/test-server/http'
+import { createBrowserXMLHttpRequest } from '../../../helpers'
+
+declare namespace window {
+  export let originalUrl: string
+}
+
+const httpServer = new HttpServer((app) => {
+  app.get('/original', (req, res) => {
+    res
+      .set('access-control-expose-headers', 'x-custom-header')
+      .set('x-custom-header', 'yes')
+      .send('hello')
+  })
+})
+
+async function prepareRuntime() {
+  const runtime = await pageWith({
+    example: path.resolve(
+      __dirname,
+      'xhr-response-patching.browser.runtime.js'
+    ),
+  })
+
+  await runtime.page.evaluate((url) => {
+    window.originalUrl = url
+  }, httpServer.http.url('/original'))
+
+  return runtime
+}
+
+beforeAll(async () => {
+  await httpServer.listen()
+})
+
+afterAll(async () => {
+  await httpServer.close()
+})
+
+test('responds to an HTTP request handled in the resolver', async () => {
+  const runtime = await prepareRuntime()
+  const callXMLHttpRequest = createBrowserXMLHttpRequest(runtime)
+
+  const [, response] = await callXMLHttpRequest({
+    method: 'GET',
+    url: 'http://localhost/mocked',
+  })
+
+  expect(response.status).toBe(200)
+  expect(response.statusText).toBe('OK')
+  expect(response.headers).toBe('x-custom-header: yes')
+  expect(response.body).toBe('hello world')
+})