[RN] Fix processing outdated loadConfig requests (continued)

Co-authored-by: Lyubo Marinov <lmarinov@atlassian.com>
Co-authored-by: Paweł Domas <pawel.domas@jitsi.org>

react/features/app/actions.js

@@ -1,4 +1,4 @@
-/* @flow */
+// @flow
 
 import type { Dispatch } from 'redux';
 
@@ -51,8 +51,9 @@ function _appNavigateToMandatoryLocation(
         newLocation: Object
 ): Promise<void> {
     const { room } = newLocation;
+    const locationURL = new URL(newLocation.toString());
 
-    dispatch(configWillLoad(newLocation));
+    dispatch(configWillLoad(locationURL));
 
     return (
         _loadConfig(dispatch, getState, newLocation)
@@ -73,33 +74,32 @@ function _appNavigateToMandatoryLocation(
      * @returns {void}
      */
     function loadConfigSettled(error, config) {
-        // Due to the asynchronous nature of the loading, the specified
-        // config may or may not be required by the time the notification
-        // arrives. If we receive the config for a location we are no longer
-        // interested in, just dump it.
-
-        const { locationURL: currentLocationURL }
-            = getState()['features/base/config'];
-
-        if (currentLocationURL !== newLocation) {
-            throw new Error('Config no longer needed');
+        let promise;
+
+        // Due to the asynchronous nature of the loading, the specified config
+        // may or may not be required by the time the notification arrives.
+        // If we receive the config for a location we are no longer interested
+        // in, "ignore" it - deliver it to the external API, for example, but do
+        // not proceed with the appNavigate procedure/process.
+        if (getState()['features/base/config'].locationURL === locationURL) {
+            promise = dispatch(setLocationURL(locationURL));
+        } else {
+            // eslint-disable-next-line no-param-reassign
+            error || (error = new Error('Config no longer needed!'));
+            promise = Promise.resolve();
         }
 
-        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.
-            return promise.then(() => {
-                dispatch(loadConfigError(error, newLocation));
+        return promise.then(() => {
+            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, locationURL));
                 throw error;
-            });
-        }
+            }
 
-        return promise.then(() =>
-            dispatch(setConfig(config)));
+            return dispatch(setConfig(config));
+        });
     }
 }
 
@@ -227,6 +227,10 @@ function _loadConfig(
 
     return loadConfig(url).then(
         /* onFulfilled */ config => {
+            // FIXME If the config is no longer needed (in the terms of
+            // _loadConfig) and that happened because of an intervening
+            // _loadConfig for the same baseURL, then the unneeded config may be
+            // stored after the needed config. Anyway.
             dispatch(storeConfig(baseURL, config));
 
             return config;

react/features/base/config/actionTypes.js

@@ -1,22 +1,22 @@
 /**
- * The redux action which signals that a configuration will be loaded for a
- * specific locationURL.
+ * The redux action which signals that a configuration (commonly known in Jitsi
+ * Meet as config.js) will be loaded for a specific locationURL.
  *
  * {
  *     type: CONFIG_WILL_LOAD,
- *     locationURL: string | URL
+ *     locationURL: 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.
+ * The redux action which signals that a configuration (commonly known in Jitsi
+ * Meet as config.js) could not be loaded due to a specific error.
  *
  * {
  *     type: LOAD_CONFIG_ERROR,
  *     error: Error,
- *     locationURL: string | URL
+ *     locationURL: URL
  * }
  */
 export const LOAD_CONFIG_ERROR = Symbol('LOAD_CONFIG_ERROR');

react/features/base/config/actions.js

@@ -5,25 +5,22 @@ import type { Dispatch } from 'redux';
 import { addKnownDomains } from '../known-domains';
 import { parseURIString } from '../util';
 
-import {
-    CONFIG_WILL_LOAD,
-    LOAD_CONFIG_ERROR,
-    SET_CONFIG
-} from './actionTypes';
+import { CONFIG_WILL_LOAD, LOAD_CONFIG_ERROR, SET_CONFIG } from './actionTypes';
 import { _CONFIG_STORE_PREFIX } from './constants';
 import { setConfigFromURLParams } from './functions';
 
 /**
- * Signals that the configuration for a specific locationURL will be loaded now.
+ * Signals that the configuration (commonly known in Jitsi Meet as config.js)
+ * for a specific locationURL will be loaded now.
  *
- * @param {string|URL} locationURL - The URL of the location which necessitated
- * the loading of a configuration.
+ * @param {URL} locationURL - The URL of the location which necessitated the
+ * loading of a configuration.
  * @returns {{
  *     type: CONFIG_WILL_LOAD,
- *     locationURL
+ *     locationURL: URL
  * }}
  */
-export function configWillLoad(locationURL: string | URL) {
+export function configWillLoad(locationURL: URL) {
     return {
         type: CONFIG_WILL_LOAD,
         locationURL
@@ -31,19 +28,20 @@ export function configWillLoad(locationURL: string | URL) {
 }
 
 /**
- * Signals that a configuration could not be loaded due to a specific error.
+ * Signals that a configuration (commonly known in Jitsi Meet as config.js)
+ * could not be loaded due to a specific error.
  *
  * @param {Error} error - The {@code Error} which prevented the successful
  * loading of a configuration.
- * @param {string|URL} locationURL - The URL of the location which necessitated
- * the loading of a configuration.
+ * @param {URL} locationURL - The URL of the location which necessitated the
+ * loading of a configuration.
  * @returns {{
  *     type: LOAD_CONFIG_ERROR,
  *     error: Error,
- *     locationURL
+ *     locationURL: URL
  * }}
  */
-export function loadConfigError(error: Error, locationURL: string | URL) {
+export function loadConfigError(error: Error, locationURL: URL) {
     return {
         type: LOAD_CONFIG_ERROR,
         error,

react/features/base/config/reducer.js

@@ -1,14 +1,10 @@
-/* @flow */
+// @flow
 
 import _ from 'lodash';
 
 import { equals, ReducerRegistry, set } from '../redux';
 
-import {
-    CONFIG_WILL_LOAD,
-    LOAD_CONFIG_ERROR,
-    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
@@ -54,20 +50,41 @@ ReducerRegistry.register(
         case CONFIG_WILL_LOAD:
             return {
                 error: undefined,
+
+                /**
+                 * The URL of the location associated with/configured by this
+                 * configuration.
+                 *
+                 * @type URL
+                 */
                 locationURL: action.locationURL
             };
 
         case LOAD_CONFIG_ERROR:
-            return {
-                error: action.error
-            };
+            // XXX LOAD_CONFIG_ERROR is one of the settlement execution paths of
+            // the asynchronous "loadConfig procedure/process" started with
+            // CONFIG_WILL_LOAD. Due to the asynchronous nature of it, whoever
+            // is settling the process needs to provide proof that they have
+            // started it and that the iteration of the process being completed
+            // now is still of interest to the app.
+            if (state.locationURL === action.locationURL) {
+                return {
+                    /**
+                     * The {@link Error} which prevented the loading of the
+                     * configuration of the associated {@code locationURL}.
+                     *
+                     * @type Error
+                     */
+                    error: action.error
+                };
+            }
+            break;
 
         case SET_CONFIG:
             return _setConfig(state, action);
-
-        default:
-            return state;
         }
+
+        return state;
     });
 
 /**

react/features/mobile/external-api/middleware.js

@@ -28,8 +28,9 @@ import { ENTER_PICTURE_IN_PICTURE } from '../picture-in-picture';
  */
 MiddlewareRegistry.register(store => next => action => {
     const result = next(action);
+    const { type } = action;
 
-    switch (action.type) {
+    switch (type) {
     case CONFERENCE_FAILED: {
         const { error, ...data } = action;
 
@@ -64,16 +65,19 @@ MiddlewareRegistry.register(store => next => action => {
         break;
 
     case ENTER_PICTURE_IN_PICTURE:
-        _sendEvent(store, _getSymbolDescription(action.type), /* data */ {});
+        _sendEvent(store, _getSymbolDescription(type), /* data */ {});
         break;
 
     case LOAD_CONFIG_ERROR: {
-        const { error, locationURL, type } = action;
+        const { error, locationURL } = action;
 
-        _sendEvent(store, _getSymbolDescription(type), /* data */ {
-            error: _toErrorString(error),
-            url: toURLString(locationURL)
-        });
+        _sendEvent(
+            store,
+            _getSymbolDescription(type),
+            /* data */ {
+                error: _toErrorString(error),
+                url: toURLString(locationURL)
+            });
         break;
     }