Support concurrent runs in Node.js

[kettanaito]

What

I suggest for the setupServer (node-request-interceptor) API to support execution in a concurrent mode (i.e. multiple test suite at the same time).

Why

Concurrency is crucial in large projects, allowing for faster execution of tests.

Current behavior

Currently node-request-interceptor patches native request issuing modules per process, which allows irrelevant mocking rules to leak to another test suites, as they all run in a single process.

How

Look into how nock operates. It performs modules patching in a similar fashion, and I'd suspect it to support concurrent mode. We can learn how they did it and evaluate if we can adopt a similar solution.

[marcosvega91]

concurrency problem should happen only with multiple instances of setupServer. Under the hood when setupServer is called http, https and XMLHttpRequest are patched to start intercepting requests.
So if I use a single instance across my application I'll not encourage any problem.

If I use multiple instances of setupServer could be possible that a test restores the patched modules before other tests, causing the issue.

It's very strange that nock is not having that problem, their restore function is very simple and don't have any kind of check 🤔

[kettanaito]

Thank you for the insights, @marcosvega91.

I suppose we can dedupe modules patching in node-request-interceptor, but that won't solve the issue of unrelated modules (i.e. test suite) being affected by the interceptors declared in other modules, as they still share the same process.

What we can investigate is how exactly modern test frameworks execute tests in parallel. For instance, do they provision forked processes? If so, perhaps we could bound request interception to the forked process instead of the parent process.

[marcosvega91]

I have tried to investigate today a little bit. it seems that nock is not having this problem simply resetting modules between tests. It is strange because patching native modules are shared between tests.
I have tried also to run jest printing memory usage and it seems that we have some memory leak in node-request-interceptor.

[kettanaito]

@marcosvega91, thanks for investigating that! I believe that the memory leaks were fixed in mswjs/interceptors#77. Regarding the patched modules reset, it's indeed peculiar, as MSW resets those patched modules in a similar fashion to nock.

[tigerabrodi]

@kettanaito @marcosvega91 Seems like we need to deeper investigate, why we have a memory leak in node-request-interceptor, even though we are resetting those patched modules in a similar fashion to nock?

When it comes to the code, where do you think I can perhaps start by in order to deeper investigate it? @marcosvega91 How can I reproduce this memory leak issue in our current Repo with the tests we have?

Also, you mentioned we do reset those patched modules in a similar fashion to nock, where in the code are we doing that, I am looking for some sort of direction.

Thanks, haha, I love working with you guys 😃 🥰 🤗

[kettanaito]

@tigerabrodi, thanks for the interest in this task. I'll try to share some details below.

Restoring patches

Each interceptor in NRI a function that, when called, patches a respective request issuing module to intercept requests, and returns a function to restore that patch. Below you can find the restoration points for two current interceptors: ClientRequest and `XMLHttpRequest.

I don't think that the memory leak Marco's mentioned is related to modules restoration. Most likely they were fixed in mswjs/interceptors#77, but it would still be nice to double-check if there are any other leaks.

ClientRequest

https://github.com/mswjs/node-request-interceptor/blob/62f2a406ea24fdc35e660aa6f888c0ff99d979d5/src/interceptors/ClientRequest/index.ts#L106-L115

XMLHttpRequest

https://github.com/mswjs/node-request-interceptor/blob/62f2a406ea24fdc35e660aa6f888c0ff99d979d5/src/interceptors/XMLHttpRequest/index.ts#L29-L35

The best way to investigate this issue is to reproduce it. Try to set up a few tests with Jest, let all of them use their own as well as shared request handlers, and run them concurrently (Jest does that by default). Once unrelated tests start respecting request handlers other than those declared in them, you got the reproduction scenario.

Technically, this happens because modules patching is per entire process, so when multiple tests run a single process they all start respecting all the patches, leaking unrelated handlers to unrelated tests.

[tigerabrodi]

@kettanaito @marcosvega91 I do not know I will ever get to this issue, am personally fully up with the OSS Raid Group aside from my side project + workshops at FrontendMasters.

[fnmunhoz]

Hi there! That's my first time here in the MSW repo, so first of all, congrats to everyone involved; the project is fantastic. ⭐

I've started to learn more about the library's internal and come across this issue, so I'm trying to understand if I should run my Jest tests using the --runInBand config option in Jest.

Considering what I read here in this issue and my current understanding of Jest's parallelization strategy, I would expect it to be OK to run multiple Jest test suites in parallel.

In practical terms, I would expect it to be acceptable to use --maxWorkers=N where N > 1 instead of --runInBand if I create a single setupServer instance per test suite (node process), which I do.

Wrapping it up, if I create a single setupServer instance per Jest test suite (node process) via beforeAll/afterAll Jest's hooks, each process will patch the native request issuing modules without affecting other parallel processes.

I would appreciate it if you could help me to clarify this. Thanks!

[kettanaito]

Hi, @fnmunhoz. Thank you for the feedback. I'll try to clarify things for you below.

Despite the parallelization in Jest, it still only spawns 1 process. The parallelization may happen via threads/child processes to achieve the execution of multiple test suites simultaneously. The request interception functionality in NodeJS for MSW patches the request issuing modules per process, so regardless of the parallelization it will affect unrelated tests, as they all run in a single process.

We could use some help in researching how exactly Jest implements parallelization. It won't give us a solution, but it may hint us as to the right direction. I believe the solution lies in handling parallel runs in node-request-interceptor (the library MSW uses for NodeJS requests interception).

[fnmunhoz]

Hey @kettanaito thanks for the update!

I'm trying to simulate this problem in Jest, but until now I'm not able to make tests from different Test suites affect each other.

I'll share what I have tried, as you might spot what I might be missing.

I've created this repo with two identical test suites except for L9 where the endpoint changes, so we can identify it in the logs:

fnmunhoz/msw-concurrent-execution@67f7cf0

The Jest output for that is as in the screenshot:

By its logs, I couldn't see any issues, since each Test suite seems to respect its own HTTP handlers.

Do you think the way these test suites are defined should demonstrate the concurrency issue?

If not, do you have any suggestions on what I could try?

I'm in hope that since Jest executes each Test suite in a separate process, it might not be an issue if we use MSW with Jest as the test runner?

[JCB-K]

@kettanaito

Despite the parallelization in Jest, it still only spawns 1 process. The parallelization may happen via threads/child processes to achieve the execution of multiple test suites simultaneously. The request interception functionality in NodeJS for MSW patches the request issuing modules per process, so regardless of the parallelization it will affect unrelated tests, as they all run in a single process.

Does this mean that I should run tests in band (jest --runInBand) if I have different request handlers per test? I've been battling randomly failing tests in CI for a few months now, and wondering if turning off parallelization would be the key here. I've recently refactored our setup to use a singleton for the MSW server, but this still causes the issue of data leaking across tests.

[kettanaito]

@JCB-K at the moment we do recommend running them in band to prevent possible issues when request handlers affect unrelated tests. This issue is aimed at discussion and adding support for parallel test runs. That being said, it looks like not everybody experiences this issue, so there's a space for some setup difference being at play.

[androa]

I'm experiencing this issue now. This is what I've found so far. As far as I can see jest does not run multiple tests at the same time by default. It needs to be explicitly set up by using the test.concurrent functions.

It does however use a worker pool, where each test file is given to a worker. All tests within that suite will use the same worker process.

Therefore a global beforeAll() (e.g. in the jest.setup.js file) will be ran for each worker. --runInBand will still spawn one worker, but reuse that worker for all the test files. Setting --max-workers=1 effectively does the same.

If you have more tests(files) than workers, jest will reuse workers (but still run beforeAll for each file).

Regardless of max-workers, Jest will not use more than the number of cores - 1 in single run mode, and number of cores / 2 in watch mode.

I can reproduce this issue quite frequently while running with --max-workers=1 or --runInBand, but not consistently.

A common denominator in my case is that I have three tests in a file. One happy path where it uses the globally registered mock handlers, and two unhappy paths where one is experiencing a delay (to verify that the spinner is shown) and one where there is a network error (to verify the error message). Those two tests registers their own handlers on the same endpoint/URL.

They seem to occasionally intersect with each other, that is, the spinner tests gets the request with an error, and the error get the delayed request.

[androa]

I have found two things in my test suite that creates the flaky behaviour. Simply clearing the cache like this does not help (all the time):

afterEach(async () => {
  cache.clear();
  server.resetHandlers();
});

When adding a artifical delay like this, it seems to consistently work:

afterEach(async () => {
  await new Promise((resolve) => setTimeout(resolve.bind(null), 0));
  cache.clear();
  server.resetHandlers();
});

I suspect this might be connected to the other thing I've found, but not been able to prove/create a consistent reproducing case.

I test a component that uses SWR to load data, but in this case it can render instantly before the fetch is complete and update it as the data comes in. I have no way to differentiate the loading status from an empty dataset, so the test passes immediately.

But the request (and the mock) is still in-flight and active, so the next test (which should have data) hits the previous mock.

Workaround was adding an invisible loader that I can wait for to disappear before passing the test.

[WayneEllery]

@androa I suspect I’m also having a similar problem to you. I tried adding an artificial delay (without cache clear) and it didn’t work. I didn’t consider using invisible loader which is a good solution so that in the test you can determine when the request has completed. In many of our tests when the api completes it doesn’t result in a screen change. There’s also many tests where an api call is made but the test doesn’t need to wait for it to complete.

I want to spend some time to create a reproduction repo but haven’t got round to it. Hopefully I can find some time soon.

[guest271314]

What is the use case and requirement?

[kettanaito]

@guest271314, the use case is to ensure the isolated application of runtime handlers (overrides) when running multiple tests relying on the same handler in parallel. The requirement is, well the same. I think the comments above will give you enough understanding about the issue.

[guest271314]

If I understand the requirement correctly, it is to run multiple instatnces of your tests at the same time, "paralellism"?

https://www.reddit.com/r/javascript/comments/y9whqr/comment/itc8do9/?utm_source=share&utm_medium=web2x&context=3

What do you mean by "parallelism"?

#474

I don't know why you would need node executable to intercept requests.

I would just use multiple ServiceWorkers, fetch(), Request, and respondWith() to test multiple instances of clients making multiple requests https://plnkr.co/edit/Cees43q7hrPsdmFK?open=lib%2Fscript.js.

[kettanaito]

@guest271314, let me clarify a thing or two. Test runners like Jest usually support "parallel runs" what they do internally is not a concern for this ticket. The end result is that there may be X amount of test files running in parallel. Those test files have test cases. Those test cases can make HTTP requests. Some of those cases may want to handle those HTTP requests differently.

Here's a scenario for you to get a grasp of the issue:

// testA.test.js
it('fetches user detail', async () => {
  await fetch('https://api.example/user')
})

// testB.test.js
import { rest } from 'msw'

it('handles user errors', async () => {
  server.use(
    // For GET https://api.example.user request ONLY within this test
    // respond with a 500 response.
    rest.get('https://api.example.user', (req, res, ctx) => res(ctx.status(500))
  )
  await fetch('https://api.example/user')
})

MSW already has request interception, runtime overrides via server.use(). The issue here is that these two tests, if run in parallel, may produce intermittent results. The first test may accidentally receive a 500 response coming from the runtime override from the second test. This used to happen for people in the past, I have no idea if this still happens now.

This very closely depends on how a test runner implement parallelism. For example, if those two tests run in separate threads, then each of their setupServer setup will be unique and there will be no way for them to have a shared state. If parallelism is achieved differently, well, then shared state may be possible.

There quite a few examples of this above, please read them. People go into much better detail of what's happening that I do in this comment.

[guest271314]

@kettanaito

Test runners like Jest usually support "parallel runs" what they do internally is not a concern for this ticket. The end result is that there may be X amount of test files running in parallel. Those test files have test cases. Those test cases can make HTTP requests. Some of those cases may want to handle those HTTP requests differently.

I think the code I posted achives the requirement - without bothering with node executable at all.

Just append N <iframes> to an HTML document and utilize fetch(), EventSource, et al. to make requests and serve Responses accordingly.

[clarksam19]

@kettanaito Would it make sense for server.use() to create a new handlers array mapped to the moduleId for that test (rather than mutating the global one)? Tests could still default to using the global array, but any test that calls server.use() would get it's handlers from a map at it's moduleId key. This would enable isolation for tests using runtime handlers without the unnecessary memory use of every test getting it's own handlers array by default.

[BrunoQuaresma]

@kettanaito is there any workaround for that?

[ubbe-xyz]

Is there any work going on with this? Being able to override responses for a given endpoint in tests confidently is basic to using msw. You don't want to just test happy paths 😅

[kettanaito]

@clarksam19, yes, scoping runtime handlers to the module ID would work. The problem is where to get that module ID? I believe I tried process.id/pid in the past but that's not a reliable indicator as there can be multiple threads in the same process, sharing that ID. I may be wrong about this so if you have a concrete example of what to use as a module ID that'd be great.

@lluia, you can provision overrides per test even now via server.use() and worker.use() API. The only limitation is that your test must run in sequence, which is what being discussed in this issue.

[EduardoSimon]

First things first, thank you for the amazing work that you are doing. I find msw amazing! On top of that, is there any workaround until there's a proper fix for the issue? We're struggling a bit with the flakyness of our test suites as we use runtime handlers to test sad paths.

Does it make a difference if we create a server per test file?

beforeAll(() =>
    server.listen({
      onUnhandledRequest: 'error'
    })
  );

  afterEach(() => {
    server.resetHandlers();
  });

afterAll(() => server.close());

[kettanaito]

@EduardoSimon, thanks for your kind words. The only workaround at the moment is disabling parallel runs in your tests. If you're using Jest, that would be adding --runInBand flag to its test command.

I think what @clarksam19 proposed has the most chance of being the right solution. I tried playing with some sort of moduleId in the past, and it haven't found a way to obtain/derive it reliably. Contributions and experiments are extremely welcome here!

One thing to try, is to check whether server.use() can understand in which module it's being called in. For example, log out __dirname in the .use() method implementation

msw/src/SetupApi.ts

Line 56 in e87e00b

public use(...runtimeHandlers: Array<RequestHandler>): void {

I think as long as its evaluated upon the function call, it should point to the directory of the module calling it.

[kettanaito]

Regarding coloring the requests happening in different test files or even test() blocks, I wonder if we can use AsyncLocalStorage for this. I have a feeling that we can get the execution/trigger async ID from Node.js and rely on it when resolving requests.

Once again, the main obstacle here is that the currentHandlers list that server.use() modifies, is a singleton that lives in-memory, and all tests reference the same list. Since it cannot be in N different state for N different tests, no solution is possible unless we compute the relevant list of handlers exactly when the request happens (don't store it anywhere).

[kettanaito]

Update

I've got a basic version of concurrent request resolution working using AsyncLocalStorage: https://github.com/kettanaito/setup-server-concurrency

This has a couple of concurrent tests in Jest running across multiple workers and using a minimal version of server.use() from MSW.

It utilizes async context with each .use() call to make any subsequent requests resolve against the list of request handlers fixed in time. So far so good. I'm looking forward to much smarter people to tell me that I'm using context incorrectly 😄 It looks too good to be true.

How this works

The issue itself is that MSW keeps an internal list of currentHandlers, which acts as a shared state between tests. Each .use() call prepends handlers to that list, and since multiple tests reference the same list, we end up with handlers leaking to irrelevant tests.

Async storage solves this problem this way:

• First, no handlers are stored in-memory. There's a getHandlers() function that either returns handlers from the async context (if any), or initial handlers.
• By default, setupServer() creates a new async context with initial handlers as the context value. This means all requests will use the initial list of handlers to resolve against.
• Calling .use() creates a new async context. There, whichever current handlers are modified with the provided override handlers. This means any subsequent request resolution uses that modified list of handlers as the source of truth.
• Calling .reset() creates yet another async context, which acts similar to the default context: resets the list of handlers to the initial list.

[kettanaito]

On AsyncLocalStorage

AsyncLocalStorage provides that "moduleId" function scope coloring we were talking about. This means it can be used to correctly scope scenarios like these:

const server = setupServer([]);

beforeAll(() => server.listen());
afterEach(() => server.reset());

test.concurrent("uses initial handlers", async () => {
  expect(await request("GET", "/one")).toEqual(null);
  expect(await request("GET", "/two")).toEqual(null);
});

test.concurrent("adds two overrides", async () => {
  server.use(["GET", "/two", 202], ["GET", "/three", 3]);
  expect(await request("GET", "/one")).toEqual(null);
  expect(await request("GET", "/two")).toEqual(202);
  expect(await request("GET", "/three")).toEqual(3);
});

In other words, we can use async storage to know if server.use() and the request resolution logic (i.e. your fetch() call) have been called in the same function (they will have the same executionAsyncId). We don't even need to do anything with that information, just update the storage with store.enterWith(newHandlers), and make request resolution grab the relevant state of handlers from the async store.

This works. Now, to the things that don't.

Scopes

In scenarios when server.use() and the request client have been called in different scopes, this approach fails.

const server = setupServer()
beforeAll(() => {
  server.listen()
  // This call will have a different execution ID...
  server.use(...handlers)
})

it('', () => {
  // ...than this call!
  fetch('/resource')
})

The async storage won't work either since those functions are considered to live in two separate universes and cannot share store. This means that you can only use server.use() in the same (or higher) scope as the request-issuing code to support concurrent runs. It's quite a hard limitation and since the implementation for this is the same for both concurrent and sequential test runs, it also breaks the sequential run entirely.

Interestingly enough, when put at the root scope of the module, functions acquire a 0 execution ID, which is implicitly shared with all subsequent execution IDs. But forcing developers where to call server.use() is (1) not a good DX; (2) may not always be possible due to the way their tests are constructed).

I'm trying to figure out what we can do with this knowledge.

[kettanaito]

Introducing server.boundary()

I believe I found an API that would support running MSW in concurrent tests and prevent shared state issue in the request handlers list. A new server.boundary() API.

Here's what it looks like:

msw/test/node/msw-api/setup-server/concurrency.test.ts

Lines 31 to 44 in c4447bf

	it(
	'resolves request against the in-test handler override',
	server.boundary(async () => {
	server.use(
	http.get('/initial', () => {
	return HttpResponse.text('override')
	}),
	)
	const response = await fetch('/initial')
	expect(response.status).toBe(200)
	expect(await response.text()).toBe('override')
	}),
	)

The idea is simple: you wrap whichever scope you want in server.boundary(), and no changes to request handlers will ever leave that boundary. You can wrap an entire test, or its part in that boundary, enabling isolation of network behavior.

Now, this is an explicit choice. Running tests in parallel is also an explicit choice so I see these two coming hand in hand.

The existing behavior of server.use() will not be affected. It will behave the same and will keep the request handlers in-memory. This is great for sequential test runs (most test frameworks don't run tests in parallel so you are getting a great default).

I already have tests passing for this feature and I think it is the way MSW will support concurrent test runs. Please share your thoughts on it below and help us shape it!

Important

You can help with reviewing the implementation: #2000. Thank you!

[kettanaito]

Released: v2.2.0 🎉

This has been released in v2.2.0!

• 📄 Release notes
• 📦 npm package

Make sure to always update to the latest version (npm i msw@latest) to get the newest features and bug fixes.

---

Predictable release automation by @ossjs/release.

[kettanaito]

Using MSW in concurrent tests

MSW can now be used in concurrent tests via the server.boundary() API!

• Read the announcement blog post
• server.boundary() API