Improve reconnection under bad network conditions (#811)

* prefer timer instead of boolean * reattach logs to trace level * reduce logs * reset socket status on reconnect * hangup the connection after detach timeout * add leaveReason to BaseConnection * set leaveReason on timeout * update types for room.left * add room.left listener in heroku demo * changeset * RESUME_TIMEOUT to 12s * restore commented code
signalwire · Jun 21, 2023 · f3711f1 · f3711f1
1 parent 29acde3
commit f3711f1
Show file tree

Hide file tree

Showing 13 changed files with 54 additions and 15 deletions.
diff --git a/.changeset/honest-tigers-promise.md b/.changeset/honest-tigers-promise.md
@@ -0,0 +1,7 @@
+---
+'@signalwire/webrtc': patch
+'@signalwire/core': patch
+'@signalwire/js': patch
+---
+
+Improve reconnection under bad network conditions.
diff --git a/internal/playground-js/src/heroku/index.js b/internal/playground-js/src/heroku/index.js
@@ -280,6 +280,9 @@ window.connect = () => {
  console.debug('>> destroy')
  restoreUI()
  })
+ roomObj.on('room.left', (payload) => {
+ console.debug('>> room.left', payload)
+ })
  roomObj.on('room.updated', (params) =>
  console.debug('>> room.updated', params)
  )

diff --git a/packages/core/src/BaseSession.ts b/packages/core/src/BaseSession.ts
@@ -369,6 +369,8 @@ export class BaseSession {
  protected async _onSocketOpen(event: Event) {
  this.logger.debug('_onSocketOpen', event.type)
  try {
+ // Reset to "unknown" in case of reconnect
+ this._status = 'unknown'
  this._clearTimers()
  await this.authenticate()
  this._status = 'connected'
@@ -399,7 +401,6 @@ export class BaseSession {
  if (this._status !== 'disconnected') {
  this._status = 'reconnecting'
  this.dispatch(sessionReconnectingAction())
- // yield put(pubSubChannel, sessionReconnectingAction())
  this._clearTimers()
  this._clearPendingRequests()
  this._reconnectTimer = setTimeout(() => {
@@ -588,6 +589,7 @@ export class BaseSession {
  status === 'disconnected' ? 'unauthorized' : 'unknown'
  )
  )
+ this._removeSocketListeners()
  this.destroySocket()
  this._checkCurrentStatus()
  }

diff --git a/packages/core/src/redux/rootSaga.ts b/packages/core/src/redux/rootSaga.ts
@@ -165,7 +165,7 @@ export function* sessionStatusWatcher(options: StartSagaOptions): SagaIterator {
  sessionForceCloseAction.type,
  ])
 
- getLogger().debug('sessionStatusWatcher', action.type, action.payload)
+ getLogger().trace('sessionStatusWatcher', action.type, action.payload)
  switch (action.type) {
  case authSuccessAction.type: {
  const { session, pubSubChannel } = options

diff --git a/packages/core/src/types/index.ts b/packages/core/src/types/index.ts
@@ -49,6 +49,9 @@ export interface BaseConnectionContract<
  /** @internal The BaseConnection options */
  readonly options: Record<any, any>
 
+ /** @internal */
+ readonly leaveReason: 'RECONNECTION_ATTEMPT_TIMEOUT' | undefined
+
  /** The id of the video device, or null if not available */
  readonly cameraId: string | null
  /** The label of the video device, or null if not available */

diff --git a/packages/core/src/types/videoRoomSession.ts b/packages/core/src/types/videoRoomSession.ts
@@ -1,4 +1,4 @@
-import type { SwEvent } from '.'
+import type { BaseConnectionContract, SwEvent } from '.'
 import type {
  CamelToSnakeCase,
  EntityUpdated,
@@ -26,6 +26,10 @@ export type RoomJoined = 'room.joined'
 export type RoomLeft = 'room.left'
 export type RoomAudienceCount = 'room.audienceCount'
 
+export type RoomLeftEventParams = {
+ reason?: BaseConnectionContract<any>['leaveReason']
+}
+
 export type VideoRoomAudienceCountEventNames = ToInternalVideoEvent<
  InternalRoomAudienceCount | RoomAudienceCount
 >

diff --git a/packages/js/src/JWTSession.ts b/packages/js/src/JWTSession.ts
@@ -40,7 +40,7 @@ export class JWTSession extends BaseJWTSession {
 
  const key = this.getProtocolSessionStorageKey()
  if (key) {
- this.logger.info('Hijacking: search protocol for', key)
+ this.logger.trace('Hijacking: search protocol for', key)
  return getStorage()?.getItem(key) ?? ''
  }
  return ''
@@ -53,7 +53,7 @@ export class JWTSession extends BaseJWTSession {
 
  const key = this.getProtocolSessionStorageKey()
  if (key) {
- this.logger.info('Hijacking: persist protocol', key, this.relayProtocol)
+ this.logger.trace('Hijacking: persist protocol', key, this.relayProtocol)
  getStorage()?.setItem(key, this.relayProtocol)
  }
  }
@@ -75,14 +75,14 @@ export class JWTSession extends BaseJWTSession {
 
  const key = this.getAuthStateSessionStorageKey()
  if (key) {
- this.logger.info('Hijacking: persist auth state', key, state)
+ this.logger.trace('Hijacking: persist auth state', key, state)
  getStorage()?.setItem(key, state)
  }
  }
 
  protected override _onSocketClose(event: CloseEvent) {
  if (this.status === 'unknown') {
- this.logger.info('Hijacking: invalid values - cleaning up storage')
+ this.logger.trace('Hijacking: invalid values - cleaning up storage')
  const protocolKey = this.getProtocolSessionStorageKey()
  if (protocolKey) {
  getStorage()?.removeItem(protocolKey)

diff --git a/packages/js/src/RoomSession.ts b/packages/js/src/RoomSession.ts
@@ -169,7 +169,7 @@ export const RoomSession = function (roomOptions: RoomSessionOptions) {
  // WebRTC connection left the room.
  room.once('destroy', () => {
  // @ts-expect-error
- room.emit('room.left')
+ room.emit('room.left', { reason: room.leaveReason })
 
  // Remove callId to reattach
  reattachManager.destroy()

diff --git a/packages/js/src/RoomSessionDevice.ts b/packages/js/src/RoomSessionDevice.ts
@@ -4,6 +4,7 @@ import {
  BaseConnectionContract,
  BaseConnectionState,
  RoomLeft,
+ RoomLeftEventParams,
 } from '@signalwire/core'
 import { BaseConnection, MediaEvent } from '@signalwire/webrtc'
 import { RoomSessionDeviceMethods } from './utils/interfaces'
@@ -12,7 +13,7 @@ type RoomSessionDeviceEventsHandlerMap = Record<
  BaseConnectionState,
  (params: RoomSessionDevice) => void
 > &
- Record<RoomLeft, (params: void) => void> &
+ Record<RoomLeft, (params?: RoomLeftEventParams) => void> &
  Record<MediaEvent, () => void>
 
 export type RoomSessionDeviceEvents = {

diff --git a/packages/js/src/RoomSessionScreenShare.ts b/packages/js/src/RoomSessionScreenShare.ts
@@ -4,6 +4,7 @@ import {
  BaseConnectionContract,
  BaseConnectionState,
  RoomLeft,
+ RoomLeftEventParams,
 } from '@signalwire/core'
 import { BaseConnection, MediaEvent } from '@signalwire/webrtc'
 import { RoomScreenShareMethods } from './utils/interfaces'
@@ -12,7 +13,7 @@ type RoomSessionScreenShareEventsHandlerMap = Record<
  BaseConnectionState,
  (params: RoomSessionScreenShare) => void
 > &
- Record<RoomLeft, (params: void) => void> &
+ Record<RoomLeft, (params?: RoomLeftEventParams) => void> &
  Record<MediaEvent, () => void>
 
 export type RoomSessionScreenShareEvents = {

diff --git a/packages/js/src/utils/interfaces.ts b/packages/js/src/utils/interfaces.ts
@@ -25,6 +25,7 @@ import type {
  RoomAudienceCount,
  VideoRoomAudienceCountEventParams,
  RoomLeft,
+ RoomLeftEventParams,
  VideoStreamEventNames,
  RoomSessionStream,
  RoomJoined,
@@ -121,7 +122,7 @@ export type RoomSessionObjectEventsHandlerMap = Record<
  RoomJoined | RoomSubscribed,
  (params: VideoRoomSubscribedEventParams) => void
  > &
- Record<RoomLeft, (params: void) => void> &
+ Record<RoomLeft, (params?: RoomLeftEventParams) => void> &
  Record<MediaEvent, () => void> &
  Record<
  RoomAudienceCount,

diff --git a/packages/webrtc/src/BaseConnection.ts b/packages/webrtc/src/BaseConnection.ts
@@ -100,6 +100,9 @@ export class BaseConnection<EventTypes extends EventEmitter.ValidEventTypes>
  EventTypes & BaseConnectionStateEventTypes
  >
  /** @internal */
+ public leaveReason: BaseConnectionContract<EventTypes>['leaveReason'] =
+ undefined
+ /** @internal */
  public cause: string
  /** @internal */
  public causeCode: string

diff --git a/packages/webrtc/src/RTCPeer.ts b/packages/webrtc/src/RTCPeer.ts
@@ -16,6 +16,8 @@ import {
 import { ConnectionOptions } from './utils/interfaces'
 import { watchRTCPeerMediaPackets } from './utils/watchRTCPeerMediaPackets'
 
+const RESUME_TIMEOUT = 12_000
+
 export default class RTCPeer<EventTypes extends EventEmitter.ValidEventTypes> {
  public uuid = uuid()
 
@@ -25,10 +27,10 @@ export default class RTCPeer<EventTypes extends EventEmitter.ValidEventTypes> {
  private _iceTimeout: any
  private _negotiating = false
  private _processingRemoteSDP = false
- private needResume = false
  private _restartingIce = false
  private _watchMediaPacketsTimer: ReturnType<typeof setTimeout>
  private _connectionStateTimer: ReturnType<typeof setTimeout>
+ private _resumeTimer?: ReturnType<typeof setTimeout>
  private _mediaWatcher: ReturnType<typeof watchRTCPeerMediaPackets>
  /**
  * Both of these properties are used to have granular
@@ -279,7 +281,7 @@ export default class RTCPeer<EventTypes extends EventEmitter.ValidEventTypes> {
 
  triggerResume() {
  this.logger.info('Probably half-open so force close from client')
- if (this.needResume) {
+ if (this._resumeTimer) {
  this.logger.info('[skipped] Already in "resume" state')
  return
  }
@@ -289,12 +291,18 @@ export default class RTCPeer<EventTypes extends EventEmitter.ValidEventTypes> {
  // @ts-expect-error
  this.call.emit('media.reconnecting')
  this.clearTimers()
- this.needResume = true
+ this._resumeTimer = setTimeout(() => {
+ this.logger.warn('Disconnecting due to RECONNECTION_ATTEMPT_TIMEOUT')
+ // @ts-expect-error
+ this.call.emit('media.disconnected')
+ this.call.leaveReason = 'RECONNECTION_ATTEMPT_TIMEOUT'
+ this.call.setState('hangup')
+ }, RESUME_TIMEOUT) // TODO: read from call verto.invite response
  this.call._closeWSConnection()
  }
 
  private resetNeedResume() {
- this.needResume = false
+ this.clearResumeTimer()
  if (this.options.watchMediaPackets) {
  this.startWatchMediaPackets()
  }
@@ -892,6 +900,7 @@ export default class RTCPeer<EventTypes extends EventEmitter.ValidEventTypes> {
  }
 
  private clearTimers() {
+ this.clearResumeTimer()
  this.clearWatchMediaPacketsTimer()
  this.clearConnectionStateTimer()
  }
@@ -904,6 +913,11 @@ export default class RTCPeer<EventTypes extends EventEmitter.ValidEventTypes> {
  clearTimeout(this._watchMediaPacketsTimer)
  }
 
+ private clearResumeTimer() {
+ clearTimeout(this._resumeTimer)
+ this._resumeTimer = undefined
+ }
+
  private emitMediaConnected() {
  // @ts-expect-error
  this.call.emit('media.connected')