fix: await in-flight requests from closing clients

src/browser/glossary.ts

@@ -62,7 +62,7 @@ export type StartHandler = (
   initialOptions: StartOptions,
 ) => StartReturnType
 
-export type StopHandler = () => void
+export type StopHandler = () => Promise<void> | void
 
 export interface SetupWorker {
   /**

src/browser/setup-worker.ts

@@ -100,15 +100,15 @@ export function setupWorker(...handlers: Array<AnyHandler>): SetupWorker {
         return registration
       }
     },
-    stop() {
+    async stop() {
       if (network.readyState === NetworkReadyState.DISABLED) {
         devUtils.warn(
           `Found a redundant "worker.stop()" call. Notice that stopping the worker after it has already been stopped has no effect. Consider removing this "worker.stop()" call.`,
         )
         return
       }
 
-      network.disable()
+      await network.disable()
       window.postMessage({ type: 'msw/worker:stop' })
     },
     events: network.events,

src/browser/sources/service-worker-source.ts

@@ -159,7 +159,7 @@ export class ServiceWorkerSource extends NetworkSource<ServiceWorkerHttpNetworkF
     return registration
   }
 
-  public disable(): void {
+  public async disable(): Promise<void> {
     /**
      * @note Do NOT call `super.disable()` because it removes any "frame" listeners
      * from this network source, effectively turning it off. The Service Worker source
@@ -180,12 +180,16 @@ export class ServiceWorkerSource extends NetworkSource<ServiceWorkerHttpNetworkF
     this.#listenerController?.abort()
     this.#listenerController = undefined
 
+    const closedPromise = new DeferredPromise<void>()
+    this.#channel.once('CLIENT_CLOSED', () => closedPromise.resolve())
+
     /**
      * @note Tell the Service Worker to drop this client from its active set
      * so it stops forwarding REQUEST events here. `stoppedAt` still guards
      * any requests the SW already forwarded before this message arrived.
      */
-    this.#channel.postMessage('CLIENT_CLOSED')
+    this.#channel.postMessage('CLIENT_CLOSE')
+    await closedPromise
 
     /**
      * @note Do NOT reset `workerPromise` here. The channel must continue to
@@ -285,7 +289,7 @@ Please consider using a custom "serviceWorker.url" option to point to the actual
         }
 
         if (worker.state !== 'redundant') {
-          this.#channel.postMessage('CLIENT_CLOSED')
+          this.#channel.postMessage('CLIENT_CLOSE')
         }
 
         clearInterval(this.#keepAliveInterval)
@@ -316,10 +320,6 @@ Please consider using a custom "serviceWorker.url" option to point to the actual
   }
 
   async #handleRequest(event: WorkerChannelRequestEvent): Promise<void> {
-    if (this.#stoppedAt && event.data.interceptedAt > this.#stoppedAt) {
-      return event.postMessage('PASSTHROUGH')
-    }
-
     const request = deserializeRequest(event.data)
     RequestHandler.cache.set(request, request.clone())
 

src/browser/utils/workerChannel.ts

@@ -13,6 +13,7 @@ export type WorkerChannelEventMap = {
       frameType: string
     }
   }>
+  CLIENT_CLOSED: TypedEvent<never>
   INTEGRITY_CHECK_RESPONSE: WorkerEvent<{
     packageVersion: string
     checksum: string
@@ -41,7 +42,6 @@ export interface IncomingWorkerRequest extends Omit<
    * intercepted by the "fetch" event in the Service Worker.
    */
   id: string
-  interceptedAt: number
   body?: ArrayBuffer | null
 }
 
@@ -114,7 +114,7 @@ type OutgoingWorkerEvents =
   | 'MOCK_ACTIVATE'
   | 'INTEGRITY_CHECK_REQUEST'
   | 'KEEPALIVE_REQUEST'
-  | 'CLIENT_CLOSED'
+  | 'CLIENT_CLOSE'
 
 export interface WorkerChannelOptions {
   getWorker: () => Promise<ServiceWorker>

src/mockServiceWorker.js

@@ -10,7 +10,12 @@
 const PACKAGE_VERSION = '<PACKAGE_VERSION>'
 const INTEGRITY_CHECKSUM = '<INTEGRITY_CHECKSUM>'
 const IS_MOCKED_RESPONSE = Symbol('isMockedResponse')
+
 const activeClientIds = new Set()
+/**
+ * @type {Map<string, Set<Promise<Response>>>}
+ */
+const pendingRequests = new Map()
 
 addEventListener('install', function () {
   self.skipWaiting()
@@ -20,75 +25,101 @@ addEventListener('activate', function (event) {
   event.waitUntil(self.clients.claim())
 })
 
-addEventListener('message', async function (event) {
+addEventListener('message', function (event) {
   const clientId = Reflect.get(event.source || {}, 'id')
 
   if (!clientId || !self.clients) {
     return
   }
 
-  if (event.data === 'CLIENT_CLOSED') {
-    const allClients = await self.clients.matchAll({
-      type: 'window',
-    })
-
-    activeClientIds.delete(clientId)
-
-    const remainingClients = allClients.filter((client) => {
-      return client.id !== clientId
-    })
-
-    // Unregister itself when there are no more clients
-    if (remainingClients.length === 0) {
-      self.registration.unregister()
-    }
-  }
-
-  const client = await self.clients.get(clientId)
-
-  if (!client) {
-    return
-  }
+  event.waitUntil(
+    (async () => {
+      if (event.data === 'CLIENT_CLOSE') {
+        const allClients = await self.clients.matchAll({
+          type: 'window',
+        })
+
+        activeClientIds.delete(clientId)
+
+        // Await any pending requests from the closing client.
+        // This makes sure that those requests are handled and not passthrough.
+        const pending = pendingRequests.get(clientId)
+        if (pending != null && pending.size > 0) {
+          await Promise.allSettled(pending)
+        }
+        pendingRequests.delete(clientId)
+
+        const remainingClients = allClients.filter((client) => {
+          return client.id !== clientId
+        })
+
+        // Unregister itself when there are no more clients
+        if (remainingClients.length === 0) {
+          await self.registration.unregister()
+        }
+
+        const client = await self.clients.get(clientId)
+
+        if (client != null) {
+          await sendToClient(client, {
+            type: 'CLIENT_CLOSED',
+          })
+        }
+
+        return
+      }
 
-  switch (event.data) {
-    case 'KEEPALIVE_REQUEST': {
-      sendToClient(client, {
-        type: 'KEEPALIVE_RESPONSE',
-      })
-      break
-    }
+      /**
+       * @note Check for the client AFTER handling "CLIENT_CLOSE".
+       * This prevents early return on "!client" in case the page has reloaded
+       * and disassociated itself from the worker. This ensures self-unregistration
+       * still fires for those pages.
+       */
+      const client = await self.clients.get(clientId)
 
-    case 'INTEGRITY_CHECK_REQUEST': {
-      sendToClient(client, {
-        type: 'INTEGRITY_CHECK_RESPONSE',
-        payload: {
-          packageVersion: PACKAGE_VERSION,
-          checksum: INTEGRITY_CHECKSUM,
-        },
-      })
-      break
-    }
-
-    case 'MOCK_ACTIVATE': {
-      activeClientIds.add(clientId)
+      if (!client) {
+        return
+      }
 
-      sendToClient(client, {
-        type: 'MOCKING_ENABLED',
-        payload: {
-          client: {
-            id: client.id,
-            frameType: client.frameType,
-          },
-        },
-      })
-      break
-    }
-  }
+      switch (event.data) {
+        case 'KEEPALIVE_REQUEST': {
+          await sendToClient(client, {
+            type: 'KEEPALIVE_RESPONSE',
+          })
+          break
+        }
+
+        case 'INTEGRITY_CHECK_REQUEST': {
+          await sendToClient(client, {
+            type: 'INTEGRITY_CHECK_RESPONSE',
+            payload: {
+              packageVersion: PACKAGE_VERSION,
+              checksum: INTEGRITY_CHECKSUM,
+            },
+          })
+          break
+        }
+
+        case 'MOCK_ACTIVATE': {
+          activeClientIds.add(clientId)
+
+          await sendToClient(client, {
+            type: 'MOCKING_ENABLED',
+            payload: {
+              client: {
+                id: client.id,
+                frameType: client.frameType,
+              },
+            },
+          })
+          break
+        }
+      }
+    })(),
+  )
 })
 
 addEventListener('fetch', function (event) {
-  const requestInterceptedAt = Date.now()
-
   // Opening the DevTools triggers the "only-if-cached" request
   // that cannot be handled by the worker. Bypass such requests.
   if (
@@ -106,23 +137,33 @@ addEventListener('fetch', function (event) {
   }
 
   const requestId = crypto.randomUUID()
-  event.respondWith(handleRequest(event, requestId, requestInterceptedAt))
+  event.respondWith(handleRequest(event, requestId))
 })
 
 /**
  * @param {FetchEvent} event
  * @param {string} requestId
- * @param {number} requestInterceptedAt
  */
-async function handleRequest(event, requestId, requestInterceptedAt) {
+async function handleRequest(event, requestId) {
   const client = await resolveMainClient(event)
   const requestCloneForEvents = event.request.clone()
-  const response = await getResponse(
-    event,
-    client,
-    requestId,
-    requestInterceptedAt,
-  )
+
+  const responsePromise = getResponse(event, client, requestId)
+
+  if (client != null) {
+    let pending = pendingRequests.get(client.id)
+
+    if (pending == null) {
+      pendingRequests.set(client.id, (pending = new Set()))
+    }
+
+    pending.add(responsePromise)
+    responsePromise
+      .finally(() => pending.delete(responsePromise))
+      .catch(() => {})
+  }
+
+  const response = await responsePromise
 
   // Send back the response clone for the "response:*" life-cycle events.
   // Ensure MSW is active and ready to handle the message, otherwise
@@ -198,10 +239,9 @@ async function resolveMainClient(event) {
  * @param {FetchEvent} event
  * @param {Client | undefined} client
  * @param {string} requestId
- * @param {number} requestInterceptedAt
  * @returns {Promise<Response>}
  */
-async function getResponse(event, client, requestId, requestInterceptedAt) {
+async function getResponse(event, client, requestId) {
   // Clone the request because it might've been already used
   // (i.e. its body has been read and sent to the client).
   const requestClone = event.request.clone()
@@ -252,7 +292,6 @@ async function getResponse(event, client, requestId, requestInterceptedAt) {
       type: 'REQUEST',
       payload: {
         id: requestId,
-        interceptedAt: requestInterceptedAt,
         ...serializedRequest,
       },
     },

test/browser/msw-api/setup-worker/start/handlers-reset.test.ts

@@ -24,7 +24,7 @@ test('does not accumulate request handlers across restarts', async ({
 
   // Stop and restart the worker.
   await page.evaluate(async () => {
-    window.msw.worker.stop()
+    await window.msw.worker.stop()
     await window.msw.worker.start({ quiet: true })
   })
 
@@ -35,7 +35,7 @@ test('does not accumulate request handlers across restarts', async ({
 
   // Stop and restart the worker again.
   await page.evaluate(async () => {
-    window.msw.worker.stop()
+    await window.msw.worker.stop()
     await window.msw.worker.start({ quiet: true })
   })
 

test/browser/msw-api/setup-worker/start/start-after-stop.test.ts

@@ -20,8 +20,8 @@ test('handles requests after starting a stopped worker', async ({
   const consoleSpy = spyOnConsole()
   await loadExample(new URL('./start-after-stop.mocks.ts', import.meta.url))
 
-  await page.evaluate(() => {
-    window.msw.worker.stop()
+  await page.evaluate(async () => {
+    await window.msw.worker.stop()
   })
 
   expect(consoleSpy.get('log')).toEqual(

test/browser/msw-api/setup-worker/stop/in-flight-request.test.ts

@@ -1,9 +1,9 @@
-import type { SetupWorkerApi } from '../../../../../src/browser'
+import type { SetupWorker } from '../../../../../src/browser'
 import { test, expect } from '../../../playwright.extend'
 
 declare namespace window {
   export const msw: {
-    worker: SetupWorkerApi
+    worker: SetupWorker
   }
 }
 
@@ -22,8 +22,8 @@ test('handles an in-flight request performed before the worker was stopped', asy
     return response.text()
   })
 
-  await page.evaluate(() => {
-    window.msw.worker.stop()
+  await page.evaluate(async () => {
+    await window.msw.worker.stop()
   })
 
   await expect(dataPromise).resolves.toBe('hello world')
@@ -49,8 +49,8 @@ test('bypasses requests made after the worker was stopped', async ({
 
   const resourceUrl = new URL('./resource', compilation.previewUrl)
 
-  await page.evaluate(() => {
-    window.msw.worker.stop()
+  await page.evaluate(async () => {
+    await window.msw.worker.stop()
   })
 
   const dataPromise = page.evaluate(async (url) => {