From e26350126377e101a2d192b3b7d17464deffc6cd Mon Sep 17 00:00:00 2001 From: Laur Sisask Date: Tue, 23 Aug 2022 18:32:00 +0300 Subject: [PATCH] Always reconnect after connection closes Phoenix creates a new WebSocket connection if the current one closes for some reason. However, for a new connection to be created, it is required that the following conditions hold: - The connection did not exit cleanly. This is needed to not make the socket reconnect after it is manually disconnected with `Socket#disconnect`. - The WebSocket exit code was not normal (something other than 1000). In some cases, the connection can close with the 1000 code and still require reconnecting. For example, when turning the internet off for around 1 minute, and turning it back on, then the connection sometimes closes with code 1000. You can test that behavior on my test site: http://34.219.19.64:5000/ Here is also a video of the issue reproduced by me: https://www.youtube.com/watch?v=j76SvA3kglQ This commit removes the requirement for the condition that the connection has to close with code 1000 to make reconnections more reliable. I found that the condition was first added in November 2021 with no explanation on why it was needed in the first place: https://github.com/phoenixframework/phoenix/commit/470337db9162284b892202ce8f6018a153aa83fc If there is a good reason why this condition is set in place, do let me know, but otherwise I would go ahead and remove it. The new behavior can be tested on my second test site: http://34.219.19.64:5001/ Vide of the new behavior: https://www.youtube.com/watch?v=q06kPuTln9Q --- assets/js/phoenix/socket.js | 3 +-- assets/test/socket_test.js | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/assets/js/phoenix/socket.js b/assets/js/phoenix/socket.js index 32a02e91ff..6ac4ca4a7f 100644 --- a/assets/js/phoenix/socket.js +++ b/assets/js/phoenix/socket.js @@ -395,11 +395,10 @@ export default class Socket { } onConnClose(event){ - let closeCode = event && event.code if(this.hasLogger()) this.log("transport", "close", event) this.triggerChanError() clearTimeout(this.heartbeatTimer) - if(!this.closeWasClean && closeCode !== 1000){ + if(!this.closeWasClean){ this.reconnectTimer.scheduleTimeout() } this.stateChangeCallbacks.close.forEach(([, callback]) => callback(event)) diff --git a/assets/test/socket_test.js b/assets/test/socket_test.js index 1daa894eed..ce2f51cc1c 100644 --- a/assets/test/socket_test.js +++ b/assets/test/socket_test.js @@ -593,14 +593,14 @@ describe("with transports", function(){ socket.connect() }) - it("does not schedule reconnectTimer if normal close", function(){ + it("schedules reconnectTimer timeout if normal close", function(){ const spy = sinon.spy(socket.reconnectTimer, "scheduleTimeout") const event = {code: 1000} socket.onConnClose(event) - assert.equal(spy.calledOnce, false) + assert.ok(spy.calledOnce) })