Using "onUnhandledRequest": "error" does not fail tests

[bennettdams]

Describe the bug

When a server ist configured to listen with onUnhandledRequest: "error", the test where this error occurs does not fail.

Based on this discussion: #943

Environment

• Next.js, Jest, React Testing Library & MSW

https://github.com/bennettdams/msw-nextjs-bug/blob/master/package.json

This happens for a fresh install with all default configuration from the Next.js & MSW docs.

• msw: 0.35.0
• nodejs: 14.18.0
• npm: 6.14.15

To Reproduce

Reproduction repo: https://github.com/bennettdams/msw-nextjs-bug

To try it out:
npm i
npm run test

---

Steps to reproduce the behavior from zero:

1. Install initial Create Next App with TypeScript
   npx create-next-app@latest --use-npm --ts .
   bennettdams/msw-nextjs-bug@2cf426a

2. Create src folder at root and move pages folder to it
   bennettdams/msw-nextjs-bug@78b657e

3. Install & configure Jest, testing libs, etc. based on Next's docs:
   npm install --save-dev jest babel-jest @testing-library/react @testing-library/jest-dom identity-obj-proxy react-test-renderer
   bennettdams/msw-nextjs-bug@aaf1fb3

4. Create a simple test file based on Next's docs:
   bennettdams/msw-nextjs-bug@bd9ba77

5. Install MSW and follow setup for mocks/handlers/server via docs:
   npm install msw --save-dev
   bennettdams/msw-nextjs-bug@cdd07c1

6. Integrate MSW with Jest
   bennettdams/msw-nextjs-bug@3973836

7. Install and utilize whatwg-fetch - needed for Polyfill with Next.js
   npm install -D whatwg-fetch
   bennettdams/msw-nextjs-bug@5cbe84a

8. Change server config to onUnhandledRequest: "error" and add some simple fetch execution in the tested component
   bennettdams/msw-nextjs-bug@f77bb0f

==> The test does not fail, even though the tests shows the error.

Expected behavior

When onUnhandledRequest is configured with error and an unhandled request is found, the test shoud fail.

[kettanaito]

Hey, @bennettdams. Thank you for opening the issue and for providing the reproduction repository. You're awesome.

I can confirm the behavior you're experiencing and conclude that's an issue. I'm sharing some context and technical details on it below, both for posterity and for us to discuss. Don't hesitate to share your opinion on the best way to tackle this problem.

Context

• Show onUnhandledRequest: 'error' in stdout and fail tests #539, the exception was rewritten to console.error because it's being swallowed.
• onUnhandledRequest: Throws an exception when using the "error" strategy #856, the exception was added back, keeping the console.error, to properly handle fetch rejections (seems like a browser-specific issue).

The issue

The challenge here is to do both of the following:

1. Properly terminate a pending request when the "error" strategy is used.
2. Provide great developer experience, which includes propagating the internal exception to the test runner.

Terminating a pending request

This is done in a fairly straightforward way: whenever handleRequest function throws an exception, it's caught by the respective block in @mswjs/interceptors (see ClientRequest, XMLHttpRequest).

From the interceptor's perspective, an exception originating from onUnhandledRequest and any arbitrary exception during the mocked request processing (i.e. exception from the handler, intentional or not) are the same. It must handle such exceptions as the underlying native class would, meaning aborting the request. By doing so, the request client will get the identical behavior as when called without MSW.

Request resolution is entirely encapsulated within the interceptor's call:

msw/src/node/createSetupServer.ts

Lines 59 to 80 in 11bb244

	const interceptor = createInterceptor({
	modules: interceptors,
	async resolver(request) {
	const mockedRequest = parseIsomorphicRequest(request)
	return handleRequest<MockedInterceptedResponse>(
	mockedRequest,
	currentHandlers,
	resolvedOptions,
	emitter,
	{
	transformResponse(response) {
	return {
	status: response.status,
	statusText: response.statusText,
	headers: response.headers.all(),
	body: response.body,
	}
	},
	},
	)
	},
	})

Exceptions in this block trigger the logic flow I've described above. From the MSW's perspective no exception has happened because interceptors gracefully handled it. Thus, you don't see that exception propagating and failing your test, even though it should.

This also makes me wonder if we should take this request termination flow into account when approaching #778.

Developer experience

It's safe to assume that a great developer experience when using the "error" option for onUnhandledRequest would be:

1. See the verbose, developer-friendly error message printed.
2. Throw an exception to fail the relevant test.

While we're good with displaying a verbose error message, we currently don't throw the exception in the test runner's context (see the explanation above). This leads to the test runner thinking that the test passed (given no other criteria to fail it).

Conclusion

I'd like to conclude a few statements based on the investigation above.

1. Exception handling on the interceptors' side is correct. I wouldn't introduce a special way to handle exceptions so that only certain exceptions are propagated to the upper scope.
2. Exception propagation should, likely, happen on the handleRequest level in MSW, not interceptors. MSW is the surface controlling the onUnhandledRequest behavior, so it's reasonable to assume it should manage any logic necessary for it to work as expected.

Propagating the exception

Before throwing the unhandled request exception, the pending request must be terminated as described above (already present behavior). That is not to leave any pending promises afterward. This, in turn, means that the resolver function of the interceptors must execute in its entirety (cannot propagate the exception there).

Note that request resolution/termination is not connected to the resolver Promise.

In a nutshell, we need to perform something along these lines:

const interceptor = createInterceptor({
  modules: interceptors,
  async resolver(request) {
    const mockedRequest = parseIsomorphicRequest(request)

    // "handleRequest" is where the exception is thrown.
    // But we cannot propagate it here, that'd still be
    // handled by the "resolver" function of the interceptors
    // and treated as an arbitrary request/response exception
    // aborting the pending request without propagating the exception.
    return handleRequest<MockedInterceptedResponse>(
      mockedRequest,
      currentHandlers,
      resolvedOptions,
      emitter,
      {
        transformResponse(response) {
          return {
            status: response.status,
            statusText: response.statusText,
            headers: response.headers.all(),
            body: response.body,
          }
        },
      },
    )
  },
})

// <-- Need to propagate the exception to the higher scope.
// At the same time ensuring that it's propagated only after
// the pending request gets terminated (it still should).

// OPTION 1: Exception instance and a new callback.
interceptor.on('unhandledException', (error) => {
  if (error instanceof UnhandledRequestError) {
    // If an exception reported by the interceptors
    // was the exception thrown by "onUnhandledRequest",
    // propagate it to the upper scope.
    throw error
  }
})

Unfortunately, when propagated, it'll result in the general unhandled promise rejection:

(node:13492) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 2)

This will not fail the test.

[kettanaito]

On exception's scope

Let's go from the expected behavior here. We expect the onUnhandledError: "error" strategy to fail the relevant test. This means that it should throw an exception within that test's closure or any child closures:

it('some test', () => {
  // Either direct exceptions
  throw new Error('Like this')

  // Or exceptions thrown anywhere
  // in the child scope.
  render(<Component />)
})

Since the user won't throw exceptions in the test, we are implying a child scope here. Note that the exception must still be thrown, and that's where the biggest difficulty comes.

MSW itself has access only to two scopes:

1. Test setup scope (setupServer call).
2. Request client scope (fetch/XHR/etc.).

Throwing an error in the test setup's scope would be unexpected, as you lose the test-failure association. Thus, the only remaining scope where the library has any influence is the request client scope.

You see, request clients don't throw exceptions. And they are right in that: you don't wish for your entire app to blow if a single fetch call has failed. Instead, request clients either utilize Promises or callbacks, abstracting any occurred exceptions to be handled asynchronously.

And that leads us to a great discovery: MSW already throws the exception in the request client's scope upon unhandled requests! That's precisely the thing that terminates pending requests as I described above. You may access that exception using the error handling strategy appropriate for your request client (that depends on how the client exposes those exceptions to you).

[bennettdams]

@kettanaito Thanks a lot for your detailed answer, but I'm lacking knowledge to give you better feedback for that.

One thing I can ask though:

You may access that exception using the error handling strategy appropriate for your request client (that depends on how the client exposes those exceptions to you).

Do you have an example on how to do that?

Let's take the easiest "request client": fetch, similar to my example in the reproduction repo.

Usually, you have some kind of reusable helper function to execute requests (which e.g. uses fetch inside):

  // http-utils.ts
  async function fetchSomething() {
    // imagine a more sophisticated implementation here
    try {
      return await fetch("https://jsonplaceholder.typicode.com/todos/1")
    } catch (error) {
      throw error
    }
  }
  
  // SomeComponent.tsx
  useEffect(() => {
    async function test() {
      const response = await fetchSomething()
      console.log("Response: ", response)
    }
    test();
  }, []);

Where would you access the exception, when the request is "interrupted" by MSW?
Even with that try..catch block in the example above, the test does not fail.

As you said, as a developer we maybe don't want to throw errors here, to not blow the entire app. How would we handle MSW's exception then to let the test fail?
But also, in this scenario, fetch does not respond with anything (as MSW "interrupts" it), so you can't check e.g. the status code. And even if you have the status code, how would you tell the test to fail?

I think one of the great benefits of MSW is that I can write my tests without thinking about requests that are used for a test every time, because I took care of that in my handlers in the first place.

---

As a compromise, if this is not working, I'd be also fine with having one single test whose only purpose is failing when an unhandled request occured in any of the other tests, based on MSW. That will not show me which test exactly is responsible for sending that request, but at least I know that I have an unhandled request somewhere and can start digging myself.

Is there a solution for this right now?
If not, there's maybe one "easy" thing we could do: Have some kind of callback on the server instance: server.onUnhandledRequest(...)

// msw-setup.ts
...
export const server = setupServer(...handlers);
...

..and then use that in a dedicated test:

describe("Requests", () => {
  it("throw an error when an unhandled request is found", () => {
    const mswSpy = jest.spyOn(server, "onUnhandledRequest")

    expect(mswSpy).not.toHaveBeenCalled();
  });
});

[kettanaito]

We cannot base this approach on any exact request client, unfortunately. People may use fetch with async/await as you do above, or may use it as a plain Promise, or may use XMLHttpRequest. Each request client decides how to handle and propagate exceptions, including not doing so at all.

Take a look at this example:

/**
 * @jest-environment jsdom
 */
import 'whatwg-fetch'

it('fetch (plain)', () => {
  fetch('https://do.not.exist')
})

it('fetch (async/await)', async () => {
  await fetch('https://do.not.exist')
})

it('XMLHttpRequest', () => {
  const xhr = new XMLHttpRequest()
  xhr.open('GET', 'https://do.not.exist')
  xhr.send()
})

Only the second test will fail requesting a resource that doesn't exist. And it will fail only because it disrupts the await flow and only because fetch rejects the promise (don't confuse with throwing an exception!).

But also, in this scenario, fetch does not respond with anything (as MSW "interrupts" it), so you can't check e.g. the status code.

Status code is a property on response. When there's an exception during a request, it won't be performed, thus, you get no response. This behavior is correct.

I think one of the great benefits of MSW is that I can write my tests without thinking about requests that are used for a test every time because I took care of that in my handlers in the first place.

While the test is not marked as failed, any assertions that depend on data will fail. The request is still terminated even as of now, so if you rely on any data from your fetch call, it won't be there. Eventually, the test will fail on your assertions. If it doesn't, then the mocked request has no effect over the expectations that you're testing (it still won't get performed, so you're safe).

As a compromise, if this is not working, I'd be also fine with having one single test whose only purpose is failing when an unhandled request occured in any of the other tests,

I strongly advise you against this. Tests are here to reflect the intention of the software you write. They must not act as logical gateways to orchestrate other tests.

[bennettdams]

While the test is not marked as failed, any assertions that depend on data will fail. The request is still terminated even as of now, so if you rely on any data from your fetch call, it won't be there. Eventually, the test will fail on your assertions. If it doesn't, then the mocked request has no effect over the expectations that you're testing (it still won't get performed, so you're safe).

I think this point is very important and comes down to why I think the tests should fail on unhandled requests anyways: I want to fill my handlers with a baseline of data that I can reuse for every typical test scenario.
Now obviously one could check in every test (or test suite) whether the expected mock data is available in the test, I just thought it's nice that a developer never has to do that, because his mock data is already defined in a central place (the MSW handlers). Only when I'm testing something that is not default (e.g. "What is my UI doing, when there's no data?"), I want to change the data for that specific test (and can easily do so by overwriting the handler in that test).

[kettanaito]

The behavior you expect is reasonable and I think the library should also behave that way. At the moment I don't see how we can achieve that, so we must research the options before moving forward. My discoveries above are a great starting point but they don't spotlight any possible solutions to this issue. We may want to look into how other similar libraries are failing tests, I'm sure they have their analogs of onUnhandledRequest: "error".

[kettanaito]

Looking into how other libraries deal with this, nock, for example, throws a direct exception within the test. It can afford that because you're using nock.disableNetConnect() directly in your test file's scope:

// my.test.js
const nock = require('nock')
nock.disableNetConnect()

That, however, doesn't fail the test, it exits the process. This is a disruptive action and I'd much prefer for tests to continue running and only the affected tests to be marked as failed.

[balzdur]

Hello @kettanaito, what is the status of your investigation ?

[kettanaito]

Hey, @balzdur. There hasn't been any additional investigation than the one above.

I did stumble upon what seems the behavior we want when rewriting the ClientRequest interceptor but I don't have tests to confirm that.

Anyone is welcome to dive into this to see how we can support failing individual tests (or the entire test suite) whenever onUnhandledRequest is set to error.

Note on semantics

I wonder if such test failures are semantic in regards to your assertions. Think of it this way: if your test setup (i.e. the beforeAll hook) throws, it's not your individual tests that are marked as failed, it's the entire test run, with the exception originating from the problematic area (i.e. the hook).

Since MSW is, effectively, a party of your testing setup, I'd say that it makes more sense to fail the entire test suite rather than try to figure out how to associate requests with the test code that issued them (spoiler: that's likely impossible).

That being said, it still poses a challenge due to how the library is architectures. As the first step, I'd try propagating the exception from onUnhandledRequest to server.listen90 scope, which is the direct scope used in your test setup.

[kettanaito]

I also wonder what would happen if MSW responds to unhandled requests with 500 (or 405 if that makes more sense)? That should halt individual tests, as that server error would propagate to the request client and cause an exception. Even if such an error is handled by the client, it still won't produce the expected state. The only downside here is that false-positive tests are still possible with this approach.

[sbdchd]

I think moving the .listen call around can help slightly with the error messages, but still not great.

If we have the listen call at the module top level the stack trace points to a line number at the listen call (which isn't inside the test)

❯ node ./node_modules/vitest/vitest.mjs src/frontend/src/components/Login.test.tsx -t login success --api 54184 --no-watch

 RUN  v0.19.1 src/frontend
      API started at http://localhost:54184

stderr | src/components/Login.test.tsx > login success
Found an unhandled POST request to http://localhost:3000/api/v1/auth/login/

 ✓ src/components/Login.test.tsx (3)

Test Files  1 passed (1)
     Tests  2 passed | 1 skipped (3)
      Time  1.14s (in thread 57ms, 1999.11%)

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run. This might cause false positive tests.
Please, resolve all the errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Error: found an unhandled POST request to http://localhost:3000/api/v1/auth/login/
 ❯ onUnhandledRequest src/components/Login.test.tsx:44:6
     42|   onUnhandledRequest(req) {
     43|     Promise.reject(
     44|       Error(`found an unhandled ${req.method} request to ${req.url.href}`),
       |      ^
     45|     )
     46| 
 ❯ onUnhandledRequest node_modules/msw/lib/node/index.js:1169:5
 ❯ handleRequest node_modules/msw/lib/node/index.js:1203:5
 ❯ setupServerListener node_modules/msw/lib/node/index.js:1272:25

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

If we put the listen call in the test itself, then it will provide a line number so we can more easily track down which test failed, but still not great.

❯ node ./node_modules/vitest/vitest.mjs src/frontend/src/components/Login.test.tsx -t login success --api 54184 --no-watch

 RUN  v0.19.1 src/frontend
      API started at http://localhost:54184

stderr | src/components/Login.test.tsx > login success
Found an unhandled POST request to http://localhost:3000/api/v1/auth/login/

 ✓ src/components/Login.test.tsx (3)

Test Files  1 passed (1)
     Tests  2 passed | 1 skipped (3)
      Time  1.34s (in thread 57ms, 2344.11%)

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Errors ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

Vitest caught 1 unhandled error during the test run. This might cause false positive tests.
Please, resolve all the errors to make sure your tests are not affected.

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Unhandled Rejection ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯
Error: found an unhandled POST request to http://localhost:3000/api/v1/auth/login/
 ❯ onUnhandledRequest src/components/Login.test.tsx:50:8
     48|     onUnhandledRequest(req) {
     49|       Promise.reject(
     50|         Error(`found an unhandled ${req.method} request to ${req.url.href}`),
       |        ^
     51|       )
     52| 
 ❯ onUnhandledRequest node_modules/msw/lib/node/index.js:1169:5
 ❯ handleRequest node_modules/msw/lib/node/index.js:1203:5
 ❯ setupServerListener node_modules/msw/lib/node/index.js:1272:25

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

edit: looking at how Apollo does it, they have a similar issue: apollographql/apollo-client#5917

[sbdchd]

I spent more time playing with this, and this is what I have so far:

const server = setupServer()
// Start server before all tests
beforeAll(() => server.listen())
// Close server after all tests
afterAll(() => server.close())
// Reset handlers after each test `important for test isolation`
afterEach(() => server.resetHandlers())
// keep track of inflightRequests because onUnhandledRequest is called after
// `afterEach`.
const inflightRequests = new Map<string, MockedRequest<DefaultBodyType>>()
server.events.on("request:start", req => {
  inflightRequests.set(req.id, req)
})
server.events.on("request:end", req => {
  inflightRequests.delete(req.id)
})
// check if we had any unhandled requests
afterEach(() => {
  const errors = [...inflightRequests.values()]
  inflightRequests.clear()
  if (errors.length) {
    // TODO: figure out if we can include where in the code the request was called
    const err = Error(
      errors
        .map(
          req => `found an unhandled ${req.method} request to ${req.url.href}`,
        )
        .join("\n\n"),
    )
    // clear the useless stack trace
    err.stack = undefined
    throw err
  }
})

This results in tests with output like:

❯ node ./node_modules/vitest/vitest.mjs ~/frontend/src/components/Login.test.tsx -t login success --no-watch

 RUN  v0.19.1 ~/frontend

stderr | src/components/Login.test.tsx > login success
[MSW] Warning: captured a request without a matching request handler:

  • POST http://localhost:3000/api/v1/auth/login/

If you still wish to intercept this unhandled request, please create a request handler for it.
Read more: https://mswjs.io/docs/getting-started/mocks

 ❯ src/components/Login.test.tsx (3)
   ↓ it works [skipped]
   × login success
     ⠙ [ afterEach ]
   ✓ login failure

⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

 FAIL  src/components/Login.test.tsx > login success
Error: found an unhandled POST request to http://localhost:3000/api/v1/auth/login/
⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯

Test Files  1 failed (1)
     Tests  1 failed | 1 passed | 1 skipped (3)
      Time  1.32s (in thread 56ms, 2357.46%)

Seems to work pretty well, only tested it in vitest but should work in jest 🤞🏻

Edit: it doesn't work, the after runs before the requests resolve 😢

stderr | src/components/Login.test.tsx > login success
1659211293806 request:start 7fa828e0-18be-40a4-90b7-2c9751a0ec40
1659211293810 after
1659211293811 request:match 7fa828e0-18be-40a4-90b7-2c9751a0ec40
1659211293811 request:end 7fa828e0-18be-40a4-90b7-2c9751a0ec40

stderr | unknown test
1659211293812 request:mocked 7fa828e0-18be-40a4-90b7-2c9751a0ec40

stderr | src/components/Login.test.tsx > login failure
1659211293812 after

Edit2: with some promises I think it now works:

import { setupServer } from "msw/node"
import { DefaultBodyType, MockedRequest } from "msw"
type Deferred<T> = {
  promise: PromiseLike<T>
  done: () => void
  error: () => void
}
function deferred<T>(data: T): Deferred<T> {
  // based from: https://stackoverflow.com/a/69027809/3720597
  let resolve: (value: T | PromiseLike<T>) => void
  let reject: (reason?: unknown) => void
  const promise = new Promise<T>((res, rej) => {
    resolve = res
    reject = rej
  })
  return {
    promise,
    done: () => {
      resolve(data)
    },
    error: () => {
      reject(data)
    },
  }
}
const server = setupServer()
beforeAll(() => server.listen())
afterAll(() => server.close())
afterEach(() => server.resetHandlers())
const inprogress = new Map<string, Deferred<MockedRequest<DefaultBodyType>>>()
server.events.on("request:start", req => {
  inprogress.set(req.id, deferred(req))
})
server.events.on("request:match", req => {
  inprogress.get(req.id)?.done()
})
server.events.on("request:unhandled", req => {
  inprogress.get(req.id)?.error()
})
afterEach(async () => {
  const errors = (
    await Promise.allSettled([...inprogress.values()].map(x => x.promise))
  )
    .filter((x): x is PromiseRejectedResult => x.status === "rejected")
    .map((x): MockedRequest<DefaultBodyType> => x.reason)
  inprogress.clear()
  if (errors.length) {
    // TODO: figure out if we can include where in the code the request was called
    const err = Error(
      errors
        .map(
          req => `found an unhandled ${req.method} request to ${req.url.href}`,
        )
        .join("\n\n"),
    )
    // clear the useless stack trace
    err.stack = undefined
    throw err
  }
})

[nico1510]

@sbdchd I have the same problem and at least for my very simple test suite the following hack worked:

onUnhandledRequest: (req) => {
        test(`${req.url} is not handled`, () => {});
      },

test here comes from jest and jest doesn't allow for tests to be nested or else it will fail with an error message. Coincidentally this will only happen when you have unhandled requests, so in order to know which request failed I pass the url to the test's name. Could you please let me know if this works for you?

[kettanaito]

@nico1510, you can use life-cycle methods to have a similar effect:

test('my test', () => {
  server.on('request:unhandled', () => {
    throw new Error('Must handle request')
  })
})

Would this work the same way? Depends whether that exception will propagate to the test scope.

[nico1510]

@kettanaito hmm but how is this different than using the onUnhandledRequest property and passing a callback?
I tested it like this:

server.events.on('request:unhandled', () => {
      throw new Error('Must handle request');
    });

and somehow it's not even executed 🤔
But anyway throwing an error inside the onUnhandledRequest handler doesn't propagate to the test scope 😢 Isn't this what this issue is about?
Only by dynamically adding another test call in the error handler I can make jest fail.

To give a little more context about why one would like to make tests fail on unhandled requests. Your point here makes a lot of sense to me:

Eventually, the test will fail on your assertions. If it doesn't, then the mocked request has no effect over the expectations that you're testing

But... In reality multiple developers work on the same code base and someone might add an unhandled call to some of the components under test. The developer who first wrote the test could not have predicted this and now there might be unexpected changes in the way his code is executed. Even if those changes don't make the tests fail I see this as a problem because at some point they might. E.g. if someone adds another request which depends on the first unhandled request he has now the burden of mocking both requests.
Usually the one who added an unhandled request to the codebase is also the one who knows how to mock it so I think the burden should be on him to do this.
I think that's why you should have the option to make tests fail when this happens. I don't think it should be the default but having the option would be nice.

I also wonder what would happen if MSW responds to unhandled requests with 500 (or 405 if that makes more sense)

Users might handle those errors in their request libraries in multiple different ways e.g. by showing an error fallback or retrying requests or whatever... So I think if someone wants to have this behaviour then they could just implement this themselves via the onUnhandledRequest callback no?

[kettanaito]

I think that's why you should have the option to make tests fail when this happens

Yeah, I'm totally on board with this. It's rather a technical challenge to make this happen. I'm open to suggestions and, hopefully, pull requests. I've started at #951 but never got anywhere with this.

As you've correctly pointed out, the exceptions are thrown in onUnhandledRequest() function will not propagate to the test because they are not related to the test() call stack in any way. Coercing such an exception as a request client exception is not also semantically correct, I agree with your points above.

So, we either need a way to hook into the individual test()'s call stack or design some magic for this to work. Both are quite sordid options.

[nico1510]

Yes that makes sense and after your explanations I'm starting to think that it shouldn't even be of any concern to msw to make tests fail... Instead the test runner should provide an option to make a test fail manually which we can then execute in the onUnhandledRequest callback. Else you'd have to account for all sorts of test runners I guess 🤔

[michael-harrison]

@kettanaito My teams is relatively new to MSW and have come up against this in our test suite. We're using Jest for our testing. To address the issue we're spying on the console.warn which is not ideal but it allows us to fail the test. For example,

const consoleSpy = jest.spyOn(global.console, "warn")

describe('Some tests', () => {
  beforeAll(() => server.listen())
  afterEach(() => {
    server.resetHandlers()
    expect(consoleSpy).not.toHaveBeenCalled()
  })
  afterAll(() => server.close())

 // tests
})

While it would be good to have tighter integration with Jest (and others) which could be a lot of effort perhaps a less integrated solution would be good for the interim:

describe('Some tests', () => {
  beforeAll(() => server.listen())
  afterEach(() => {
    server.checkUnhandledRequests()
    server.resetHandlers()
  })
  afterAll(() => server.close())

 // tests

If there are unhandled requests the checkUnhandledRequests() function would throw and error.

[kettanaito]

Hi, @michael-harrison. That's a neat implementation. It would mess up the stack trace a bit though, as the exception would happen in the afterEach hook and won't be associated with the precise test that has failed the onUnhandledRequest strategy, will it?

[michael-harrison]

@kettanaito Thanks, it's just a thought at the moment.

TLDR; the v1.1.0 update release a few days ago resolves the issue for us.

I assume by messed up you're meaning the stack trace will not point you to the exact line in the test that has a API call with a missing response. Yes this is true however, Jest will report the test that failed. I did a quick test with jest for proof of concept:

import {afterEach, describe, it} from "@jest/globals"
let throwMe

describe('Some tests', () => {
  afterEach(() => {
    if (throwMe) {
      throw new Error('You failed')
    }
  })

  describe('when some condition 1', () => {
    it('will have the expected result', async () => {
      // Test
      throwMe = true
    })
  })

  describe('when some condition 2', () => {
    it('will have the expected result', async () => {
      // Test
      throwMe = false
    })
  })
})

Output from the test

 FAIL  tests/ui/testThrow.spec.js
  Some tests
    when some condition 1
      ✕ will have the expected result
    when some condition 2
      ✓ will have the expected result (1 ms)

  ● Some tests › when some condition 1 › will have the expected result

    You failed

       5 |   afterEach(() => {
       6 |     if (throwMe) {
    >  7 |       throw new Error('You failed')
         |             ^
       8 |     }
       9 |   })
      10 |

      at Object.<anonymous> (tests/ui/testThrow.spec.js:7:13)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 1 passed, 2 total
Snapshots:   0 total
Time:        0.765 s, estimated 1 s

This may not be the same for all test frameworks.

That aside, since encountering this issue (about 4 days ago) I noted that an updated for MSW has been released V1.0.0 > V1.1.0 3 days ago. I've done the update and I can confirm that the update resolves this issue for my team; the following now causes tests to fail when there are unhandled requests:

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

Which yields something like

Error sending the query 'myQuery' FetchError {
      message: 'request to http://core.test.lan/app/graphql failed, reason: [MSW] Cannot bypass a request when using the "error" strategy for the "onUnhandledRequest" option.',
      type: 'system',
      errno: undefined,
      code: undefined
    }

The stack trace allows us to dig down into the library calling the API library where the call came from.

[kettanaito]

That is pretty nice. We haven't shipped this support explicitly but maybe something in the context changed, which now results in those unhandled exceptions propagating as request errors. Come to think of it, they should've always been treated as response errors but I frankly haven't touched this part of the library in quite some time so I don't remember.

[Nilegfx]

@kettanaito thanks for the amazingly detailed explanation of the problem.

after reading all the comment I ended up using this solution.

import {
  vi,
  describe,
  beforeAll,
  afterEach,
  afterAll,
  test,
  expect,
} from 'vitest';

import { setupServer } from 'msw/node';
import { rest } from 'msw';
import Axios from 'axios';

const getData = (url: string) => {
  return Axios.get(url).then(({ data }) => data);
};

const onUnhandledRequest = vi.fn();

const server = setupServer(
  rest.get('https://jsonplaceholder.typicode.com/todos/1', (_, res, ctx) => {
    return res(
      ctx.status(200),
      ctx.json({
        a: 'a',
      })
    );
  })
);

describe('onUnhandledRequest workaround', () => {
  beforeAll(() => {
    server.listen({
      onUnhandledRequest,
    });
  });

  afterEach(() => {
    server.resetHandlers();
    expect(onUnhandledRequest).not.toHaveBeenCalled();
    onUnhandledRequest.mockClear();
  });

  afterAll(() => {
    server.close();
  });
  test('request handler matched', async () => {
    const data = await getData('https://jsonplaceholder.typicode.com/todos/1');
    expect(data).toMatchObject({
      a: 'a',
    });
  });

  test('request handler not matched', async () => {
    const data = await getData('https://do.not.have.a.handler');
    expect(data).toMatchObject({
      a: 'a',
    });
  });
});

this should be straight-forward to implement with any test runner. for vite I am able to see which test is failing as below which is enough for me.

[kettanaito]

Update

There's a bit of news regarding this. When using global fetch in Node.js, you will get individual test failures because the "error" strategy will manifest as the TypeError: Failed to fetch error thrown by Undici (global fetch). I believe that will result in that exception failing a particular test correctly.

Ultimately, this will always come down to how the request client you're using is handling/forwarding unhandled exceptions. Most likely, those will be treated as network errors and will, once again, fail the relevant requests and tests.

We can't do anything else really on MSW's side here. MSW sits below your request client on the environment level so we don't have access to the individual test's context to affect it in any way. I'm reluctant to consider things like module IDs because those are likely not to be reliable.

I'm closing this issue in its current state. I recommend upgrading to msw@next, removing any fetch polyfills from your tests, and observing the behavior. Pull requests are welcome to improve this as well!

[kettanaito]

Since 2.2.0, I believe we can use server.boundary() to help us fail individual tests. At least, it provides us with the scope to do so. It may require additional rework internally because onUnhandledRequest does its own thing, but I do believe it's technically possible.

Pull requests are welcome!