Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
13 changes: 12 additions & 1 deletion src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
RequestHandler,
WebSocketHandler,
getResponse,
isCommonAssetRequest,
} from 'msw'
import {
type WebSocketClientEventMap,
Expand All @@ -24,7 +25,8 @@ import {
} from '@mswjs/interceptors/WebSocket'

export interface CreateNetworkFixtureArgs {
initialHandlers: Array<RequestHandler | WebSocketHandler>
ignoreCommonAssetRequests?: boolean
initialHandlers?: Array<RequestHandler | WebSocketHandler>
}

/**
Expand Down Expand Up @@ -57,6 +59,7 @@ export function createNetworkFixture(
async ({ page }, use) => {
const worker = new NetworkFixture({
page,
ignoreCommonAssetRequests: args?.ignoreCommonAssetRequests ?? true,
initialHandlers: args?.initialHandlers || [],
})

Expand All @@ -70,13 +73,16 @@ export function createNetworkFixture(

export class NetworkFixture extends SetupApi<LifeCycleEventsMap> {
#page: Page
#ignoreCommonAssetRequests: boolean

constructor(args: {
page: Page
ignoreCommonAssetRequests: boolean
initialHandlers: Array<RequestHandler | WebSocketHandler>
}) {
super(...args.initialHandlers)
this.#page = args.page
this.#ignoreCommonAssetRequests = args.ignoreCommonAssetRequests
}

public async start() {
Expand All @@ -88,6 +94,11 @@ export class NetworkFixture extends SetupApi<LifeCycleEventsMap> {
body: request.postDataBuffer(),
})

if (this.#ignoreCommonAssetRequests && isCommonAssetRequest(fetchRequest)) {
Copy link
Member

Choose a reason for hiding this comment

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

There's one thing I still don't quite understand. By ignoring requests here we are claiming that the performance degradation originates from the getResponse() logic below. But that is highly unlikely (unless you have millions of handlers, which isn't the case).

I believe we need to profile this problem better before merging a solution.

  1. Is page.route(/.+/, () => route.continue()) slow for the same scenario?
  2. Is there anything in your scenario that suggests getResponse() becoming the root cause?

Copy link
Author

Choose a reason for hiding this comment

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

Is page.route(/.+/, () => route.continue()) slow for the same scenario?

I think this would be slower at scale (e.g. thousands of requests) due to unnecessary overhead, but not meaningful for fewer total requests.

Is there anything in your scenario that suggests getResponse() becoming the root cause?

getResponse seems to be a contributing factor, but is not necessarily the only cause. I think this really is a scaling problem, on the order of # of requests X # of handlers X request overhead. If a test is making lots of requests (in my case thousands of vite requests), a few ms of overhead for each of these adds up quickly.

I did some not-very-scientific profiling, and here are the results I am seeing:

With ignoreCommonAssetRequests === false:

Setup (http://localhost:5173/) took 6.999666999999931ms
Setup (http://localhost:5173/@vite/client) took 4.065000000000055ms
Setup (http://localhost:5173/node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/vite/dist/client/env.mjs) took 2.3236670000000004ms

With ignoreCommonAssetRequests === true:

Setup (http://localhost:5173/) took 4.779042000000004ms
Setup (http://localhost:5173/@vite/client) took 0.9514169999999922ms
Setup (http://localhost:5173/node_modules/.pnpm/[email protected]_@[email protected][email protected]/node_modules/vite/dist/client/env.mjs) took 0.771541999999954ms

Setup here represents the cost of executing the code before route.continue() is eventually called. This was done with 25 placeholder MSW handlers.

My conclusions/suspicions are as follows:

  • More handlers === more potential overhead for each request
  • Intercepted, but then continued requests are the most expensive
  • Something about vite request matching is expensive...maybe the URLs are expensive to match in the underlying getResponses code?
  • Hundreds to thousands of these requests kicking off at once might be a source of resource contention

route.continue()
return
}

const response = await getResponse(
this.handlersController.currentHandlers().filter((handler) => {
return handler instanceof RequestHandler
Expand Down
40 changes: 39 additions & 1 deletion tests/requests.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { test as testBase, expect } from '@playwright/test'
import { http } from 'msw'
import { http, HttpResponse } from 'msw'
import { createNetworkFixture, type NetworkFixture } from '../src/index.js'

interface Fixtures {
Expand Down Expand Up @@ -107,3 +107,41 @@ test('intercepts a POST request with array buffer body', async ({
expect(request.url).toBe('http://localhost:5173/action')
await expect(request.text()).resolves.toBe('hello world')
})

test('intercepts an asset request when interceptCommonAssetRequests is true', async ({ network, page }) => {
network.use(
http.get('/index.html', () => {
return HttpResponse.text('NOT VALID HTML')
}),
)

await page.goto('/')
const htmlContent = await page.evaluate(async () => {
const res = await fetch('/index.html')
return res.text()
})

expect(htmlContent).toContain('DOCTYPE html')
})

const testMockAssets = testBase.extend<Fixtures>({
network: createNetworkFixture({ ignoreCommonAssetRequests: false }),
})

testMockAssets('passes through an asset request when interceptCommonAssetRequests is false', async ({ network, page }) => {
network.use(
http.get('/index.html', () => {
return HttpResponse.text('NOT VALID HTML')
}),
)

await page.goto('/')
const htmlContent = await page.evaluate(async () => {
const res = await fetch('/index.html')
return res.text()
})

expect(htmlContent).toContain('NOT VALID HTML')
})