Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Server has websocket.binarytype set to arraybuffer, not nodebuffer #718

Closed
akwirick opened this issue Oct 7, 2023 · 4 comments
Closed

Comments

@akwirick
Copy link

akwirick commented Oct 7, 2023

I saw an issue on my logs where the ws library was complaining about a type mismatch when emitting messages. Confirmed that patching the websocket binarytype to nodebuffer fixes the underlying issues. Basically, since ws is both a browser and node library, we're inadvertently opting in to browser behavior on the server and that's no bueno.

See

this.webSocket.binaryType = 'arraybuffer'
for the line to fix.

I can probably put together a PR next week, but it's Friday night local time and I'm not gonna get to it til after the weekend.

@janthurau
Copy link
Collaborator

@akwirick Can you share the log / error that you were seeing? Which node version do you use?
Havent seen any issues related to this, and I actually have zero idea what that changes means. I just put together a PR and will check now what tests say, but not sure what that is doing :)

@akwirick
Copy link
Author

Here's a snippet from my logs when the binarytype is set to arraybuffer

{"level":50,"time":1696907675863,"pid":20597,"hostname":"Bear","err":{"type":"TypeError","message":"The \"chunk\" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of ArrayBuffer","stack":"TypeError [ERR_INVALID_ARG_TYPE]: The \"chunk\" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of ArrayBuffer\n    at __node_internal_captureLargerStackTrace (node:internal/errors:496:5)\n    at new NodeError (node:internal/errors:405:5)\n    at readableAddChunk (node:internal/streams/readable:285:13)\n    at Readable.push (node:internal/streams/readable:253:10)\n    at WebSocket.message (/home/awirick/src/ai-platform/node_modules/.pnpm/ws@8.14.2/node_modules/ws/lib/stream.js:64:17)\n    at WebSocket.emit (node:events:526:35)\n    at Receiver.receiverOnMessage (/home/awirick/src/ai-platform/node_modules/.pnpm/ws@8.14.2/node_modules/ws/lib/websocket.js:1192:20)\n    at Receiver.emit (node:events:514:28)\n    at Receiver.dataMessage (/home/awirick/src/ai-platform/node_modules/.pnpm/ws@8.14.2/node_modules/ws/lib/receiver.js:545:14)\n    at Receiver.getData (/home/awirick/src/ai-platform/node_modules/.pnpm/ws@8.14.2/node_modules/ws/lib/receiver.js:478:17)\n    at Receiver.startLoop (/home/awirick/src/ai-platform/node_modules/.pnpm/ws@8.14.2/node_modules/ws/lib/receiver.js:167:22)\n    at Receiver._write (/home/awirick/src/ai-platform/node_modules/.pnpm/ws@8.14.2/node_modules/ws/lib/receiver.js:93:10)\n    at writeOrBuffer (node:internal/streams/writable:399:12)\n    at _write (node:internal/streams/writable:340:10)\n    at Writable.write (node:internal/streams/writable:344:10)\n    at Socket.socketOnData (/home/awirick/src/ai-platform/node_modules/.pnpm/ws@8.14.2/node_modules/ws/lib/websocket.js:1286:35)\n    at Socket.emit (node:events:514:28)\n    at addChunk (node:internal/streams/readable:343:12)\n    at readableAddChunk (node:internal/streams/readable:316:9)\n    at Readable.push (node:internal/streams/readable:253:10)\n    at TCP.onStreamRead (node:internal/stream_base_commons:190:23)","code":"ERR_INVALID_ARG_TYPE"},"msg":"The \"chunk\" argument must be of type string or an instance of Buffer or Uint8Array. Received an instance of ArrayBuffer"}

Basically, the ws library is expecting a Buffer or Uint8Array but is receiving the ArrayBuffer. Theoretically, it should be possible to get ArrayBuffer's to work as well but it really doesn't make sense to not use either nodebuffer or fragment if we are running in a Node environment.

@akwirick
Copy link
Author

For more context, this is running on a fastify server operating in http1.1 mode. Everything else seems to work fine there, just this one little unexpected type issue.

@janthurau
Copy link
Collaborator

ah, this probably happened because you are using ws@^8 already, while we were still at ^7. Just weird that I didn't see this, because I actually upgraded this week 🤔 anyway, will merge it. thx!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants