botpress / v12

Botpress OSS – v12
https://v12.botpress.com
GNU Affero General Public License v3.0
68 stars 83 forks source link

[BUG] "Sticky Sessions" should not be required with websockets #1177

Closed hacheybj closed 2 years ago

hacheybj commented 3 years ago

Describe the bug Sticky sessions are required to be able to establish a Websocket connection

To Reproduce 2+ Botpress nodes running behind a 1/1 round robin load balancer (of any kind), with Websockets enabled and no sticky sessions configured.

Try to use the webchat.

Connection does not establish during the "handshake"

Expected behavior The connection to be established, and be able to chat.

Screenshots N/A

Environment (please complete the following information): N/A

Additional context Socket IO documentation: https://socket.io/docs/v3/using-multiple-nodes/index.html

JustusNBB commented 3 years ago

As I understand the socket.io docs, the handshake failure only happens with polling enabled!

Solution proposal: Disable socket.io longpolling by default, and only enable it by env var - document the env var to require sticky sessions.

   transports: !ENV_POLLING ? ["websocket"] : ["polling", "websocket"]

Note: this is a breaking change - socket.io will no longer try to establish a polling connection by default anymore, and if enabled only as a fallback. https://socket.io/docs/v3/server-api : The order of the transports array is important. By default, a long-polling connection is established first, and then upgraded to WebSocket if possible. Using ['websocket'] means there will be no fallback if a WebSocket connection cannot be opened.

Related: https://github.com/botpress/v12/issues/1079 https://github.com/botpress/v12/issues/1122 https://forum.botpress.com/t/not-sending-messages-on-ubuntu-server-websocket-handshake/2190

If I get the last one right, this should be a workaround: Open botpress.config.json and set the configuration httpServer.socketTransports = ['websocket']

JustusNBB commented 1 year ago

I just noticed that the production checklist was updated and wanted to leave a thumbs up for the new docs with reference to socket.io v4! image