From c05c8e0f1eded9ffbcd799ec41045fb25b3a3d66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sa=C3=BAl=20Ibarra=20Corretg=C3=A9?= Date: Wed, 29 Nov 2017 15:15:57 +0100 Subject: [PATCH] [RN] Handle config loading errors They will be stored in redux and the PageReloadOverlay will be displayed. Note that this commit also introduces a subtle (and yet important!) change: the location URL is now always set, regardless of the configuration loading or not. This is needed in order for the retry logic to pick it up. --- react/features/app/actions.js | 21 ++++++++--------- react/features/base/config/actionTypes.js | 11 +++++++++ react/features/base/config/actions.js | 23 ++++++++++++++++++- react/features/base/config/reducer.js | 23 +++++++++++++++---- .../components/AbstractPageReloadOverlay.js | 7 ++++-- 5 files changed, 67 insertions(+), 18 deletions(-) diff --git a/react/features/app/actions.js b/react/features/app/actions.js index edc03d079..2d34ef438 100644 --- a/react/features/app/actions.js +++ b/react/features/app/actions.js @@ -1,7 +1,7 @@ /* @flow */ import { setRoom } from '../base/conference'; -import { loadConfigError, setConfig } from '../base/config'; +import { configWillLoad, loadConfigError, setConfig } from '../base/config'; import { setLocationURL } from '../base/connection'; import { loadConfig } from '../base/lib-jitsi-meet'; import { parseURIString } from '../base/util'; @@ -44,6 +44,8 @@ function _appNavigateToMandatoryLocation( ): Promise { const { room } = newLocation; + dispatch(configWillLoad(newLocation)); + return ( _loadConfig(newLocation) .then( @@ -67,23 +69,20 @@ function _appNavigateToMandatoryLocation( // config may or may not be required by the time the notification // arrives. + const promise + = dispatch(setLocationURL(new URL(newLocation.toString()))); + if (error) { // XXX The failure could be, for example, because of a // certificate-related error. In which case the connection will // fail later in Strophe anyway. - dispatch(loadConfigError(error, newLocation)); - - // Cannot go to a room if its configuration failed to load. - if (room) { - dispatch(appNavigate(undefined)); - + return promise.then(() => { + dispatch(loadConfigError(error, newLocation)); throw error; - } + }); } - return ( - dispatch(setLocationURL(new URL(newLocation.toString()))) - .then(() => dispatch(setConfig(config)))); + return promise.then(() => dispatch(setConfig(config))); } } diff --git a/react/features/base/config/actionTypes.js b/react/features/base/config/actionTypes.js index 4be2581b0..74846d614 100644 --- a/react/features/base/config/actionTypes.js +++ b/react/features/base/config/actionTypes.js @@ -1,3 +1,14 @@ +/** + * The redux action which signals that a configuration will be loaded for a + * specific locationURL. + * + * { + * type: CONFIG_WILL_LOAD, + * locationURL: string | URL + * } + */ +export const CONFIG_WILL_LOAD = Symbol('CONFIG_WILL_LOAD'); + /** * The redux action which signals that a configuration could not be loaded due * to a specific error. diff --git a/react/features/base/config/actions.js b/react/features/base/config/actions.js index 8e1814c93..88fd65030 100644 --- a/react/features/base/config/actions.js +++ b/react/features/base/config/actions.js @@ -1,6 +1,27 @@ /* @flow */ -import { LOAD_CONFIG_ERROR, SET_CONFIG } from './actionTypes'; +import { + CONFIG_WILL_LOAD, + LOAD_CONFIG_ERROR, + SET_CONFIG +} from './actionTypes'; + +/** + * Signals that the configuration for a specific locationURL will be loaded now. + * + * @param {string|URL} locationURL - The URL of the location which necessitated + * the loading of a configuration. + * @returns {{ + * type: CONFIG_WILL_LOAD, + * locationURL + * }} + */ +export function configWillLoad(locationURL: string | URL) { + return { + type: CONFIG_WILL_LOAD, + locationURL + }; +} /** * Signals that a configuration could not be loaded due to a specific error. diff --git a/react/features/base/config/reducer.js b/react/features/base/config/reducer.js index 9ddb79e8a..f638d130c 100644 --- a/react/features/base/config/reducer.js +++ b/react/features/base/config/reducer.js @@ -4,7 +4,11 @@ import _ from 'lodash'; import { equals, ReducerRegistry, set } from '../redux'; -import { SET_CONFIG } from './actionTypes'; +import { + CONFIG_WILL_LOAD, + LOAD_CONFIG_ERROR, + SET_CONFIG +} from './actionTypes'; /** * The initial state of the feature base/config when executing in a @@ -47,6 +51,16 @@ ReducerRegistry.register( 'features/base/config', (state = _getInitialState(), action) => { switch (action.type) { + case CONFIG_WILL_LOAD: + return { + error: undefined + }; + + case LOAD_CONFIG_ERROR: + return { + error: action.error + }; + case SET_CONFIG: return _setConfig(state, action); @@ -81,20 +95,21 @@ function _getInitialState() { * @returns {Object} The new state of the feature base/lib-jitsi-meet after the * reduction of the specified action. */ -function _setConfig(state, action) { - let { config } = action; - +function _setConfig(state, { config }) { // The mobile app bundles jitsi-meet and lib-jitsi-meet at build time and // does not download them at runtime from the deployment on which it will // join a conference. The downloading is planned for implementation in the // future (later rather than sooner) but is not implemented yet at the time // of this writing and, consequently, we must provide legacy support in the // meantime. + + // eslint-disable-next-line no-param-reassign config = _translateLegacyConfig(config); const newState = _.merge( {}, config, + { error: undefined }, // The config of _getInitialState() is meant to override the config // downloaded from the Jitsi Meet deployment because the former contains diff --git a/react/features/overlay/components/AbstractPageReloadOverlay.js b/react/features/overlay/components/AbstractPageReloadOverlay.js index 9bcfb8c91..32bc7a83e 100644 --- a/react/features/overlay/components/AbstractPageReloadOverlay.js +++ b/react/features/overlay/components/AbstractPageReloadOverlay.js @@ -66,12 +66,14 @@ export default class AbstractPageReloadOverlay extends Component<*, *> { */ static needsRender(state) { const conferenceError = state['features/base/conference'].error; + const configError = state['features/base/config'].error; const connectionError = state['features/base/connection'].error; return ( (connectionError && isFatalJitsiConnectionError(connectionError)) || (conferenceError && isFatalJitsiConferenceError(conferenceError)) + || configError ); } @@ -253,10 +255,11 @@ export default class AbstractPageReloadOverlay extends Component<*, *> { */ export function abstractMapStateToProps(state: Object) { const conferenceError = state['features/base/conference'].error; + const configError = state['features/base/config'].error; const connectionError = state['features/base/connection'].error; return { - isNetworkFailure: Boolean(connectionError), - reason: (connectionError || conferenceError).message + isNetworkFailure: Boolean(configError || connectionError), + reason: (configError || connectionError || conferenceError).message }; }