search

Atmosphere / atmosphere-javascript

atmosphere-javascript
Apache License 2.0
122 stars 98 forks source link

Websocket reconnect causes huge problems with tomcat #263

Closed danielsjolin closed 4 years ago

danielsjolin commented 4 years ago

Description When atmosphere.js tries to reconnect a websocket it does not pass along the usual headers in the reconnect attempt. This causes a weird behaviour where atmosphere-runtime accepts the request and schedules it in Heartbeatinterceptor, whereas tomcat doesn't see it as an upgrade request and therefore keeps the request/response objects in a pool for later usage.

The reconnect happens inside _websocket.onerror function:

if (_request.curWebsocketErrorRetries++ < _request.maxWebsocketErrorRetries) {
   _reconnect(_websocket, _request, _request.pollingInterval);

The effect is the same as seen in https://github.com/Atmosphere/atmosphere/issues/2143 but the cause is different. This is not because of misconfigured atmosphere - it can happen when the server is under heavy load or if there's network issues. I've reproduced this issue using Meteor PubSub from atmosphere-samples, see reproducing.

In the first websocket connect attempt, atmosphere sends the usual HTTP headers to the server:

Request: /atmosphere-meteor-pubsub/pubsub/bertil?X-Atmosphere-tracking-id=0&X-Atmosphere-Framework=3.0.5-javascript&X-Atmosphere-Transport=websocket&X-atmo-protocol=true

=== MimeHeaders ===
host = localhost:8080
connection = Upgrade
pragma = no-cache
cache-control = no-cache
user-agent = Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/85.0.4183.83 Safari/537.36
upgrade = websocket
origin = http://localhost:8080
sec-websocket-version = 13
accept-encoding = gzip, deflate, br
accept-language = en-US,en;q=0.9,sv;q=0.8
sec-websocket-key = y78amtC5JpBDqo9tV+/t/Q==
sec-websocket-extensions = permessage-deflate; client_max_window_bits

The reconnect attempt however does not contain the expected headers

Request: /atmosphere-meteor-pubsub/pubsub/bertil?X-Atmosphere-tracking-id=0&X-Atmosphere-Framework=3.0.5-javascript&X-Atmosphere-Transport=websocket&X-atmo-protocol=true&_=1599482971412

=== MimeHeaders ===
host = localhost:8080
connection = keep-alive
user-agent = Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/85.0.4183.83 Safari/537.36
accept = */*
sec-fetch-site = same-origin
sec-fetch-mode = cors
sec-fetch-dest = empty
referer = http://localhost:8080/atmosphere-meteor-pubsub/
accept-encoding = gzip, deflate, br
accept-language = en-US,en;q=0.9,sv;q=0.8

Atmosphere Info

Reproducing It's easy to simulate a hogged server or network issue by stopping the first websocket upgrade request in a breakpoint.

What usually happens is strange messages being published where the content is parts of the published message and the letter X. See screenshots where messages even leaks through to another browser (subscribed to some other topic)

Expected behavior The correct headers must be present when reconnecting the websocket

Screenshots Firefox: Atmosphere_gone_wild

Chrome: Atmosphere_gone_wild_2

Systems

Additional context The issue has been present since https://github.com/Atmosphere/atmosphere-javascript/pull/245

jfarcand commented 4 years ago

@rehnberg Great analysis. Let me investigate once I have a chance....if you have the cycle to find a fix, let me know and I will cut a new release.

jfarcand commented 4 years ago

@rehnberg Can you try this one and let me know if that fixes the issue

danielsjolin commented 4 years ago

@jfarcand It fixes the issue but introduces another.

The reconnect is now identical with the first websocket attempt, which eliminates the shared Response objects in Tomcat. Everythings good that far. However the client connects with the fallback protocol before trying the websocket again. My browser ends up with an open Streaming connection AND an open WS.

Seen in browser: Atmosphere_dual_reconnect

Captured on server:

/atmosphere-meteor-pubsub/pubsub/test?X-Atmosphere-tracking-id=0&X-Atmosphere-Framework=3.0.5-javascript&X-Atmosphere-Transport=websocket&X-atmo-protocol=true
=== MimeHeaders ===
host = localhost:8080
connection = Upgrade
pragma = no-cache
cache-control = no-cache
user-agent = Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/85.0.4183.83 Safari/537.36
upgrade = websocket
origin = http://localhost:8080
sec-websocket-version = 13
accept-encoding = gzip, deflate, br
accept-language = en-US,en;q=0.9,sv;q=0.8
sec-websocket-key = uQpLrCxr+N3eNqJBl2W1pQ==
sec-websocket-extensions = permessage-deflate; client_max_window_bits

/atmosphere-meteor-pubsub/pubsub/test?X-Atmosphere-tracking-id=0&X-Atmosphere-Framework=3.0.5-javascript&X-Atmosphere-Transport=streaming&X-atmo-protocol=true&_=1599802523274
=== MimeHeaders ===
host = localhost:8080
connection = keep-alive
pragma = no-cache
cache-control = no-cache
user-agent = Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/85.0.4183.83 Safari/537.36
accept = */*
sec-fetch-site = same-origin
sec-fetch-mode = cors
sec-fetch-dest = empty
referer = http://localhost:8080/atmosphere-meteor-pubsub/
accept-encoding = gzip, deflate, br
accept-language = en-US,en;q=0.9,sv;q=0.8

/atmosphere-meteor-pubsub/pubsub/test?X-Atmosphere-tracking-id=0&X-Atmosphere-Framework=3.0.5-javascript&X-Atmosphere-Transport=websocket&X-atmo-protocol=true
=== MimeHeaders ===
host = localhost:8080
connection = Upgrade
pragma = no-cache
cache-control = no-cache
user-agent = Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/85.0.4183.83 Safari/537.36
upgrade = websocket
origin = http://localhost:8080
sec-websocket-version = 13
accept-encoding = gzip, deflate, br
accept-language = en-US,en;q=0.9,sv;q=0.8
sec-websocket-key = vA8Ml7h6OmuWIYoMPZC4KQ==
sec-websocket-extensions = permessage-deflate; client_max_window_bits
inad9300 commented 4 years ago

Doesn't https://github.com/Atmosphere/atmosphere-javascript/commit/7e7788b8d96cb587b945ed06fa95a9700afeb8f2#diff-db9ba3b79d60117ac320363bcb013ec3 break the reconnection logic altogether? I've been experiencing problems lately with automatic reconnections... As well as with missed open and closed events.

Edit I confirm that rolling back to older versions of Atmosphere makes things better – honestly, the farther back I go, the better it works. Here is a bit of a summary (I followed the same procedure at least twice with all version combinations): atmosphere-runtime atmosphere.js Result
2.6.3 3.0.6 2 reconnection attempts are made, then they stop despite maxReconnectOnClose: Number.MAX_VALUE
2.6.1 3.0.5 An indefinite number of reconnection attempts are made in Firefox, as configured. Chrome stops after a few and never manages to reconnect
2.5.13 2.3.8 Both Firefox and Chrome keep trying to reconnect forever and eventually manage when I bring things back online
jfarcand commented 4 years ago

maxReconnectOnClose is not what you want to set. It's maxWebsocketErrorRetries if the websocket isn't closed properly by the server. @inad9300 Can you give more info on what you are doing exactly?

inad9300 commented 4 years ago

@jfarcand could you link to documentation talking about maxWebsocketErrorRetries?

As for what I'm doing, I basically have a Spring back end and Vue.js front end. During development, I use vue-cli-service to run the front end, which starts a simple HTTP server which serves the front end files and forwards back end requests to the Spring server. I regularly restart both front end and back end servers to evaluate reconnection behaviour. The table above summarizes my findings when stopping and starting the front end server specifically.

henriAbel commented 3 years ago

Just spent hour debugging why client is not reconnecting. There is no mentioning maxWebsocketErrorRetries in any of the samples or documentation.