Support large body request + thread safer clients count per worker + separation between regular http requests to WS

[MatanYemini]

Should solve: https://github.com/socketio/socket.io-sticky/issues/6

This PR includes:

• Handling of large body requests (using "data" events)
• Separation between WS to regular http requests to keep main logic for WS the same
• Thread safer client count

[darrachequesne]

Hi! Thanks a lot for your work on this :+1:

It seems the tests are timing out, could you please check? https://github.com/socketio/socket.io-sticky/actions/runs/3336290143/jobs/5521261400

[MatanYemini]

Sure, will also make sure no memory leaks. (need to validate the passthrough is closed correctly without leftovers)

[volnyansky]

Hi! Thanks a lot for your work on this 👍

It seems the tests are timing out, could you please check? https://github.com/socketio/socket.io-sticky/actions/runs/3336290143/jobs/5521261400

@darrachequesne I've fixed tests. Websocket connection detection was not perferct.

[MatanYemini]

In particular, the original PR was not supporting transport=polling. @volnyansky Added this commit ♥

[volnyansky]

@darrachequesne Node 10 tests also fixed

[darrachequesne]

I've added a few tests in https://github.com/socketio/socket.io-sticky/commit/bb47fc4c22934ee4a7b8174891b778d141cda271 but the HTTP long-polling only test fails with this PR and I don't understand why. Could you please check?

[volnyansky]

@darrachequesne changed socket.io connection detection, now check is valid only for GET requests. POST requests in polling mode are sent through a new mechanic. Your new tests works for me,

[volnyansky]

@darrachequesne there is strange behavior, polling requests work with a buffer size bellow 8*1e5 bytes.

[MatanYemini]

@volnyansky I think there is some kind of limitation in socket io. Not remember clearly, but I think this is indeed the default max buffer. We can control the size via - maxHttpBufferSize will check this a bit later (hopefully today).

Yeah - https://socket.io/docs/v4/server-options/#maxhttpbuffersize I was right... limit of 1MB

[abhemanyus]

This is corrupting the multipart request right now.

dev-aru-1  | 13 - - - [Sat, 29 Oct 2022 06:42:18 GMT] "POST /apis/v1/document/create HTTP/1.0" 500 - "-" "insomnia/2022.5.1"
dev-aru-1  | Error: Unexpected end of multipart data
dev-aru-1  |     at /app/node_modules/dicer/lib/Dicer.js:62:28
dev-aru-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:77:11)
dev-aru-1  | /app/node_modules/@socket.io/sticky/index.js:218
dev-aru-1  |         tunnel.write(data);
dev-aru-1  |                ^
dev-aru-1  | 
dev-aru-1  | TypeError: Cannot read properties of undefined (reading 'write')
dev-aru-1  |     at process.<anonymous> (/app/node_modules/@socket.io/sticky/index.js:218:16)
dev-aru-1  |     at process.emit (node:events:525:35)
dev-aru-1  |     at emit (node:internal/child_process:937:14)
dev-aru-1  |     at process.processTicksAndRejections (node:internal/process/task_queues:83:21)
dev-aru-1  | 
dev-aru-1  | Node.js v18.11.0
dev-aru-1  | Worker 13 died
dev-aru-1  | Server started 57

Edit: Weirdly enough, using the socketio:main library, the connection times out when using nginx as a reverse proxy/ssl offloader, but works fine without it in the way.

[darrachequesne]

@MatanYemini you are right, the maxHttpBufferSize must be increased to 1.4 * 1e6, to account for the base64 encoding of the buffer with HTTP long-polling.

That being said, I'm not sure about this solution, because all HTTP chunks goes through the master process... What do you think?

[volnyansky]

@darrachequesne Fixed for node v12 and later:

• refactored work with streams
• added one more test for large outgoing package
• refactored tests
• setMacHttpBufferSize to 2*1e6

[volnyansky]

@abhemanyus Can you add test with multpipart/form-data

[abhemanyus]

@abhemanyus Can you add test with multpipart/form-data

Here you go https://github.com/abhemanyus/socket.io-sticky/commit/1704b60c72f8423ae4af357697b6f2767d50b590

[MatanYemini]

@abhemanyus @volnyansky Thanks for the updates guys, commits look good to me! Will try to wrap this tomorrow / Tuesday.

[darrachequesne]

Merged as https://github.com/socketio/socket.io-sticky/commit/a124d0beb5c1b78be5b75f10153859a9e4672862. Thanks a lot for your work on this :heart: