msw icon indicating copy to clipboard operation
msw copied to clipboard

feat: support cross-process interception via `setupRemoteServer`

Open kettanaito opened this issue 1 year ago • 32 comments

This is an experimental feature. It's unlikely to ship before 2.0.

Intention

Introduce an API that allows one process to modify the traffic of another process. The most apparent application for this is testing server-side behaviors of a JavaScript application:

// app.js
forwardNetworkToRemote()

export const loader = async () => {
  const res = await fetch('https://example.com/resource')
}
// app.test.js
it('fetches the user server-side', async () => {
  let a = listenToRemoteNetwork(targetProcess)
  modifyNetwork(a)
  // ...render
  // ...assert
})

This is an example API. For the exact proposed API, keep reading.

This API is designed exclusively for use cases when the request-issuing process and the request-resolving process (i.e. where you run MSW) are two different processes.

Proposed API

With consideration to the existing MSW user experience, I suggest we add a setupRemoteServer() API that implements the SetupApi interface and has a similar API to setupServer. The main user-facing distinction here is that setupRemoteServer is affecting a remote process, as indicated by the name.

import { http } from 'msw'
import { setupRemoteServer } from 'msw/node'

const remote = setupRemoteServer(...initialHandlers)

// Notice: async!
beforeAll(async () => await remote.listen())
afterEach(() => remote.resetHandlers())
afterAll(async () => await remote.close())

The .listen() and .close() methods of the remote server become async since they now establish and terminate an internal server instance respectively.

Similar to the setupServer integration, it would be recommended to call setupRemoteServer once as a part of your global testing setup. Closing the WebSocket server after each test suite will have performance implications since each next test suite would wait while remote.listen() spawns that server again.

You can then operate with the remote server as you would with a regular setupServer, keeping in mind that it doesn't affect the current process (your test) but instead, any remote process that runs setupServer (your app).

it('handles user errors', () => {
  // Appending and removing request handlers is sync
  // because they are stored in the current (test) process.
  remote.use(
    http.get('/user', () => {
      return new Response(null, { status: 500 })
    })
  )

  // ...interact and assert your app.
})

By fully extending the SetupApi, the setupRemoteServer API provides the user with full network-managing capabilities. This includes defining initial and runtime request handlers, as well as observing the outgoing traffic of a remote process using the Life-cycle API (remote.events.on(event, listener)). I think this is a nice familiarity that also provides the user with more power when it comes to controlling the network.

Implementation

I've considered multiple ways of implementing this feature. Listing them below.

(Chosen) WebSocket server

The setupRemoteServer API can establish an internal WebSocket server that can route the outgoing traffic from any server-side MSW instance anywhere and deliver it to the remote server to potentially resolve.

Technically, the WebSocket server acts as a resolution point (i.e. your handlers) while the remote MSW process acts as a request supplier (similar to how the Service Worker acts in the browser).

Very roughly, this implies that the regular setupServer instances now have a fixed request handler that tries to check if any outgoing request is potentially handled by an existing remote WebSocket server:

// setupServer.js
await handleRequest(
  request,
  requestId,
  [
    // A very basic idea on how a "remote" request handler works.
    http.all('*', async ({ request }) => {
      wsServer.emit('request', serializeRequest(request))
      await wsServer.on('response', (serializedResponse) => {
        return deserializeResponse(serializedResponse)
      })
    }),
    ...this.currentHandlers,
  ]
)

Unlike request handler (i.e. function) serialization, it is perfectly fine to serialize Request and Response instances and transfer them over any message channel, like a WebSocket transport.

If no WebSocket server was found or establishing a connection with it fails within a sensible timeout period (~500ms), the setupServer instance of the app continues to operate as normal.

Alternatively, we can skip the WebSocket server lookup altogether and make it opt-in via some remote: true option on the app's side.

IPC

The test process and the app process can utilize IPC (interprocess communication) to implement a messaging protocol. Using that protocol, the app can signal back any outgoing requests and the test can try resolving them against the request handlers you defined immediately in the test.

This approach is similar to the WebSocket approach above with the exception that it relies on IPC instead of a standalone running server. With that, it also gains its biggest disadvantage: the app process must be a child process of the test process. This is not easy to guarantee. Depending on the framework's internal implementation, the user may not achieve this parent/child relationship, and the IPC implementation will not work.

Given such a demanding requirement, I've decided not to use this implementation.

Limitations

  • useRemoteServer() affects the network resolution for the entire app. This means that you cannot have multiple tests that override request handlers for the same app at the same time. I think this is more than reasonable since you know you're running 1 app instance that can only behave in a single way at a single point in time. Still, I expect users to be confused when they parallelize their E2E tests and suddenly see some network behaviors leaking across the test cases.

Concerns

  • Can we rely on a fixed local port to always be available?
  • Is it safe to introduce a WebSocket server that will be, effectively, routing HTTP messages over the local network (during tests only)?
    • Yes. If someone can intercept that WebSocket communication, they are already in your machine and can do things far worse than that.
  • Is it clear that setupRemoteServer only affects the server-side network behavior of any running application process with the server-side MSW integration? To affect the client-side network behavior from a test you have to 1) have setupWorker integration in the app; 2) set a global window.worker instance; 3) use window.worker.use() to add runtime request handlers. This stays as it is right now, no changes here.

The API is TBD and is subjected to change.

Roadmap

  • [x] Ensure the sync server connection is awaited before the first request handler runs.
  • [x] Introduce serialize/deserialize utilities for requests and responses (used both in the worker and in the WS sync layer now).
  • [x] Fix listeners' memory leaks on hot updates (clean up listeners).
  • [x] Make the WS events map type-safe
  • [x] Rely on the internal request header when bypassing Socket IO connection requests in the rest.all() handler.
  • [ ] Handle socket timeout and errors when awaiting for the response in setupServer.
  • [x] Support ReadableStream from the remote request handler (may consider transferring ReadableStream over the WS messages instead of ArrayBuffer, if that's allowed).
    • This may not be needed, in the end, but if we can pull off ReadableStream transfer over WebSockets that would be great.
  • [x] Support all Life-cycle events.
  • [x] Support setting a custom WebSocket server port number through environment variables.
  • [x] Make the remotePort and port an implementation detail of setupRemoteServer and setupServer({ remote: true }). The developer mustn't care about those.
  • [x] Do not spread the list of user-defined request handlers to prepend the fixed remote server handler (spreading of large lists may have performance implications).
    • Not an issue until proven otherwise; have no wish to optimize prematurely.
  • [ ] Solve the test/app catch 22 by attaching a self-replicating one-time handlers only for the first-time requests (those fetched when the testing framework pings your app).
  • [x] Fix: failing use() test (may have something to do with the handlers management refactoring as a part of the server.boundary()).
  • [ ] Support differentiating between requests done in different Playwright workers (see this).
  • [ ] Add more tests, specifically for different response body types.
  • [ ] Consider adding setupWorker support (see https://github.com/mswjs/msw/pull/1617#issuecomment-2356776359).

Blockers

  • [ ] https://github.com/socketio/socket.io/issues/4621 (type definitions for socket.io-parser are broken for the CJS build).
  • [x] Modern Remix requires sourcemaps. I added those for Interceptors but adding them to MSW breaks due to tsup: https://github.com/egoist/tsup/issues/1073
    • Fixed in https://github.com/mswjs/msw/pull/1958
  • [ ] Support different tests provisioning overrides for the same running app (https://github.com/mswjs/msw/pull/1617#issuecomment-2332499183).

kettanaito avatar May 12 '23 17:05 kettanaito

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit f78d69e1f339a74cd13ec826ae76aff56cd50c67:

Sandbox Source
MSW React Configuration

codesandbox-ci[bot] avatar May 12 '23 17:05 codesandbox-ci[bot]

ESM in DeferredPromise

Looks like the package doesn't ship as ESM, breaking MSW's ESM build, respectively. Need to add an ESM support there.

kettanaito avatar Jun 06 '23 14:06 kettanaito

Interesting approach with runtimes and interceptors. I am worried that maintaining so many runtimes is already complex for users to understand, and will only grow more complex as frameworks develop. In order to understand how to mock a request in Next.js or Remix, a user has to understand where the request is being issued (client or server), what process that code is running in (is it a separate node process, is it connected to my test runner somehow?), and what runtime it is using (browser, Nodejs or "edge" like Next.js middleware). I wonder if a unified mock server could simplify the mental model for writing tests. Something like the http-middleware from v1.

Drawbacks:

  • A mock server can only intercept requests to one domain.
  • The mock server would still need to be started by the test runner for test-time mocking.
  • Maintenance can be challenging but could be eased with a fetch-native API and MSW integration.

An alternative would be to try and hide these runtime and communication details from users more, possibly with adapters for each framework.

Eager to dive into the code and contribute. Great work as always!

kristojorg avatar Oct 27 '23 20:10 kristojorg

I believe Artem's intention is for handlers to be interchangeable between client and server environments. You should be able to assign handlers to a constant and call it with both setupRemoteServer and setupServer, though I think it would be more convenient for setupRemoteServer to do this automatically.

nickserv avatar Oct 27 '23 22:10 nickserv

I am worried that maintaining so many runtimes is already complex for users to understand, and will only grow more complex as frameworks develop.

To be clear, this feature introduces no additional runtimes. The inter-process interception is still meant for Node.js but you can, technically, combine tests in Node and runtime in something like React Native and that should still work.

You only need MSW in your development runtime. And that is still JavaScript, which is either the browser or Node, so in this regard nothing changes, even if you're developing edge functions or Cloudflare Workers, etc. You are still developing/testing them in Node.js.

In order to understand how to mock a request in Next.js or Remix, a user has to understand where the request is being issued (client or server), what process that code is running in (is it a separate node process, is it connected to my test runner somehow?), and what runtime it is using (browser, Nodejs or "edge" like Next.js middleware).

The user doesn't have to know any of that. Request handlers remain environment-agnostic, the only thing that changes is this:

  • If I want to affect a network of another process, I reach out for this new special setupRemoteServer() function in my tests.

This feature doesn't discard previously existing practices, it simply expands the capabilities of your tests for better server-side network control.

I wonder if a unified mock server could simplify the mental model for writing tests. Something like the http-middleware from v1.

Standalone HTTP server for mocking has a number of disadvantages and I would only recommend it in particular scenarios. That's why you still have @mswjs/http-middleware and V1/V2 combination if you need that setup. It should not be the default.

This feature unifies the mocking experience even more, because if you take a look at its implementation, you can notice how very little changes here. MSW is a very simple tool. It needs a source of network to get requests, and it resolves those requests against the list of request handlers. It's been like that 5 years ago, and it's still like that today. What changes is that the new setupRemoteServer API allows an independent process to become the source of the network, similar to how the Service Worker API allows the browser to become the source of the network. I find it rather beautiful.

An alternative would be to try and hide these runtime and communication details from users more, possibly with adapters for each framework.

I appreciate your advice but time has taught me that this is a slippery slope. What may seem like a better direction often leads to indirect complexity leaking to the end user. What I tend to do instead, is base my decisions on the established principles of MSW. Those served me well, they still do today and will continue so after this feature is merged. I don't want folks to configure adapters or runtimes. That's completely unnecessary and MSW has proven it so.

But that's the beauty in its API design: you still can have all that if you want. You can abstract to whichever degree you feel comfortable, which includes designing your configurations, publishing your setup to the internal package registry for other teams, making it HTTP server-first, etc. Support strong defaults, allow opinionated customization. That's the key.

Eager to dive into the code and contribute. Great work as always!

Thanks! I will certainly need some help with testing this once the implementation is ready. Hope to give it more attention later this/next month.

kettanaito avatar Oct 30 '23 14:10 kettanaito

@nickmccurdy is completely correct. All setupRemoteServer does is introduce an additional source for the network requests. Here are some possible usage combinations for this setup that can help you understand the intention behind this feature better:

Setup 1: Test-driven network

  • App: setupServer() + no request handlers.
  • Test: setupRemoteServer() + request handlers.

This way the test controls the server-side traffic of the tested application entirely.

Setup 2: Mixed network

  • App: setupServer() + happy-path handlers.
  • Test: setupRemoteServer() + per-test overrides with .use() to test deviating behaviors like network errors or error responses.

This is what you'd call a traditional testing setup for Node.js right now. It's still applicable to this inter-process testing story in the same shape.

Setup 3: App-driven network

  • App: setupServer() + request handlers.
  • Test: setupRemoteServer() + no handlers at all.

This allows to strategically turn on/off the network in your app for whichever requests you wish, while the tests (or any other process, like a script) can monitor the outgoing traffic.

kettanaito avatar Oct 30 '23 15:10 kettanaito

I think the remote style of connection must be an explicit choice by providing the remote: true option on the server.listen() call on the application's side:

// app/entry.server.js
server.listen({
  // By doing this, I tell MSW that I expect this application's network
  // to be controlled by the remote instance of MSW (e.g. in my tests). 
  remote: true
})

This way we'd save a bit on roundtrips of having to wait for the remote request handler if the remote is set to false, and have that entire fixed handler skipped altogether.

This is also where we may introduce some identity checks. It'd be a good idea to allow the developer to specify what exact process to connect to. Granted, that should be controlled on the setupRemoteServer()'s side (e.g. in the tests), and it needs some reliable way to establish said identification since things like process.pid are non-reproducible and random by design, fitting poorly for this purpose.

kettanaito avatar Nov 16 '23 18:11 kettanaito

ReadableStream support in WebSocket

WebSocketStream API in Chrome

Chrome is working on the spec for a new WebSocketStream API that would be nice to use here but it's not yet even experimental (simply doesn't exist in the stable Chrome).

Looks like for now, we'd have to map stream pushes to the WebSocket events manually.

websocket-stream package

The websocket-stream package seems to implement what we need. It promises both Node.js and browser support, which is nice. My only concern is that I don't see it directly mentioning you can use ReadableStream with it. I hope you can.

kettanaito avatar Jan 07 '24 17:01 kettanaito

Update the .use() API is failing

Looks like the setupServer() is having trouble connecting to the remote WebSocket server for some reason. Needs investigation.

Reproduction steps

pnpm build
pnpm test:node pnpm test:node test/node/msw-api/setup-remote-server/use.node.test.ts

Expected both tests to pass. The second one, the one using the override, is now failing.

kettanaito avatar Feb 20 '24 15:02 kettanaito

I think one of the tests fails because the test wasn't updated to pass the port value to the listen call.

Separately, the very long repeated logs can be solved (iirc) by testing for socket requests and passing the results through (#2041 has changes to SetupServerCommonApi.ts from around line 73 in 3ea224f which I believe was to prevent this recursive problem).

chrisb2244 avatar Feb 21 '24 03:02 chrisb2244

Just saw this PR. Since we're doing playwright integration testing in Remix and want to mock our actual backend (not the Remix backend for frontend), we're kind of reimplementing a poor man's version. Looking forward to when its picked back up!

Phoenixmatrix avatar Jun 10 '24 14:06 Phoenixmatrix

@Phoenixmatrix I'm unsure if you're willing to try unpublished versions, but if you wanted to try out my PR at https://github.com/mswjs/msw/pull/2041 I'd be grateful for any advance feedback (@kettanaito is I think busy with various other PRs and activities currently, but if you can point out any problems or confirm that it works as you'd expect, I can make changes if needed to try and smooth the merge).

chrisb2244 avatar Jun 11 '24 08:06 chrisb2244

FYI: There is a possible workaround for this i found during my search for this problem: Test your SvelteKit app end-to-end with Playwright and MSW

ar4hc avatar Jun 13 '24 07:06 ar4hc

FYI: There is a possible workaround for this i found during my search for this problem: Test your SvelteKit app end-to-end with Playwright and MSW

Glad that my post was of some help to you! :smile:

spuxx-dev avatar Jul 15 '24 10:07 spuxx-dev