interceptors icon indicating copy to clipboard operation
interceptors copied to clipboard

feat: add WebSocket interceptor

Open kettanaito opened this issue 2 years ago • 6 comments

SocketIO support

  • [x] "websocket" transport (browser)
  • [ ] "polling" transport (browser XMLHttpRequest)
    • [x] Add send/emit integration tests
    • [ ] Support sending requests to the original server
    • [x] ~~Support XHR polling outside of socket.io~~
  • [ ] "websocket" transport (Node.js), uses ws
  • [x] "polling" transport (Node.js)
    • [x] Clear ping/poing interval when the connection is closed (introduce Transport.close()).

Polishing

  • [ ] Add interceptor.once(), it's useful for tests. Alternatively, clean up listeners between tests in another way.

Nice-to-have

  • [ ] Use Service Worker instead of XMLHttpRequestInterceptor in the browser

kettanaito avatar Apr 07 '22 15:04 kettanaito

Connection and Transport

A “connection” is a user-facing instance they receive in the “connection” event listener of the interceptor:

wsInterceptor.on('connection', (socket) => {
  socket // this is a connection instance
})

For semantics, it’s often named “socket” but it’s not an actual socket reference.

Connection is responsible for receiving and emitting events. Keep in mind that it still remains an abstract bridge between the interceptor and the user.

  • Connection can transform incoming/outgoing data before sending it via the transport.
  • Each connection must have exactly one transport.
  • Connection tells the transport when to initiate a handshake sequence. In turn, the interceptor notifies the connection about it.
  • Interceptor signals about a new connection via connection.onOpen.
  • Interceptor signals about an outgoing client-side event via connection.onMessage. Each connection may override this method to implement custom logic (i.e. decoding the sent data).

Transport

A “transport” is responsible for actually sending events to the WebSocket instance, or mocking HTTP responses in the case of HTTP polling. Transport is tight to the underlying transport of SocketIO.

  • Each transport implements the same public contract and only differs in implementation details of that transport.
  • Transport is responsible for the handshake because it must be responded to differently (dispatch events on window.WebSocket, or mock a response during HTTP polling).
  • Transport always sends MessageEvent events. If any data transformation is needed, it must be handled in the connection before sending the data via the transport.

kettanaito avatar Apr 09 '22 14:04 kettanaito

In the browser, we will only support:

  • Native window.WebSocket
  • And all socket.io transport: websocket, polling.

Nothing else will be supported. You cannot implement WebSocket via HTTP polling without resorting to a framework.

kettanaito avatar Apr 09 '22 14:04 kettanaito

Current state

Need to decide on how interceptor listeners persist (or don't) after .dispose(). My stance is that disposing of an interceptor must remove all listeners it or its underlying emitter have. Dispose is a destructive action, and you would have to add request listeners again if you wish to revive the same interceptor.

Currently, that seems to be causing failures in tests. I suspect leaking state between unrelated tests:

 FAIL  test/modules/fetch/intercept/fetch.test.ts
  ● intercepts an HTTP HEAD request

    expect(jest.fn()).toHaveBeenCalledTimes(expected)

    Expected number of calls: 1
    Received number of calls: 2

      50 |   })
      51 |
    > 52 |   expect(resolver).toHaveBeenCalledTimes(1)
         |                    ^
      53 |   expect(resolver).toHaveBeenCalledWith<
      54 |     Parameters<HttpRequestEventMap['request']>
      55 |   >({

      at modules/fetch/intercept/fetch.test.ts:52:20
      at fulfilled (modules/fetch/intercept/fetch.test.ts:5:58)
          at runMicrotasks (<anonymous>)
 FAIL  test/features/remote/remote.test.ts (6.927 s) promise rejections are deprecated. In the future, promise rejections that are not h
  ● intercepts an HTTP request made in a child process

    thrown: "Exceeded timeout of 5000 ms for a test.
    Use jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test."

      37 | })
      38 |
    > 39 | test('intercepts an HTTP request made in a child process', async () => {
         | ^
      40 |   child.send('make:request')
      41 |
      42 |   const response = await new Promise((resolve, reject) => {

      at Object.<anonymous> (features/remote/remote.test.ts:39:1)

The remote.test.ts seems to always be present in these failures, other tests seem random.

kettanaito avatar Apr 12 '22 16:04 kettanaito

For once, if interceptor.on would prepend a listener, it'd provision an hierarchy and try to guarantee that most recent listeners would be the most relevant ones.

kettanaito avatar Apr 12 '22 16:04 kettanaito

SocketIO requests intercepted by a different test

I've got this resolved by https://github.com/mswjs/interceptors/pull/236/commits/8f2c6043f692ecc53cc707cd712cd76ac758cdd4. It was a matter of pushing the emitter.removeAllListeners() as a part of the interceptor's apply and not the constructor.

The issue was that if the listener disposal was added in the constructor, once the interceptor is disposed of, that side-effect function is removed (side-effects are pop-ed, so their list is reduced). If the side-effect is added in the apply instead, it will always be added, even if a single module alternated between apply/dispose multiple times.

kettanaito avatar May 02 '22 14:05 kettanaito

Clear ping/pong interval if the connection is closed

This is already implemented. Here's the stack trace:

- Connection.close()
  - Transport.dispose()
    - invokes `resolve(createPingResponse())` side-effect

Whenever a polling interval is created, the SocketIoPollingTransport adds a new subscription that resolves the pending request body Promise and clears the polling interval (because clearing the interval happens in the .finally() callback of the request body Promise).

kettanaito avatar May 02 '22 14:05 kettanaito

@kettanaito any updates here? :)

SerkanSipahi avatar Oct 17 '22 06:10 SerkanSipahi

I think the current state of things reflects where we are in terms of WebSocket support. A lot of work done but probably even more so left to do. I don't have time to dedicate to WebSockets this year so maybe sometime another year? Generally, whenever there's an update I will share it so everyone would see.

kettanaito avatar Oct 17 '22 09:10 kettanaito

@kettanaito is there a task list of work left? Maybe others can chip away at parts in the meantime.

bmulholland avatar Oct 17 '22 11:10 bmulholland

@bmulholland, yeah, it's right in the description—those checkboxes. I don't think this is an accessible feature to dive in but if anyone wants to, it's certainly welcome! The main complexity comes from the many ways that WebSocket is implemented. I adopt the concept of "transports" from SocketIO in the polyfill because it makes sense: you can use window.WebSocket, or http/XMLHttpRequest polling. This variability was the main reason I've added a BatchedInterceptor, so we can have dedupe http/XHR interceptors for both regular request and WebSocket interception.

The main challenge I was facing the last time I worker on this was the support for WebSocket in Node. There's no global class there, so people often use the ws package. I still don't know how we can support this transparently, given we don't patch third-party modules—that's never a good idea. So, the next step was to:

  1. See how the ws module works. What Node internals it utilizes?
  2. Hopefully, it's implemented over http, so we can use our existing ClientRequest interceptor and just chime in. If ws is implemented on a lower level with net.Socket, we wouldn't be able to support it at all.

kettanaito avatar Oct 17 '22 11:10 kettanaito

I thought I remembered reading something not long ago about a native WebSocket coming to Node, but now I can't find any evidence of it, so I must have dreamed it. There is this longstanding issue discussing support for it in Node: https://github.com/nodejs/node/issues/19308. The community seems to have standardized around https://github.com/websockets/ws in the interim (and many use Socket.io as you already know).

There's also this interesting comment about a successor to the WebSocket API from the linked Node issue, copy/pasted here for convenience:

FYI everyone: There's a new spec WebSocketStream that is intended to supersede WebSocket in the same way that fetch superseded xhr. The new spec supports backpressure.

If the decision is to add ws to Node.js, then perhaps we should skip the current API and just implement this new one, since backpressure is quite important. https://groups.google.com/a/chromium.org/forum/m/#!msg/blink-dev/X7rWpAkMCyg/j6K7mEEwAgAJ

(We should also wait until this API stabilizes and there is buy-in from other browsers besides Chrome)

jimmycuadra avatar Oct 17 '22 19:10 jimmycuadra

What's the state on this? seems there's been little activity since last year?

luchillo17 avatar Aug 22 '23 18:08 luchillo17

Status update

@luchillo17, I'm not actively working on this task since my attention goes to MSW 2.0 (Fetch API compliance). The latest state of the WebSocket support is accurately reflected in the description of this pull request. The main challenge, and the main risk factor, still remains WebSocket support in Node.js given there's no standard API to create WebSockets in that environment. I am yet to weigh the options and decide what would make sense to the users.

Help is very much appreciated here. If anyone has time, I'd be thankful for you to look into how to approach WebSocket interception in Node.js. Ideally, without relying on third-party like ws, which not every developer may be using. I tried experimenting with net.Socket-based interception to some degree of success but it's too low-level to operate with (see more in #399).

Regarding the priorities, I don't expect to get to this task this year. Bringing WebSocket support is something I'm still dedicated to making but I have only as many hours in a day besides the full-time job, family, and personal life. I do this to the best extent of my availability and I hope you meet this with understanding.

kettanaito avatar Aug 24 '23 08:08 kettanaito

The main challenge, and the main risk factor, still remains WebSocket support in Node.js given there's no standard API to create WebSockets in that environment. I am yet to weigh the options and decide what would make sense to the users.

While still early, it seems like undici had made its WebSocket implementation part of the global Node namespace. As of Node 21.0.0 it looks like only the client is being promoted (if that’s not too strong a word) but there is a server implementation as well.

Let’s assume this migrates from experimental to stable and that Node has an officially supported client and server WebSocket implementation exposed as globals in the core language, as with ’fetch. Would it be a stretch to suggest that if any foundation were to be suitable for msw to build on this would be it?

hugo avatar Nov 08 '23 22:11 hugo

@hugo, that is correct. I'm extremely excited that Node.js is adding the standard WebSocket class. We will support that class exclusively in Node.js as a part of the WebSocket interceptor in MSW. It's simply unfeasible to deal with all the possible custom WebSocket server implementations out there. Neither is that necessary. I expect most implementations to migrate to the standard WebSocket class eventually anyway.

This is extremely good news. As I mentioned, the browser-side WebSocket support is about done (tests missing). Once WebSocket lands in Node.js natively, the Node.js interception will be simplified drastically. Looking forward to that.

kettanaito avatar Nov 09 '23 12:11 kettanaito

Released: v0.26.0 🎉

This has been released in v0.26.0!

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


Predictable release automation by @ossjs/release.

kettanaito avatar Feb 14 '24 16:02 kettanaito