Skip to content

Commit

Permalink
Fix heartbeat after disconnect. Closes phoenixframework#4623
Browse files Browse the repository at this point in the history
  • Loading branch information
@chrismccord @laurglia
chrismccord authored and laurglia committed Aug 29, 2022
1 parent 940215e commit 1c8156a
Show file tree
Hide file tree
Showing 8 changed files with 74 additions and 29 deletions.
19 changes: 13 additions & 6 deletions assets/js/phoenix/socket.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ export default class Socket {
this.params = closure(opts.params || {})
this.endPoint = `${endPoint}/${TRANSPORTS.websocket}`
this.vsn = opts.vsn || DEFAULT_VSN
this.heartbeatTimeoutTimer = null
this.heartbeatTimer = null
this.pendingHeartbeatRef = null
this.reconnectTimer = new Timer(() => {
Expand Down Expand Up @@ -317,6 +318,12 @@ export default class Socket {
/**
* @private
*/

clearHeartbeats(){
clearTimeout(this.heartbeatTimer)
clearTimeout(this.heartbeatTimeoutTimer)
}

onConnOpen(){
if(this.hasLogger()) this.log("transport", `connected to ${this.endPointURL()}`)
this.closeWasClean = false
Expand Down Expand Up @@ -344,8 +351,8 @@ export default class Socket {
resetHeartbeat(){
if(this.conn && this.conn.skipHeartbeat){ return }
this.pendingHeartbeatRef = null
clearTimeout(this.heartbeatTimer)
setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs)
this.clearHeartbeats()
this.heartbeatTimer = setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs)
}

teardown(callback, code, reason){
Expand Down Expand Up @@ -397,7 +404,7 @@ export default class Socket {
onConnClose(event){
if(this.hasLogger()) this.log("transport", "close", event)
this.triggerChanError()
clearTimeout(this.heartbeatTimer)
this.clearHeartbeats()
if(!this.closeWasClean){
this.reconnectTimer.scheduleTimeout()
}
Expand Down Expand Up @@ -515,7 +522,7 @@ export default class Socket {
if(this.pendingHeartbeatRef && !this.isConnected()){ return }
this.pendingHeartbeatRef = this.makeRef()
this.push({topic: "phoenix", event: "heartbeat", payload: {}, ref: this.pendingHeartbeatRef})
this.heartbeatTimer = setTimeout(() => this.heartbeatTimeout(), this.heartbeatIntervalMs)
this.heartbeatTimeoutTimer = setTimeout(() => this.heartbeatTimeout(), this.heartbeatIntervalMs)
}

flushSendBuffer(){
Expand All @@ -529,9 +536,9 @@ export default class Socket {
this.decode(rawMessage.data, msg => {
let {topic, event, payload, ref, join_ref} = msg
if(ref && ref === this.pendingHeartbeatRef){
clearTimeout(this.heartbeatTimer)
this.clearHeartbeats()
this.pendingHeartbeatRef = null
setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs)
this.heartbeatTimer = setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs)
}

if(this.hasLogger()) this.log("receive", `${payload.status || ""} ${topic} ${event} ${ref && "(" + ref + ")" || ""}`, payload)
Expand Down
23 changes: 23 additions & 0 deletions assets/test/socket_test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,29 @@ describe("with transports", function(){

assert.ok(!spy.calledWith("phx_error"))
})

it("does not send heartbeat after explicit disconnect", function(done){
let clock = sinon.useFakeTimers()
const spy = sinon.spy(socket, "sendHeartbeat")
socket.onConnOpen()
socket.disconnect()
clock.tick(30000)
assert.ok(spy.notCalled)
clock.restore()
done()
})

it("does not timeout the heartbeat after explicit disconnect", function(done){
let clock = sinon.useFakeTimers()
const spy = sinon.spy(socket, "heartbeatTimeout")
socket.onConnOpen()
socket.disconnect()
clock.tick(30000)
clock.tick(30000)
assert.ok(spy.notCalled)
clock.restore()
done()
})
})

describe("onConnError", function(){
Expand Down
17 changes: 11 additions & 6 deletions priv/static/phoenix.cjs.js
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 2 additions & 2 deletions priv/static/phoenix.cjs.js.map
View file

Large diffs are not rendered by default.

17 changes: 11 additions & 6 deletions priv/static/phoenix.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -926,6 +926,7 @@ var Phoenix = (() => {
this.params = closure(opts.params || {});
this.endPoint = `${endPoint}/${TRANSPORTS.websocket}`;
this.vsn = opts.vsn || DEFAULT_VSN;
this.heartbeatTimeoutTimer = null;
this.heartbeatTimer = null;
this.pendingHeartbeatRef = null;
this.reconnectTimer = new Timer(() => {
Expand Down Expand Up @@ -1024,6 +1025,10 @@ var Phoenix = (() => {
});
return true;
}
clearHeartbeats() {
clearTimeout(this.heartbeatTimer);
clearTimeout(this.heartbeatTimeoutTimer);
}
onConnOpen() {
if (this.hasLogger())
this.log("transport", `connected to ${this.endPointURL()}`);
Expand All @@ -1050,8 +1055,8 @@ var Phoenix = (() => {
return;
}
this.pendingHeartbeatRef = null;
clearTimeout(this.heartbeatTimer);
setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs);
this.clearHeartbeats();
this.heartbeatTimer = setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs);
}
teardown(callback, code, reason) {
if (!this.conn) {
Expand Down Expand Up @@ -1103,7 +1108,7 @@ var Phoenix = (() => {
if (this.hasLogger())
this.log("transport", "close", event);
this.triggerChanError();
clearTimeout(this.heartbeatTimer);
this.clearHeartbeats();
if (!this.closeWasClean) {
this.reconnectTimer.scheduleTimeout();
}
Expand Down Expand Up @@ -1185,7 +1190,7 @@ var Phoenix = (() => {
}
this.pendingHeartbeatRef = this.makeRef();
this.push({ topic: "phoenix", event: "heartbeat", payload: {}, ref: this.pendingHeartbeatRef });
this.heartbeatTimer = setTimeout(() => this.heartbeatTimeout(), this.heartbeatIntervalMs);
this.heartbeatTimeoutTimer = setTimeout(() => this.heartbeatTimeout(), this.heartbeatIntervalMs);
}
flushSendBuffer() {
if (this.isConnected() && this.sendBuffer.length > 0) {
Expand All @@ -1197,9 +1202,9 @@ var Phoenix = (() => {
this.decode(rawMessage.data, (msg) => {
let { topic, event, payload, ref, join_ref } = msg;
if (ref && ref === this.pendingHeartbeatRef) {
clearTimeout(this.heartbeatTimer);
this.clearHeartbeats();
this.pendingHeartbeatRef = null;
setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs);
this.heartbeatTimer = setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs);
}
if (this.hasLogger())
this.log("receive", `${payload.status || ""} ${topic} ${event} ${ref && "(" + ref + ")" || ""}`, payload);
Expand Down
2 changes: 1 addition & 1 deletion priv/static/phoenix.min.js
View file

Large diffs are not rendered by default.

17 changes: 11 additions & 6 deletions priv/static/phoenix.mjs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -897,6 +897,7 @@ var Socket = class {
this.params = closure(opts.params || {});
this.endPoint = `${endPoint}/${TRANSPORTS.websocket}`;
this.vsn = opts.vsn || DEFAULT_VSN;
this.heartbeatTimeoutTimer = null;
this.heartbeatTimer = null;
this.pendingHeartbeatRef = null;
this.reconnectTimer = new Timer(() => {
Expand Down Expand Up @@ -995,6 +996,10 @@ var Socket = class {
});
return true;
}
clearHeartbeats() {
clearTimeout(this.heartbeatTimer);
clearTimeout(this.heartbeatTimeoutTimer);
}
onConnOpen() {
if (this.hasLogger())
this.log("transport", `connected to ${this.endPointURL()}`);
Expand All @@ -1021,8 +1026,8 @@ var Socket = class {
return;
}
this.pendingHeartbeatRef = null;
clearTimeout(this.heartbeatTimer);
setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs);
this.clearHeartbeats();
this.heartbeatTimer = setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs);
}
teardown(callback, code, reason) {
if (!this.conn) {
Expand Down Expand Up @@ -1074,7 +1079,7 @@ var Socket = class {
if (this.hasLogger())
this.log("transport", "close", event);
this.triggerChanError();
clearTimeout(this.heartbeatTimer);
this.clearHeartbeats();
if (!this.closeWasClean) {
this.reconnectTimer.scheduleTimeout();
}
Expand Down Expand Up @@ -1156,7 +1161,7 @@ var Socket = class {
}
this.pendingHeartbeatRef = this.makeRef();
this.push({ topic: "phoenix", event: "heartbeat", payload: {}, ref: this.pendingHeartbeatRef });
this.heartbeatTimer = setTimeout(() => this.heartbeatTimeout(), this.heartbeatIntervalMs);
this.heartbeatTimeoutTimer = setTimeout(() => this.heartbeatTimeout(), this.heartbeatIntervalMs);
}
flushSendBuffer() {
if (this.isConnected() && this.sendBuffer.length > 0) {
Expand All @@ -1168,9 +1173,9 @@ var Socket = class {
this.decode(rawMessage.data, (msg) => {
let { topic, event, payload, ref, join_ref } = msg;
if (ref && ref === this.pendingHeartbeatRef) {
clearTimeout(this.heartbeatTimer);
this.clearHeartbeats();
this.pendingHeartbeatRef = null;
setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs);
this.heartbeatTimer = setTimeout(() => this.sendHeartbeat(), this.heartbeatIntervalMs);
}
if (this.hasLogger())
this.log("receive", `${payload.status || ""} ${topic} ${event} ${ref && "(" + ref + ")" || ""}`, payload);
Expand Down
4 changes: 2 additions & 2 deletions priv/static/phoenix.mjs.map
View file

Large diffs are not rendered by default.

0 comments on commit 1c8156a

Please sign in to comment.