From 84b589719f4d1e9d29870ebc0d741822338ec89e Mon Sep 17 00:00:00 2001 From: virtuacoplenny Date: Mon, 2 Jul 2018 14:22:51 -0700 Subject: [PATCH] fix(connection): reload immediately on possible split-brain (#3162) * fix(connection): reload immediately on possible split-brain There isn't an explicit way to know when a split brain scenario has happened. It is assumed it arises when an "item-not-found" connection error is encountered early on in the conference. So, store when a connection has happened so it be calculated how much time has elapsed and if the threshold has not been exceeded then do an immediate reload of the app instead of showing the overlay with a reload timer. * squash: rename isItemNotFoundError -> isShardChangedError --- config.js | 1 + connection.js | 2 +- react/features/analytics/AnalyticsEvents.js | 16 ++++++ react/features/app/actions.js | 27 ++++++++- react/features/base/conference/middleware.js | 56 +++++++++++++++++++ react/features/base/connection/actionTypes.js | 3 +- .../base/connection/actions.native.js | 24 +++++--- react/features/base/connection/reducer.js | 14 +++-- react/features/overlay/actions.js | 27 --------- .../components/AbstractPageReloadOverlay.js | 4 +- .../components/PageReloadOverlay.native.js | 6 +- .../overlay/components/ReloadButton.js | 5 +- 12 files changed, 136 insertions(+), 49 deletions(-) diff --git a/config.js b/config.js index d6f5243731..e39fe5e5c0 100644 --- a/config.js +++ b/config.js @@ -346,6 +346,7 @@ var config = { // List of undocumented settings used in jitsi-meet /** + _immediateReloadThreshold autoRecord autoRecordToken debug diff --git a/connection.js b/connection.js index f9614ff19d..fe725239de 100644 --- a/connection.js +++ b/connection.js @@ -132,7 +132,7 @@ function connect(id, password, roomName) { * */ function handleConnectionEstablished() { - APP.store.dispatch(connectionEstablished(connection)); + APP.store.dispatch(connectionEstablished(connection, Date.now())); unsubscribe(); resolve(connection); } diff --git a/react/features/analytics/AnalyticsEvents.js b/react/features/analytics/AnalyticsEvents.js index 41bdd7576c..cff8fb2005 100644 --- a/react/features/analytics/AnalyticsEvents.js +++ b/react/features/analytics/AnalyticsEvents.js @@ -97,6 +97,22 @@ export function createAudioOnlyChangedEvent(enabled) { }; } +/** + * Creates an event for about the JitsiConnection. + * + * @param {string} action - The action that the event represents. + * @param {boolean} attributes - Additional attributes to attach to the event. + * @returns {Object} The event in a format suitable for sending via + * sendAnalytics. + */ +export function createConnectionEvent(action, attributes = {}) { + return { + action, + actionSubject: 'connection', + attributes + }; +} + /** * Creates an event for an action on the deep linking page. * diff --git a/react/features/app/actions.js b/react/features/app/actions.js index 3455b12249..54ec122b29 100644 --- a/react/features/app/actions.js +++ b/react/features/app/actions.js @@ -12,10 +12,13 @@ import { } from '../base/config'; import { setLocationURL } from '../base/connection'; import { loadConfig } from '../base/lib-jitsi-meet'; -import { parseURIString } from '../base/util'; +import { parseURIString, toURLString } from '../base/util'; +import { setFatalError } from '../overlay'; import { APP_WILL_MOUNT, APP_WILL_UNMOUNT } from './actionTypes'; +const logger = require('jitsi-meet-logger').getLogger(__filename); + declare var APP: Object; /** @@ -266,6 +269,28 @@ export function redirectWithStoredParams(pathname: string) { }; } +/** + * Reloads the page. + * + * @protected + * @returns {Function} + */ +export function reloadNow() { + return (dispatch: Dispatch, getState: Function) => { + dispatch(setFatalError(undefined)); + + const { locationURL } = getState()['features/base/connection']; + + logger.info(`Reloading the conference using URL: ${locationURL}`); + + if (navigator.product === 'ReactNative') { + dispatch(appNavigate(toURLString(locationURL))); + } else { + dispatch(reloadWithStoredParams()); + } + }; +} + /** * Reloads the page by restoring the original URL. * diff --git a/react/features/base/conference/middleware.js b/react/features/base/conference/middleware.js index 974da3d632..6b76681e06 100644 --- a/react/features/base/conference/middleware.js +++ b/react/features/base/conference/middleware.js @@ -1,9 +1,11 @@ // @flow +import { reloadNow } from '../../app'; import { ACTION_PINNED, ACTION_UNPINNED, createAudioOnlyChangedEvent, + createConnectionEvent, createPinnedEvent, sendAnalytics } from '../../analytics'; @@ -194,6 +196,14 @@ function _connectionEstablished({ dispatch }, next, action) { * @returns {Object} The value returned by {@code next(action)}. */ function _connectionFailed({ dispatch, getState }, next, action) { + // In the case of a split-brain error, reload early and prevent further + // handling of the action. + if (_isMaybeSplitBrainError(getState, action)) { + dispatch(reloadNow()); + + return; + } + const result = next(action); // FIXME: Workaround for the web version. Currently, the creation of the @@ -235,6 +245,52 @@ function _connectionFailed({ dispatch, getState }, next, action) { return result; } +/** + * Returns whether or not a CONNECTION_FAILED action is for a possible split + * brain error. A split brain error occurs when at least two users join a + * conference on different bridges. It is assumed the split brain scenario + * occurs very early on in the call. + * + * @param {Function} getState - The redux function for fetching the current + * state. + * @param {Action} action - The redux action {@code CONNECTION_FAILED} which is + * being dispatched in the specified {@code store}. + * @private + * @returns {boolean} + */ +function _isMaybeSplitBrainError(getState, action) { + const { error } = action; + const isShardChangedError = error + && error.message === 'item-not-found' + && error.details + && error.details.shard_changed; + + if (isShardChangedError) { + const state = getState(); + const { timeEstablished } = state['features/base/connection']; + const { _immediateReloadThreshold } = state['features/base/config']; + + const timeSinceConnectionEstablished + = timeEstablished && Date.now() - timeEstablished; + const reloadThreshold = typeof _immediateReloadThreshold === 'number' + ? _immediateReloadThreshold : 1500; + + const isWithinSplitBrainThreshold = !timeEstablished + || timeSinceConnectionEstablished <= reloadThreshold; + + sendAnalytics(createConnectionEvent('failed', { + ...error, + connectionEstablished: timeEstablished, + splitBrain: isWithinSplitBrainThreshold, + timeSinceConnectionEstablished + })); + + return isWithinSplitBrainThreshold; + } + + return false; +} + /** * Notifies the feature base/conference that the action {@code PIN_PARTICIPANT} * is being dispatched within a specific redux store. Pins the specified remote diff --git a/react/features/base/connection/actionTypes.js b/react/features/base/connection/actionTypes.js index db4eb303ac..5ee025140d 100644 --- a/react/features/base/connection/actionTypes.js +++ b/react/features/base/connection/actionTypes.js @@ -15,7 +15,8 @@ export const CONNECTION_DISCONNECTED = Symbol('CONNECTION_DISCONNECTED'); * * { * type: CONNECTION_ESTABLISHED, - * connection: JitsiConnection + * connection: JitsiConnection, + * timeEstablished: number, * } */ export const CONNECTION_ESTABLISHED = Symbol('CONNECTION_ESTABLISHED'); diff --git a/react/features/base/connection/actions.native.js b/react/features/base/connection/actions.native.js index dd2ac8cd3f..0ee82fbf5f 100644 --- a/react/features/base/connection/actions.native.js +++ b/react/features/base/connection/actions.native.js @@ -49,7 +49,7 @@ export type ConnectionFailedError = { /** * The details about the connection failed event. */ - details?: string, + details?: Object, /** * Error message. @@ -126,7 +126,7 @@ export function connect(id: ?string, password: ?string) { connection.removeEventListener( JitsiConnectionEvents.CONNECTION_ESTABLISHED, _onConnectionEstablished); - dispatch(connectionEstablished(connection)); + dispatch(connectionEstablished(connection, Date.now())); } /** @@ -138,16 +138,21 @@ export function connect(id: ?string, password: ?string) { * used to authenticate and the authentication failed. * @param {string} [credentials.jid] - The XMPP user's ID. * @param {string} [credentials.password] - The XMPP user's password. + * @param {Object} details - Additional information about the error. * @private * @returns {void} */ - function _onConnectionFailed( - err: string, msg: string, credentials: Object) { + function _onConnectionFailed( // eslint-disable-line max-params + err: string, + msg: string, + credentials: Object, + details: Object) { unsubscribe(); dispatch( connectionFailed( connection, { credentials, + details, name: err, message: msg } @@ -197,16 +202,21 @@ function _connectionDisconnected(connection: Object, message: string) { * * @param {JitsiConnection} connection - The {@code JitsiConnection} which was * established. + * @param {number} timeEstablished - The time at which the + * {@code JitsiConnection} which was established. * @public * @returns {{ * type: CONNECTION_ESTABLISHED, - * connection: JitsiConnection + * connection: JitsiConnection, + * timeEstablished: number * }} */ -export function connectionEstablished(connection: Object) { +export function connectionEstablished( + connection: Object, timeEstablished: number) { return { type: CONNECTION_ESTABLISHED, - connection + connection, + timeEstablished }; } diff --git a/react/features/base/connection/reducer.js b/react/features/base/connection/reducer.js index 3d08176b29..7bb8aed327 100644 --- a/react/features/base/connection/reducer.js +++ b/react/features/base/connection/reducer.js @@ -65,7 +65,8 @@ function _connectionDisconnected( return assign(state, { connecting: undefined, - connection: undefined + connection: undefined, + timeEstablished: undefined }); } @@ -81,12 +82,16 @@ function _connectionDisconnected( */ function _connectionEstablished( state: Object, - { connection }: { connection: Object }) { + { connection, timeEstablished }: { + connection: Object, + timeEstablished: number + }) { return assign(state, { connecting: undefined, connection, error: undefined, - passwordRequired: undefined + passwordRequired: undefined, + timeEstablished }); } @@ -143,7 +148,8 @@ function _connectionWillConnect( // done before the new one is established. connection: undefined, error: undefined, - passwordRequired: undefined + passwordRequired: undefined, + timeEstablished: undefined }); } diff --git a/react/features/overlay/actions.js b/react/features/overlay/actions.js index b8a15529fa..ea69b69895 100644 --- a/react/features/overlay/actions.js +++ b/react/features/overlay/actions.js @@ -1,14 +1,9 @@ -import { appNavigate, reloadWithStoredParams } from '../app'; -import { toURLString } from '../base/util'; - import { MEDIA_PERMISSION_PROMPT_VISIBILITY_CHANGED, SET_FATAL_ERROR, SUSPEND_DETECTED } from './actionTypes'; -const logger = require('jitsi-meet-logger').getLogger(__filename); - /** * Signals that the prompt for media permission is visible or not. * @@ -30,28 +25,6 @@ export function mediaPermissionPromptVisibilityChanged(isVisible, browser) { }; } -/** - * Reloads the page. - * - * @protected - * @returns {Function} - */ -export function _reloadNow() { - return (dispatch, getState) => { - dispatch(setFatalError(undefined)); - - const { locationURL } = getState()['features/base/connection']; - - logger.info(`Reloading the conference using URL: ${locationURL}`); - - if (navigator.product === 'ReactNative') { - dispatch(appNavigate(toURLString(locationURL))); - } else { - dispatch(reloadWithStoredParams()); - } - }; -} - /** * Signals that suspend was detected. * diff --git a/react/features/overlay/components/AbstractPageReloadOverlay.js b/react/features/overlay/components/AbstractPageReloadOverlay.js index f161a2fa43..d9bbfa94c4 100644 --- a/react/features/overlay/components/AbstractPageReloadOverlay.js +++ b/react/features/overlay/components/AbstractPageReloadOverlay.js @@ -7,13 +7,13 @@ import { createPageReloadScheduledEvent, sendAnalytics } from '../../analytics'; +import { reloadNow } from '../../app'; import { isFatalJitsiConferenceError, isFatalJitsiConnectionError } from '../../base/lib-jitsi-meet'; import { randomInt } from '../../base/util'; -import { _reloadNow } from '../actions'; import ReloadButton from './ReloadButton'; declare var APP: Object; @@ -215,7 +215,7 @@ export default class AbstractPageReloadOverlay extends Component<*, *> { this._interval = undefined; } - this.props.dispatch(_reloadNow()); + this.props.dispatch(reloadNow()); } else { this.setState(prevState => { return { diff --git a/react/features/overlay/components/PageReloadOverlay.native.js b/react/features/overlay/components/PageReloadOverlay.native.js index 6f3d02a66a..8e711c53d0 100644 --- a/react/features/overlay/components/PageReloadOverlay.native.js +++ b/react/features/overlay/components/PageReloadOverlay.native.js @@ -2,13 +2,13 @@ import React from 'react'; import { Text, View } from 'react-native'; import { connect } from 'react-redux'; -import { appNavigate } from '../../app'; +import { appNavigate, reloadNow } from '../../app'; import { translate } from '../../base/i18n'; import { LoadingIndicator } from '../../base/react'; import AbstractPageReloadOverlay, { abstractMapStateToProps } from './AbstractPageReloadOverlay'; -import { _reloadNow, setFatalError } from '../actions'; +import { setFatalError } from '../actions'; import OverlayFrame from './OverlayFrame'; import { pageReloadOverlay as styles } from './styles'; @@ -55,7 +55,7 @@ class PageReloadOverlay extends AbstractPageReloadOverlay { */ _onReloadNow() { clearInterval(this._interval); - this.props.dispatch(_reloadNow()); + this.props.dispatch(reloadNow()); } /** diff --git a/react/features/overlay/components/ReloadButton.js b/react/features/overlay/components/ReloadButton.js index 9a9f916e10..e7c64fc292 100644 --- a/react/features/overlay/components/ReloadButton.js +++ b/react/features/overlay/components/ReloadButton.js @@ -4,10 +4,9 @@ import PropTypes from 'prop-types'; import React, { Component } from 'react'; import { connect } from 'react-redux'; +import { reloadNow } from '../../app'; import { translate } from '../../base/i18n'; -import { _reloadNow } from '../actions'; - /** * Implements a React Component for button for the overlays that will reload * the page. @@ -82,7 +81,7 @@ function _mapDispatchToProps(dispatch: Function): Object { * @returns {Object} Dispatched action. */ _reloadNow() { - dispatch(_reloadNow()); + dispatch(reloadNow()); } }; }