Merge pull request #1859 from matrix-org/dbkr/fix_session_restore_error_handling

Fix error handling on session restore

src/Lifecycle.js

@@ -1,6 +1,7 @@
 /*
 Copyright 2015, 2016 OpenMarket Ltd
 Copyright 2017 Vector Creations Ltd
+Copyright 2018 New Vector Ltd
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -64,33 +65,33 @@ import sdk from './index';
  *     Resolves to `true` if we ended up starting a session, or `false` if we
  *     failed.
  */
-export function loadSession(opts) {
-    const fragmentQueryParams = opts.fragmentQueryParams || {};
-    let enableGuest = opts.enableGuest || false;
-    const guestHsUrl = opts.guestHsUrl;
-    const guestIsUrl = opts.guestIsUrl;
-    const defaultDeviceDisplayName = opts.defaultDeviceDisplayName;
+export async function loadSession(opts) {
+    try {
+        let enableGuest = opts.enableGuest || false;
+        const guestHsUrl = opts.guestHsUrl;
+        const guestIsUrl = opts.guestIsUrl;
+        const fragmentQueryParams = opts.fragmentQueryParams || {};
+        const defaultDeviceDisplayName = opts.defaultDeviceDisplayName;
 
-    if (!guestHsUrl) {
-        console.warn("Cannot enable guest access: can't determine HS URL to use");
-        enableGuest = false;
-    }
+        if (!guestHsUrl) {
+            console.warn("Cannot enable guest access: can't determine HS URL to use");
+            enableGuest = false;
+        }
 
-    if (enableGuest &&
-        fragmentQueryParams.guest_user_id &&
-        fragmentQueryParams.guest_access_token
-       ) {
-        console.log("Using guest access credentials");
-        return _doSetLoggedIn({
-            userId: fragmentQueryParams.guest_user_id,
-            accessToken: fragmentQueryParams.guest_access_token,
-            homeserverUrl: guestHsUrl,
-            identityServerUrl: guestIsUrl,
-            guest: true,
-        }, true).then(() => true);
-    }
-
-    return _restoreFromLocalStorage().then((success) => {
+        if (enableGuest &&
+            fragmentQueryParams.guest_user_id &&
+            fragmentQueryParams.guest_access_token
+           ) {
+            console.log("Using guest access credentials");
+            return _doSetLoggedIn({
+                userId: fragmentQueryParams.guest_user_id,
+                accessToken: fragmentQueryParams.guest_access_token,
+                homeserverUrl: guestHsUrl,
+                identityServerUrl: guestIsUrl,
+                guest: true,
+            }, true).then(() => true);
+        }
+        const success = await _restoreFromLocalStorage();
         if (success) {
             return true;
         }
@@ -101,7 +102,9 @@ export function loadSession(opts) {
 
         // fall back to login screen
         return false;
-    });
+    } catch (e) {
+        return _handleLoadSessionFailure(e);
+    }
 }
 
 /**
@@ -195,9 +198,9 @@ function _registerAsGuest(hsUrl, isUrl, defaultDeviceDisplayName) {
 //      The plan is to gradually move the localStorage access done here into
 //      SessionStore to avoid bugs where the view becomes out-of-sync with
 //      localStorage (e.g. teamToken, isGuest etc.)
-function _restoreFromLocalStorage() {
+async function _restoreFromLocalStorage() {
     if (!localStorage) {
-        return Promise.resolve(false);
+        return false;
     }
     const hsUrl = localStorage.getItem("mx_hs_url");
     const isUrl = localStorage.getItem("mx_is_url") || 'https://matrix.org';
@@ -215,26 +218,23 @@ function _restoreFromLocalStorage() {
 
     if (accessToken && userId && hsUrl) {
         console.log(`Restoring session for ${userId}`);
-        try {
-            return _doSetLoggedIn({
-                userId: userId,
-                deviceId: deviceId,
-                accessToken: accessToken,
-                homeserverUrl: hsUrl,
-                identityServerUrl: isUrl,
-                guest: isGuest,
-            }, false).then(() => true);
-        } catch (e) {
-            return _handleRestoreFailure(e);
-        }
+        await _doSetLoggedIn({
+            userId: userId,
+            deviceId: deviceId,
+            accessToken: accessToken,
+            homeserverUrl: hsUrl,
+            identityServerUrl: isUrl,
+            guest: isGuest,
+        }, false);
+        return true;
     } else {
         console.log("No previous session found.");
-        return Promise.resolve(false);
+        return false;
     }
 }
 
-function _handleRestoreFailure(e) {
-    console.log("Unable to restore session", e);
+function _handleLoadSessionFailure(e) {
+    console.log("Unable to load session", e);
 
     const def = Promise.defer();
     const SessionRestoreErrorDialog =
@@ -255,7 +255,7 @@ function _handleRestoreFailure(e) {
         }
 
         // try, try again
-        return _restoreFromLocalStorage();
+        return loadSession();
     });
 }
 

src/components/structures/MatrixChat.js

@@ -1,7 +1,7 @@
 /*
 Copyright 2015, 2016 OpenMarket Ltd
 Copyright 2017 Vector Creations Ltd
-Copyright 2017 New Vector Ltd
+Copyright 2017, 2018 New Vector Ltd
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -351,16 +351,16 @@ export default React.createClass({
                     guestIsUrl: this.getCurrentIsUrl(),
                     defaultDeviceDisplayName: this.props.defaultDeviceDisplayName,
                 });
-            }).catch((e) => {
-                console.error('Error attempting to load session', e);
-                return false;
             }).then((loadedSession) => {
                 if (!loadedSession) {
                     // fall back to showing the login screen
                     dis.dispatch({action: "start_login"});
                 }
             });
-        }).done();
+            // Note we don't catch errors from this: we catch everything within
+            // loadSession as there's logic there to ask the user if they want
+            // to try logging out.
+        });
     },
 
     componentWillUnmount: function() {

src/components/views/dialogs/BaseDialog.js

@@ -36,6 +36,9 @@ export default React.createClass({
 
     propTypes: {
         // onFinished callback to call when Escape is pressed
+        // Take a boolean which is true if the dialog was dismissed
+        // with a positive / confirm action or false if it was
+        // cancelled (BaseDialog itself only calls this with false).
         onFinished: PropTypes.func.isRequired,
 
         // called when a key is pressed
@@ -77,12 +80,12 @@ export default React.createClass({
         if (e.keyCode === KeyCode.ESCAPE) {
             e.stopPropagation();
             e.preventDefault();
-            this.props.onFinished();
+            this.props.onFinished(false);
         }
     },
 
     _onCancelClick: function(e) {
-        this.props.onFinished();
+        this.props.onFinished(false);
     },
 
     render: function() {

src/components/views/dialogs/SessionRestoreErrorDialog.js

@@ -1,5 +1,6 @@
 /*
 Copyright 2017 Vector Creations Ltd
+Copyright 2018 New Vector Ltd
 
 Licensed under the Apache License, Version 2.0 (the "License");
 you may not use this file except in compliance with the License.
@@ -41,10 +42,14 @@ export default React.createClass({
         Modal.createTrackedDialog('Session Restore Error', 'Send Bug Report Dialog', BugReportDialog, {});
     },
 
-    _continueClicked: function() {
+    _onContinueClick: function() {
         this.props.onFinished(true);
     },
 
+    _onCancelClick: function() {
+        this.props.onFinished(false);
+    },
+
     render: function() {
         const BaseDialog = sdk.getComponent('views.dialogs.BaseDialog');
         const DialogButtons = sdk.getComponent('views.elements.DialogButtons');
@@ -81,8 +86,8 @@ export default React.createClass({
                     { bugreport }
                 </div>
                 <DialogButtons primaryButton={_t("Continue anyway")}
-                    onPrimaryButtonClick={this._continueClicked} focus={shouldFocusContinueButton}
-                    onCancel={this.props.onFinished} />
+                    onPrimaryButtonClick={this._onContinueClick} focus={shouldFocusContinueButton}
+                    onCancel={this._onCancelClick} />
             </BaseDialog>
         );
     },

src/components/views/elements/DialogButtons.js

@@ -39,6 +39,10 @@ module.exports = React.createClass({
         focus: PropTypes.bool,
     },
 
+    _onCancelClick: function() {
+        this.props.onCancel();
+    },
+
     render: function() {
         let primaryButtonClassName = "mx_Dialog_primary";
         if (this.props.primaryButtonClass) {
@@ -53,7 +57,7 @@ module.exports = React.createClass({
                     { this.props.primaryButton }
                 </button>
                 { this.props.children }
-                <button onClick={this.props.onCancel}>
+                <button onClick={this._onCancelClick}>
                     { _t("Cancel") }
                 </button>
             </div>