ws icon indicating copy to clipboard operation
ws copied to clipboard

Support sending Blob

Open kettanaito opened this issue 1 year ago • 13 comments

Is there an existing issue for this?

  • [X] I've searched for any related issues and avoided creating a duplicate issue.

Description

Currently, ws doesn't accept Blob as an argument to the .send() function:

wss.on('connection', (ws) => ws.send(new Blob(['hello']))

I believe this is mainly for historic reasons because there used to be no Blob in Node.js. There is now, and it would be great to align the sending (and receiving also) with the WHATWG WebSocket standard.

ws version

8.16.0

Node.js Version

v20.11.0

System

Irrelevant.

Expected result

ws supports sending Blob from the server.

Actual result

  • Type error on the unknown input type to .send();
  • A runtime TypeError:
TypeError: The first argument must be of type string or an instance of Buffer, ArrayBuffer, or Array or an Array-like Object. Received an instance of Blob

Attachments

No response

kettanaito avatar Mar 12 '24 12:03 kettanaito

It is not for historic reasons, see https://github.com/nodejs/undici/pull/1795#discussion_r1038928224.

lpinca avatar Mar 12 '24 18:03 lpinca

This is an important context, thanks for sharing! So, Blobs being written first is a Node.js thing?

This is also interesting:

In practice, even though it requires explicitly setting binaryType, 97% of messages are received as ArrayBuffers.

Especially given that the default binaryType value for blob. I believe Undici did end up supporting sending Blob from the client (at least, I seem to be able to do that in tests).

kettanaito avatar Mar 12 '24 18:03 kettanaito

So, Blobs being written first is a Node.js thing?

Yes, because there is no official API to get an ArrayBuffer synchronously from a Blob and streams do not support Blobs natively.

I believe Undici did end up supporting sending Blob from the client (at least, I seem to be able to do that in tests).

I think it is affected by the issue I wrote in that comment.

lpinca avatar Mar 12 '24 19:03 lpinca

See also https://github.com/nodejs/undici/pull/1795#issuecomment-1353535714.

lpinca avatar Mar 12 '24 19:03 lpinca

Got it. Thanks for the great references! Do you consider this proposal to be out of scope? We should close it then.

kettanaito avatar Mar 12 '24 22:03 kettanaito

It is doable but in my opinion it is not worth it. Added complexity for little to no benefit.

lpinca avatar Mar 13 '24 20:03 lpinca

What about supporting this only on the surface level? Accepting the Blob but converting it internally to nodebuffer and proceeding as ws does now? If that makes any sense, of course (would love to learn if it doesn't).

My main use case is showcasing ws for testing, and it's not great to omit sending/receiving Blob data.

kettanaito avatar Mar 14 '24 18:03 kettanaito

Accepting the Blob but converting it internally to nodebuffer and proceeding as ws does now?

It is not trivial because again, converting a Blob to an ArrayBuffer/Buffer synchronously is not supported by Node.js. We could pause the Sender like we do when compressing data but I'm not very happy with it. Also what about support for Node.js versions where Blob is not supported?

It is a lot easier to convert the Blob before calling websocket.send().

websocket.send(await blob.arrayBuffer());

lpinca avatar Mar 14 '24 18:03 lpinca

It is not trivial because again, converting a Blob to an ArrayBuffer/Buffer synchronously is not supported by Node.js.

I keep missing this bit. Got it.

Also what about support for Node.js versions where Blob is not supported?

Blob is supported since Node.js 18, which by itself reaches the maintenance mode in a few months. Does ws support EOL versions of Node.js (16 and older)? If anything, I see this as a motivation to drop those old versions but you would know far better than me here.

kettanaito avatar Mar 14 '24 18:03 kettanaito

Yes, ws still supports Node.js 10.

lpinca avatar Mar 14 '24 18:03 lpinca

It is a lot easier to convert the Blob before calling websocket.send().

I understand this. What I'm proposing is not a matter of convenience but expectations. As in, valid WebSocket usage as per spec should work. Sending a Blob is a valid usage. Not supporting it is a limitation of ws (as the immediate agent; the issue runs deeper as we've discussed) so it may be a good thing to try to find a solution/compromise here.

I also understand that ws doesn't promise full WHATWG compliance as it's a server library and the spec has nothing about servers, afaik. I'm coming purely from the client usage. If a WebSocket client, like Undici, sends a Blob, it's okay if ws receives and handles it as something else. The issue is that the client will never receive a Blob, even if its binaryType = 'blob' (I will double-check on this but I recall that being the outcome of my testing).

kettanaito avatar Mar 14 '24 18:03 kettanaito

Strictly following the WHATWG specification is a non-goal for ws.

lpinca avatar Mar 14 '24 19:03 lpinca

The issue is that the client will never receive a Blob, even if its binaryType = 'blob' (I will double-check on this but I recall that being the outcome of my testing)

That is correct for ws. That binary type is not supported. Receiving a Blob is easier as it can be created synchronously from an ArrayBuffer but if we allow receiving it we should also allow sending it.

lpinca avatar Mar 14 '24 19:03 lpinca

Thank you for your work on this! 💪

kettanaito avatar Jul 03 '24 13:07 kettanaito