Skip to content

Commit

Permalink
ConnectionManager: always allow event queueing while connecting
Browse files Browse the repository at this point in the history
Even if was previously suspended or after a failed connection freshness check

The current behaviour is there so that after you go into suspended, and
starts new connection attempts every 30s, it'll still reject publishes
immediately unless/until it becomes connected again. But this results in
some slightly unintuitive behaviour post-device-wake -- an eg presence
enter might fail milliseconds before the sdk reconnects, because it was
never actually having connectivity issues it was just asleep, and it
would have been fine to queue it. Queueing messages for a few seconds
during a connecting cycle (and then failing them if the attempt fails
and the sdk falls back into the suspended state) wouldn't be so bad.

Made the analagous change to the suspended state as well because if you
just remove the connection.queueevents=false line in the freshness
check, but then it fails that and then can't reconnect and goes into
suspended, it would behave differently from if it goes into suspended
via the suspendTimer.

(TBH I've never liked how we mutate the states struct in this way in
 various places. IMO would be a lot more elegant to have a completely
 frozen states struct, junk the suspendTimer, and decide whether to go
 into suspended just by special-casing disconnected in notifystate,
 checking how long it's been since we were last connected, and
 substituting suspended if it's been over 2 minutes, or something like
 that. But that's a change for another time.)
  • Loading branch information
SimonWoolf committed Jul 28, 2022
1 parent 546f482 commit 8f4e3bf
Showing 1 changed file with 0 additions and 3 deletions.
3 changes: 0 additions & 3 deletions src/common/lib/transport/connectionmanager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,6 @@ class ConnectionManager extends EventEmitter {
);
this.clearConnection();
this.states.connecting.failState = 'suspended';
this.states.connecting.queueEvents = false;
}
}

Expand Down Expand Up @@ -1434,7 +1433,6 @@ class ConnectionManager extends EventEmitter {
'requesting new state: suspended'
);
this.states.connecting.failState = 'suspended';
this.states.connecting.queueEvents = false;
this.notifyState({ state: 'suspended' });
}
}, this.connectionStateTtl);
Expand All @@ -1446,7 +1444,6 @@ class ConnectionManager extends EventEmitter {

cancelSuspendTimer(): void {
this.states.connecting.failState = 'disconnected';
this.states.connecting.queueEvents = true;
if (this.suspendTimer) {
clearTimeout(this.suspendTimer as number);
this.suspendTimer = null;
Expand Down

0 comments on commit 8f4e3bf

Please sign in to comment.