Show onUnhandledRequest: 'error' in stdout and fail tests

[tobiasbueschel]

Happy new year & thanks a lot for msw, it's a fantastic library 🙏

Is your feature request related to a problem? Please describe.
I have an app bootstrapped with create-react-app and various tests that I run with Jest. I would like to configure my tests to fail if msw detects any unhandled network request. Hence, I've implemented the following configuration:

server.listen({ onUnhandledRequest: 'error' })

Unfortunately, doing so logs the unhandled network request errors to stderr, which the Jest watch mode by default doesn't show & doesn't fail the test execution. Hence, I've switched back to warn so that I can at least see if any network requests are unhandled when running the tests.

Describe the solution you'd like

1. It would be great if there's a way to direct the error level to stdout through the options of server.listen
2. Likewise, maybe we could create an additional option for an event handler (e.g. onUnhandledRequestHandler) that one can customise to throw or run process.exit(1) when we encounter an unhandled network request

Additional context

"bypass"	Performs an unhandled request as-is.
"warn"	Prints a warning into stdout of the current process.
"error"	Prints an error into stderr of the current process.

Source: docs

[kettanaito]

Hey, @tobiasbueschel. Happy new year to you too! Thank you for your kind words.

Would it work for you if onUnhandledRequest: 'error' killed the process in NodeJS?

I don't think that the introduction of additional options is necessary. The "error" value is designed to produce an exception that, in the case of NodeJS, should break the current process. The premise here is that this option is most commonly used during testing to catch unexpected API calls.

[kettanaito]

Note that while we're discussing this, you can provide a custom function to onUnhandledRequest that would end the process as you expect:

server.listen({
  onUnhandledRequest(req) {
    // Print any necessary error message based on the `req`.
    process.exit(1)
  }
})

I'd like this behavior to come as a default, but don't wish to block your integration for the time of discussion.

[timdeschryver]

I was thinking that this won't work when you're using Jest's watch mode, so I tried it in the example app.
The result was that the test fails (this is expected), but you don't see a good error message (this can be fixed by adding a log), but the watch mode is also canceled. See the example output below

$ react-scripts test --env=jest-environment-jsdom-sixteen --watch
  ●  process.exit called with "1"

      11 |     onUnhandledRequest(req) {
      12 |       // Print any necessary error message based on the `req`.
    > 13 |       process.exit(1);
         |               ^
      14 |     },
      15 |   });
      16 | });

      at onUnhandledRequest (src/setupTests.js:13:15)
      at onUnhandledRequest (../../node_modules/msw/node/index.js:713:9)
      at Object.<anonymous> (../../node_modules/msw/node/index.js:784:21)
      at fulfilled (../../node_modules/msw/node/index.js:26:58)

 RUNS  src/LoginForm.test.js
error Command failed with exit code 1.

[kettanaito]

I don't think that killing the process is the intended behavior. @tobiasbueschel is right that when we throw new Error as a part of the unhandled request callback, jest ignores it.

Alternatively, we can use console.error (which is writing to stderr, which will be displayed in Jest) for the "error" value of onUnhandledRequest. This would produce this following output during the test run:

  ● test name here

    TypeError: Network request failed

      at node_modules/whatwg-fetch/dist/fetch.umd.js:535:18

  console.error
    [MSW] Error: captured a GET https://api.github.com/unhandled request without a corresponding request handler.
    
      If you wish to intercept this request, consider creating a request handler for it:
    
      rest.get('https://api.github.com/unhandled', (req, res, ctx) => {
        return res(ctx.text('body'))
      })

      4077 |     switch (handler) {
      4078 |         case 'error': {
    > 4079 |             console.error(`[MSW] Error: ${message}`);
           |                     ^
      4080 |             throw new Error(`[MSW] Error: ${message}`);
      4081 |         }
      4082 |         case 'warn': {

      at onUnhandledRequest (../../mswjs/msw/node/index.js:4079:21)
      at Object.next (../../mswjs/msw/node/index.js:4179:25)
      at fulfilled (../../mswjs/msw/node/index.js:37:58)

This will not crash your app/tests, but I'm not sure that's ever the intention. Exiting the process with an error code would mean that none of your other tests will execute, which is never what you want. I originally anticipated that throwing an exception from within MSW would be treated exactly like I've posted above (visible exception message, process not stopped).

[kettanaito]

I've opened a pull request with the suggestion above (#544). @tobiasbueschel, could you please check out that PR and use its Codesandbox preview or local build to see if it behaves as you expect now? Thanks.

[tobiasbueschel]

Thanks a lot @kettanaito and sorry about the late reply - it's been rather busy at work! I've tried out the fix you've published as part of #544 and it's been working well for me 👏