Skip to content

Commit

Permalink
fix(bbb-html5): customHeartbeat would not close stale sessions, + (bi…
Browse files Browse the repository at this point in the history
…gbluebutton#19017)

* fix(bbb-html5): customHeartbeat would not close stale sessions, +

The [disabled by default] custom heartbeat included in Meteor's server
does not end connections when they are considered unhealthy/stale, which
deviates a bit from the default implementation. See:
bigbluebutton#11486.

This commit includes a call to the default heartbeat termination timeout
so sockets are correctly cleaned up when the custom heartbeat is
activated. It also adds a customHeartbeatUseDataFrames config to allow
controlling whether the custom heartbeat should use WS data frames as
valid heartbeats as well - this should only be useful for
testing/debugging purposes and the default behavior (true) is
maintained.

As a side note: this change spun off from an investigation where some
problematic networks were triggering periodic client re-connects due to
the default heartbeat failing. Investigation points to the control
frames being put alongside fragmented WS data frames and the server side
failing to recognize the former - which means pong frames would be missed and
the health check would fail. Since the default heartbeat _does not_
account for data frame traffic (eg DDP payloads), it would shut down the
client's WS even though it was healthy.
The custom heartbeat _does_ account for data frames, which mitigates
that scenario and prevents unecessary reconnections.

* fix(bbb-html5): frontend crash due to undefined vars in customHeartbeat

Meteor frontends may crash when customHeartbeat is enabled
due to an undefined access in the heartbeat`s logger.

Add optional chaining to the session props access so it won`t crash and tune down some log levels around that area.
  • Loading branch information
antobinary committed Mar 15, 2024
1 parent 4df5a5d commit f5a70cd
Showing 1 changed file with 3 additions and 3 deletions.
6 changes: 3 additions & 3 deletions bigbluebutton-html5/imports/startup/server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ Meteor.startup(() => {
// Skipping heartbeat, because websocket is sending data
if (currentTime - this.ws.lastSentFrameTimestamp < 10000) {
try {
Logger.info('Skipping heartbeat, because websocket is sending data', {
Logger.debug('Skipping heartbeat, because websocket is sending data', {
currentTime,
lastSentFrameTimestamp: this.ws.lastSentFrameTimestamp,
userId: this.session.connection._meteorSession.userId,
userId: this.session?.connection?._meteorSession?.userId,
});
return;
} catch (err) {
Expand All @@ -102,7 +102,7 @@ Meteor.startup(() => {
if (supportsHeartbeats) {
this.hto_ref = setTimeout(() => {
try {
Logger.info('Heartbeat timeout', { userId: this.session.connection._meteorSession.userId, sentAt: currentTime, now: new Date().getTime() });
Logger.warn('Heartbeat timeout', { userId: this.session?.connection?._meteorSession?.userId, sentAt: currentTime, now: new Date().getTime() });
} catch (err) {
Logger.error(`Heartbeat timeout error: ${err}`);
} finally {
Expand Down

0 comments on commit f5a70cd

Please sign in to comment.