Skip to content

Commit

Permalink
Always reconnect after connection closes
Browse files Browse the repository at this point in the history
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:
phoenixframework@470337d

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
  • Loading branch information
laurglia committed Aug 24, 2022
1 parent 3eb2628 commit e263501
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 4 deletions.
3 changes: 1 addition & 2 deletions assets/js/phoenix/socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
4 changes: 2 additions & 2 deletions assets/test/socket_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})


Expand Down

0 comments on commit e263501

Please sign in to comment.