socket.io-parser icon indicating copy to clipboard operation
socket.io-parser copied to clipboard

Expose a higher-level string -> packet utility

Open kettanaito opened this issue 1 year ago • 14 comments
trafficstars

Hi!

Thank you so much for exposing both the SocketIO and EngineIO parsers! I have a feature proposal and I will open it in this repo but feel free to move it to where it makes more sense.

Proposal

Add two new methods called encodeMessage and decodeMessage.

// Takes the raw data and encodes it with both
// engine+socket encoding, producing an encoded
// packet string ready to be sent to a SocketIO server.
function encodeMessage(data: RawData) string

// Takes the encoded socket+engine message
// sent over WebSocket and returns the actual
// data sent in that message.
function decodeMessage(message: string): Array<RawData>

Motivation

I'm working on the WebSocket mocking support in MSW, and I'm finishing the WebSocket class interceptor right now. The interception will work on the WebSocket global class level, which means that the developer will receive encoded messages by SocketIO:

interceptor.on('connection', ({ client }) => {
  // This will fire on each outgoing client event.
  client.on('message', (event) => {
    // On the WebSocket class level, the "event.data"
    // will be the encoded string. Not nice!
    console.log(event.data) // "42["message","hello"]"
  })
})

What's needed here are precisely those two helper functions to help the developer read and write messages that will be understood by SocketIO:

client.on('message', (event) => {
  const data = decodeMessage(event.data)
  // ["hello"]

  if (data[0] === 'hello') {
    // The "server.send()" API is proprietary to the interceptor.
    server.send(encodeMessage('mocked hello'))
  }
})

Solutions I've tried

Well, right now I'm simply implementing those two functions using the socket.io-parser and engine.io-parser packages:

// encodeMessage.js
const encodeMessage = async (data) => {
  return new Promise((resolve) => {
    encodePacket(
      {
        type: 'message', // 4
        data: encoder.encode({
          type: 2,
          /**
           * @noto Not sure if prepending "message"
           * manually is correct. Either encoder doesn't
           * do that though.
           */
          data: ['message', data],
          nsp: '/',
        }),
      },
      true,
      (encodedEngineIoPacket) => {
        resolve(encodedEngineIoPacket)
      }
    )
  })
}
// decodeMessage.js
const decodeMessage = (
  // @ts-expect-error
  encodedEngineIoPacket
) => {
  const decodedEngineIoPacket = decodePacket(encodedEngineIoPacket)

  if (decodedEngineIoPacket.type !== 'message') {
    return
  }
  const decodedSocketIoPacket = decoder['decodeString'](
    decodedEngineIoPacket.data
  )

  if (decodedSocketIoPacket.type !== PacketType.EVENT) {
    return
  }

  return decodedSocketIoPacket.data.slice(1)
}

But I'd hate to ask developers to implement those. Moreover, these functions require some basic understanding of the parsers, their order of execution, and what 4 and 2 stand for in the encoded messages.

"Why don't you add this to your interceptor then?" Because MSW doesn't ship anything third-party-specific. This is a rule. This is a good rule. It has served us for years and it helps us maintain truly agnostic API mocking. I will not see that rule changed.

Thus, I propose these high-level utilities be added to the SocketIO natively. That way, developers would get first-class WebSocket mocking with the right level of convenience when it comes to working with SocketIO clients and servers.

Pull request

I would be willing to implement this change if you find it fitting. Please, let's discuss this and create a better experience for everyone!

kettanaito avatar Jan 31 '24 14:01 kettanaito

I've also noted that when encoding data, I need to add the "message" string to the array of data passed to the encoder. I suspect that's not the right usage, is it? For some reason, combining both encoders doesn't actually push that event name to the data array (just adds 4 for MESSAGE and 2 for EVENT). SocketIO won't understand that kind of encoding, and the event will be ignored.

// Message expected by SocketIO
4 2 ["message", data]
| |   |
^ engine.io-parser
  |   |
  ^ socket.io-parser
      |
      ^ nothing adds this

kettanaito avatar Jan 31 '24 14:01 kettanaito

Since this package doesn't depend on socket.io-engine, I wouldn't introduce this dependency as a part of this change. Perhaps this is a fitting proposal for a standalone package? socket.io-utils? I'm fine with making it a community package but I wanted to her what the maintainers of SocketIO think about this first.

kettanaito avatar Jan 31 '24 17:01 kettanaito

Hi! If I understand correctly, the goal is to be able to create a Socket.IO connection in your browser:

const socket = io("https://example.com");

socket.emit("foo", "abc");

socket.on("bar", () => {
  // ...
});

And handle those events with an interceptor:

interceptor.on("connection", ({ client }) => {
  client.on("foo", (val) => {
    console.log("received:", val);
  });

  client.emit("bar", "123");
});

Is that right?

In that case, couldn't you create a Socket.IO class interceptor instead (maybe as a third-party package?), in order to you hide the internals of the Socket.IO connection to your users?

References:

  • https://socket.io/how-to/build-a-basic-client
  • https://socket.io/docs/v4/socket-io-protocol

darrachequesne avatar Feb 01 '24 07:02 darrachequesne

@darrachequesne, hi! Thanks for the swift reply.

If I understand correctly, the goal is to be able to create a Socket.IO connection in your browser: ... And handle those events with an interceptor:

Yes, that's precisely the case! Since Socket.IO encodes the outgoing messages, the end developer will receive the encoded message in the dispatched MessageEvent.data. This both leaks the Socket.IO implementation details and makes it hard to manipulate those messages.

Why Socket.IO-based interceptor isn't what I'm after

In that case, couldn't you create a Socket.IO class interceptor instead (maybe as a third-party package?)

I'm afraid I'm pursuing a bit larger picture here. I want to support mocking anything running via the standard WebSocket class. This works for Socket.IO but this also works for any other client, which is what I'm after. Choosing the right layer for the interception is crucial.

I have nothing against a package targeted at intercepting Socket.IO specifically. It's just not what I'm building, and not quite the goal I have in mind.

The WebSocket class-based interceptor I've written already supports Socket.IO, given it uses "websocket" transport. What I'm proposing, and what makes much more sense to me as it will, hopefully, to the end developer, is to be able to encode and decode outgoing/incoming messages when using Socket.IO. This really sounds like a task for a middleware of sorts, or a custom parser. I believe that's the right approach to solve the problem and give people the ability to use the same interceptor with their favorite WebSocket client, like Socket.IO.

It would be great to have this experience:

import { encodeMessage, decodeMessage } from 'socket.io-somewhere'
import { WebSocketInterceptor } from '@mswjs/interceptors/WebSocket'

const interceptor = WebSocketInterceptor()

interceptor.on('connection', ({ client, server }) => {
  // Apply the parsers on a per-connection basis
  // since the interceptor will intercept ALL WebSocket connections,
  // and some of those may not be made via a Socket.IO client
  // or heading to a Socket.IO server.
  client.parser = { encodeMessage, decodeMessage }
  server.parser = { encodeMessage, decodeMessage }

  client.on('message', (event) => {
    // Although Socket.IO will encode the outgoing MessageEvent,
    // since it provides a native encoding/decoding functions
    // hooked into the interceptor's client/server APIs,
    // those are applied appropriately, giving you the raw data.
    console.log(event.data) // ["hello"]
  })
})

const ws = io('...', { transports: ['websocket'] })
ws.on('connection', () => {
  ws.send('hello')
})

The API to encode/decode outgoing/incoming messages may be useful outside of Socket.IO as well! I can imagine developers sending JSON over a WebSocket connection, and they could use the custom parser that would parse and stringify that JSON so it's a bit easier to work with data in the interceptor. Socket.IO also supports custom parsers, so if teams are using that, they can plug-in their custom parser for mocks and have a consistent experience when testing and developing their WebSocket APIs.

Once I experimented with the .parser part of this, I quickly learned that this will effectively mean encoding and decoding all messages. The original snippets I posted above will not suffice as they lack some of the parser's logic (setting the right event, correctly parsing outgoing encoded messages likes 40, which just means "OPEN", etc).

The essence of the proposal

In its essence, my proposal comes down to those two functions: encodeMessge and decodeMessage. I'm choosing the "message" in the name because the developer won't be dealing with packets—those will be created internally. Perhaps a few more usage examples would clarify the intended behavior here.

// Accepts a raw data I want to send
// and returns the encoded string accepted
// by Socket.IO.
encodeMessage('hello') // 42["message","hello"]

// Similar for binary data like Blob or ArrayBuffer.
encodeMessage(new Blob(['hello'])) // 42["message",<binary>]

// Decoding accepts the intercepted incoming message
// from the Socket.IO server and returns the raw data
// sent in it. Same for Blob and ArrayBuffer.
decodeMessage('42["message","hello"]') // { type: "message", data: ["hello"] }

I believe decodeMessage should indeed return a Packet so it can be encoded back in the case of internal events like OPEN/PING/CLOSE/etc. That way, they can be intercepted, decoded, analyzed, and then encoded back to be forwarded to the original server.

But as I've mentioned, if those are applied to all events, they must also encode/decode internal events like connect, ping, etc. to provide a proper passthrough and functionality for the WebSocket client connection.

// Socket.IO client emits an OPEN (40) event.
// It gets intercepted, and gets decoded. But since
// it's an internal event, the developer must skip it
// and just forward it to the original Socket.IO server
// so the connection is established correctly.

interceptor.on('connection', ({ client, server }) => {
  server.connect()
  // The "message" listener intercepts ALL outgoing
  // client messages, including internal Socket.IO
  // OPEN/PING/CLOSE/etc. messages.
  client.on('message', (event) => {
    // "event.data" will be decoded here (per proposal).
    // But it may not be the actual data message.
    if (event.data.type !== 'message') {
      server.send(event.data)
      return
    }

    // If intercepted an outgoing client message,
    // access it and, perhaps, send a modified message
    // to the actual server instead.
    if (event.data.data[0] === 'hello') {
      // Since the "server" API of the interceptor also has
      // parser attached, any sent events will be encoded in
      // the format that the Socket.IO server will understand.
      // e.g. 42["message","mocked hello"]
      server.send('mocked hello')
    }
  })
})

Do you think it would be possible to extract and expose that encoding functionality as a standalone set of functions?

kettanaito avatar Feb 01 '24 09:02 kettanaito

Thanks for the explanation :+1:

My only concern is that your users will have:

  • Socket.IO-specific syntax in the client code:
const socket = io("https://example.com");

socket.emit("foo", "abc");

socket.on("bar", () => {
  // ...
});
  • WebSocket-specific syntax in the interceptor code
interceptor.on('connection', ({ client, server }) => {
  client.on("message", (event) => {
    switch (event.data[0]) {
      case "foo":
        // ...
        break;
    }
  })
  
  server.send("bar");
})

Else, a util package sounds good to me. Regarding the implementation, please be aware that the encodeMessage() method may return multiple WebSocket packets, if the payload contains binary:

encodeMessage(new Blob(['hello'])) // '451-["message",{"_placeholder":true,"num":0}]' + blob

Reference: https://socket.io/docs/v4/socket-io-protocol/#format

darrachequesne avatar Feb 01 '24 13:02 darrachequesne

My only concern is that your users will have: Socket.IO-specific syntax in the client code: ... WebSocket-specific syntax in the interceptor code

This is expected. You write your client-side code using whichever tooling you prefer. You intercept WebSocket connections using the interceptor API that's modeled after the WHATWG WebSocket standard. This is a good common ground for the interceptor API to remain client-agnostic (one API regardless of what actual WebSocket client you're using).

Regarding the implementation, please be aware that the encodeMessage() method may return multiple WebSocket packets, if the payload contains binary:

Oh, thanks for letting me know! I suppose, thus the encodePayload handles multiple packets too.

But we get that behavior out of the encoder/decoder out of the box, right? We must only be aware that sometimes 1 raw data results in N number of packets?

kettanaito avatar Feb 01 '24 14:02 kettanaito

Is there any chance to add this utility package as a part of the official Socket.IO toolbelt? That way, it can stay consistent with whichever updates happen to Socket.IO. I don't mind writing this as a community package but I believe it makes more sense to be exposed from Socket.IO. What do you think about this?

kettanaito avatar Feb 01 '24 14:02 kettanaito

regardless of what actual WebSocket client you're using

Please note that the Socket.IO client is not really a WebSocket client, even though it uses a WebSocket connection under the hood.

On top of the WebSocket connection, the user will also have to handle:

client.send('0{"sid":"lv_VI97HAXpY6yYWAAAC","upgrades":[],"pingInterval":25000,"pingTimeout":20000,"maxPayload":1000000}');
  • the heartbeat, which means periodically sending a PING packet to the client:
client.send('2');
client.on('message', (ev) => {
  if (ev.data === '40') {
    client.send('40');
  }
});

Do you think it should be included in the (en|de)codeMessage() methods?

darrachequesne avatar Feb 01 '24 17:02 darrachequesne

@darrachequesne, I see your concerns and I agree with them. Once I gave the full encoder/decoder a try, I quickly realized the interceptor now has to deal with all Socket.IO messages, which is not ideal as it effectively disrupts and re-creates the original event communication (think of the passthrough scenarios when the traffic still goes from client to server but the interceptor is used to observe it).

What I ended up doing is building a package that wraps the underlying interceptor connection classes and manages only encoding/decoding of actual data messages: https://github.com/mswjs/socket.io-utils

I believe this is the way to go. This means that the actual traffic can still be forwarded to the original server as-is (no interference):

// ...in the interceptor.
// This will get raw intercepted (encoded) messages
// and send them to the Socket.IO server as-is.
client.on('message', (event) => server.send(event.data)

While working with messages (and even custom events) becomes the only surface where the encoding/decoding is applied:

// ...in the interceptor.
const io = needAGooNameHere({ client, server })

io.client.on('hello', (data) => {
  // The client has emitted a "hello" event with data.
  // In reality, it's a regular "message" event with the
  // encoded event metadata: 42["hello",...data]
})

// Emitting custom events gets encoded to the correct
// "42" + [eventName,...data] messages and sent to the client
// as if the server has sent them. The same works for the server.
io.client.emit('custom', 'foo')

Implementation

The implementation for the custom encoding and decoding logic is rather straightforward. Please, if you can give this a brief look and let me know about obvious mistakes, I'd be thankful!

My only concern with this approach is that the wrapper has to support Socket.IO features to some extent. While coercing custom events to regular messages is quite simple, things like rooms, namespaces, and broadcasting may put an additional overhead on the wrapper layer.

That being said, I don't think that full feature parity is reasonable right now, neither will it be required by every user. I wonder if we can implement some of those features in the wrapper without re-creating much of the Socket.IO internals?

kettanaito avatar Feb 02 '24 13:02 kettanaito

Also, if you wish to collaborate on this package it would be incredible!

I'm working on the WebSocket mocking in MSW, and having first-class support for Socket.IO would benefit both MSW and Socket.IO users. They would mock, test, develop, and debug WebSocket connections as a part of their end-to-end mocking setup.

kettanaito avatar Feb 02 '24 13:02 kettanaito

Please, if you can give this a brief look and let me know about obvious mistakes, I'd be thankful!

Just a little note: this won't work with custom namespaces (format: 40/chat,{"sid":"test"}, where "chat" is the name of the namespace).

Else, it looks good to me!

My only concern with this approach is that the wrapper has to support Socket.IO features to some extent.

Indeed. Not sure how you can work around this...

I wonder if we can implement some of those features in the wrapper without re-creating much of the Socket.IO internals?

That's a great question! Maybe one can reuse the code of the socket.io package, and just change the underlying plumbing? This would need some investigation :eyes:

darrachequesne avatar Feb 05 '24 10:02 darrachequesne

@darrachequesne, thank you for the input!

Just a little note: this won't work with custom namespaces

So the sid stands for the namespace name (same as nsp)? I wonder what this should be set to in case of the root / namespace?

kettanaito avatar Feb 05 '24 11:02 kettanaito

The sid is the identifier assigned to the session (the socket.id attribute):

  • for the main namespace:
40{"sid":"abcd"}

4              => Engine.IO "message" packet type
0              => Socket.IO "CONNECT" packet type
{"sid": ...}   => payload which includes the session ID

(the namespace is implicitly set to "/")

  • for a custom namespace:
40/chat,{"sid":"abcd"}

4              => Engine.IO "message" packet type
0              => Socket.IO "CONNECT" packet type
/chat,         => the "chat" namespace
{"sid": ...}   => payload which includes the session ID

darrachequesne avatar Feb 05 '24 16:02 darrachequesne

Oh, got it now! Thanks for pointing this out 👍 This falls under the "have to re-implement features" category, I suspect. I don't plan on shipping namespaces and rooms in the initial version of that binding. I can iterate on it later once the folks give it a try.

Let me review what I have and see whether this open issue still makes sense.

kettanaito avatar Feb 05 '24 17:02 kettanaito

I think this can be closed now. Please reopen if needed.

darrachequesne avatar Jul 09 '24 10:07 darrachequesne