Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fail more softly on homeserver liveliness errors #3067

Merged
merged 7 commits into from
Jun 7, 2019
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion res/css/structures/_GenericErrorPage.scss
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
right: 0;
margin: auto;
width: 500px;
height: 200px;
height: 125px;
border: 1px solid #f22;
padding: 10px 10px 20px;
background-color: #fcc;
Expand Down
5 changes: 5 additions & 0 deletions res/css/structures/auth/_Login.scss
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,11 @@ limitations under the License.
margin-bottom: 12px;
}

.mx_Login_error.mx_Login_serverError {
text-align: left;
font-weight: normal;
}

.mx_Login_type_container {
display: flex;
align-items: center;
Expand Down
8 changes: 2 additions & 6 deletions src/components/structures/GenericErrorPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,18 @@ limitations under the License.

import React from 'react';
import PropTypes from 'prop-types';
import {_t} from "../../languageHandler";

export default class GenericErrorPage extends React.PureComponent {
static propTypes = {
title: PropTypes.object.isRequired, // jsx for title
message: PropTypes.object.isRequired, // jsx to display
};

render() {
return <div className='mx_GenericErrorPage'>
<div className='mx_GenericErrorPage_box'>
<h1>{_t("Error loading Riot")}</h1>
<h1>{this.props.title}</h1>
<p>{this.props.message}</p>
<p>{_t(
"If this is unexpected, please contact your system administrator " +
"or technical support representative.",
)}</p>
</div>
</div>;
}
Expand Down
56 changes: 52 additions & 4 deletions src/components/structures/auth/ForgotPassword.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import sdk from '../../../index';
import Modal from "../../../Modal";
import SdkConfig from "../../../SdkConfig";
import PasswordReset from "../../../PasswordReset";
import {ValidatedServerConfig} from "../../../utils/AutoDiscoveryUtils";
import AutoDiscoveryUtils, {ValidatedServerConfig} from "../../../utils/AutoDiscoveryUtils";

// Phases
// Show controls to configure server details
Expand Down Expand Up @@ -53,9 +53,40 @@ module.exports = React.createClass({
password: "",
password2: "",
errorText: null,

// We perform liveliness checks later, but for now suppress the errors.
// We also track the server dead errors independently of the regular errors so
// that we can render it differently, and override any other error the user may
// be seeing.
serverIsAlive: true,
serverDeadError: "",
};
},

componentWillMount: function() {
this._checkServerLiveliness(this.props.serverConfig);
},

componentWillReceiveProps: async function(newProps) {
turt2live marked this conversation as resolved.
Show resolved Hide resolved
if (newProps.serverConfig.hsUrl === this.props.serverConfig.hsUrl &&
newProps.serverConfig.isUrl === this.props.serverConfig.isUrl) return;

// Do a liveliness check on the new URLs
this._checkServerLiveliness(newProps.serverConfig);
},

_checkServerLiveliness: async function(serverConfig) {
try {
await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(
serverConfig.hsUrl,
serverConfig.isUrl,
);
this.setState({serverIsAlive: true});
} catch (e) {
this.setState(AutoDiscoveryUtils.authComponentStateForError(e));
}
},

submitPasswordReset: function(email, password) {
this.setState({
phase: PHASE_SENDING_EMAIL,
Expand Down Expand Up @@ -89,6 +120,8 @@ module.exports = React.createClass({
onSubmitForm: function(ev) {
ev.preventDefault();

if (!this.state.serverIsAlive) return;
turt2live marked this conversation as resolved.
Show resolved Hide resolved

if (!this.state.email) {
this.showErrorDialog(_t('The email address linked to your account must be entered.'));
} else if (!this.state.password || !this.state.password2) {
Expand Down Expand Up @@ -173,11 +206,20 @@ module.exports = React.createClass({
const Field = sdk.getComponent('elements.Field');

let errorText = null;
const err = this.state.errorText || this.props.defaultServerDiscoveryError;
const err = this.state.errorText;
if (err) {
errorText = <div className="mx_Login_error">{ err }</div>;
}

let serverDeadSection;
if (!this.state.serverIsAlive) {
serverDeadSection = (
<div className="mx_Login_error mx_Login_serverError">
{this.state.serverDeadError}
</div>
);
}

let yourMatrixAccountText = _t('Your Matrix account on %(serverName)s', {
serverName: this.props.serverConfig.hsName,
});
Expand Down Expand Up @@ -207,11 +249,12 @@ module.exports = React.createClass({
}

return <div>
{errorText}
{serverDeadSection}
<h3>
{yourMatrixAccountText}
{editLink}
</h3>
{errorText}
<form onSubmit={this.onSubmitForm}>
<div className="mx_AuthBody_fieldRow">
<Field
Expand Down Expand Up @@ -246,7 +289,12 @@ module.exports = React.createClass({
'A verification email will be sent to your inbox to confirm ' +
'setting your new password.',
)}</span>
<input className="mx_Login_submit" type="submit" value={_t('Send Reset Email')} />
<input
className="mx_Login_submit"
type="submit"
value={_t('Send Reset Email')}
disabled={!this.state.serverIsAlive}
/>
</form>
<a className="mx_AuthBody_changeFlow" onClick={this.onLoginClick} href="#">
{_t('Sign in instead')}
Expand Down
80 changes: 67 additions & 13 deletions src/components/structures/auth/Login.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,13 @@ module.exports = React.createClass({
phase: PHASE_LOGIN,
// The current login flow, such as password, SSO, etc.
currentFlow: "m.login.password",

// We perform liveliness checks later, but for now suppress the errors.
// We also track the server dead errors independently of the regular errors so
// that we can render it differently, and override any other error the user may
// be seeing.
serverIsAlive: true,
serverDeadError: "",
};
},

Expand Down Expand Up @@ -233,7 +240,7 @@ module.exports = React.createClass({
username: username,
busy: doWellknownLookup, // unset later by the result of onServerConfigChange
errorText: null,
canTryLogin: true,
canTryLogin: this.state.serverIsAlive,
turt2live marked this conversation as resolved.
Show resolved Hide resolved
});
if (doWellknownLookup) {
const serverName = username.split(':').slice(1).join(':');
Expand All @@ -247,7 +254,19 @@ module.exports = React.createClass({
if (e.translatedMessage) {
message = e.translatedMessage;
}
this.setState({errorText: message, busy: false, canTryLogin: false});

let errorText = message;
let discoveryState = {};
if (AutoDiscoveryUtils.isLivelinessError(e)) {
errorText = this.state.errorText;
discoveryState = this._stateForDiscoveryError(e);
}

this.setState({
busy: false,
errorText,
...discoveryState,
});
}
}
},
Expand All @@ -272,7 +291,7 @@ module.exports = React.createClass({
} else {
this.setState({
errorText: null,
canTryLogin: true,
canTryLogin: this.state.serverIsAlive,
});
}
},
Expand All @@ -297,13 +316,25 @@ module.exports = React.createClass({
});
},

_initLoginLogic: function(hsUrl, isUrl) {
const self = this;
_stateForDiscoveryError: function(err) {
return {
canTryLogin: false,
...AutoDiscoveryUtils.authComponentStateForError(err),
};
},

_initLoginLogic: async function(hsUrl, isUrl) {
hsUrl = hsUrl || this.props.serverConfig.hsUrl;
isUrl = isUrl || this.props.serverConfig.isUrl;

// TODO: TravisR - Only use this if the homeserver is the default homeserver
const fallbackHsUrl = this.props.fallbackHsUrl;
let isDefaultServer = false;
if (this.props.serverConfig.isDefault
&& hsUrl === this.props.serverConfig.hsUrl
&& isUrl === this.props.serverConfig.isUrl) {
isDefaultServer = true;
}

const fallbackHsUrl = isDefaultServer ? this.props.fallbackHsUrl : null;

const loginLogic = new Login(hsUrl, isUrl, fallbackHsUrl, {
defaultDeviceDisplayName: this.props.defaultDeviceDisplayName,
Expand All @@ -315,6 +346,19 @@ module.exports = React.createClass({
loginIncorrect: false,
});

// Do a quick liveliness check on the URLs
try {
await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(hsUrl, isUrl);
this.setState({serverIsAlive: true, errorText: "", canTryLogin: true});
} catch (e) {
const discoveryState = this._stateForDiscoveryError(e);
this.setState({
busy: false,
...discoveryState,
});
return; // Server is dead - do not continue.
}

loginLogic.getFlows().then((flows) => {
// look for a flow where we understand all of the steps.
for (let i = 0; i < flows.length; i++ ) {
Expand All @@ -339,14 +383,14 @@ module.exports = React.createClass({
"supported by this client.",
),
});
}, function(err) {
self.setState({
errorText: self._errorTextFromError(err),
}, (err) => {
this.setState({
errorText: this._errorTextFromError(err),
loginIncorrect: false,
canTryLogin: false,
});
}).finally(function() {
self.setState({
}).finally(() => {
this.setState({
busy: false,
});
}).done();
Expand Down Expand Up @@ -485,7 +529,7 @@ module.exports = React.createClass({
onForgotPasswordClick={this.props.onForgotPasswordClick}
loginIncorrect={this.state.loginIncorrect}
serverConfig={this.props.serverConfig}
disableSubmit={this.isBusy()}
disableSubmit={this.isBusy() || !this.state.serverIsAlive}
turt2live marked this conversation as resolved.
Show resolved Hide resolved
/>
);
},
Expand Down Expand Up @@ -522,6 +566,15 @@ module.exports = React.createClass({
);
}

let serverDeadSection;
if (!this.state.serverIsAlive) {
serverDeadSection = (
<div className="mx_Login_error mx_Login_serverError">
{this.state.serverDeadError}
</div>
);
}

return (
<AuthPage>
<AuthHeader />
Expand All @@ -531,6 +584,7 @@ module.exports = React.createClass({
{loader}
</h2>
{ errorTextSection }
{ serverDeadSection }
{ this.renderServerComponent() }
{ this.renderLoginComponentForStep() }
<a className="mx_AuthBody_changeFlow" onClick={this.onRegisterClick} href="#">
Expand Down
33 changes: 32 additions & 1 deletion src/components/structures/auth/Registration.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { _t, _td } from '../../../languageHandler';
import SdkConfig from '../../../SdkConfig';
import { messageForResourceLimitError } from '../../../utils/ErrorUtils';
import * as ServerType from '../../views/auth/ServerTypeSelector';
import {ValidatedServerConfig} from "../../../utils/AutoDiscoveryUtils";
import AutoDiscoveryUtils, {ValidatedServerConfig} from "../../../utils/AutoDiscoveryUtils";

// Phases
// Show controls to configure server details
Expand Down Expand Up @@ -79,6 +79,13 @@ module.exports = React.createClass({
// Phase of the overall registration dialog.
phase: PHASE_REGISTRATION,
flows: null,

// We perform liveliness checks later, but for now suppress the errors.
// We also track the server dead errors independently of the regular errors so
// that we can render it differently, and override any other error the user may
// be seeing.
serverIsAlive: true,
serverDeadError: "",
};
},

Expand Down Expand Up @@ -152,6 +159,19 @@ module.exports = React.createClass({
errorText: null,
});
if (!serverConfig) serverConfig = this.props.serverConfig;

// Do a liveliness check on the URLs
try {
await AutoDiscoveryUtils.validateServerConfigWithStaticUrls(
serverConfig.hsUrl,
serverConfig.isUrl,
);
this.setState({serverIsAlive: true});
} catch (e) {
this.setState(AutoDiscoveryUtils.authComponentStateForError(e));
return; // Server is dead - do not continue.
}

const {hsUrl, isUrl} = serverConfig;
this._matrixClient = Matrix.createClient({
baseUrl: hsUrl,
Expand Down Expand Up @@ -447,6 +467,7 @@ module.exports = React.createClass({
onEditServerDetailsClick={onEditServerDetailsClick}
flows={this.state.flows}
serverConfig={this.props.serverConfig}
canSubmit={this.state.serverIsAlive}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here for not preventing form submission without an obvious way to re-check (for registration I guess you could toggle server-type but the user wouldn't know to do that).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Registration has a different phases cycle from the other 3, so I'm not convinced I need to fix the form here. There is no button to click if the server is down, regardless of liveliness checks: it's just a spinner.

/>;
}
},
Expand All @@ -462,6 +483,15 @@ module.exports = React.createClass({
errorText = <div className="mx_Login_error">{ err }</div>;
}

let serverDeadSection;
if (!this.state.serverIsAlive) {
serverDeadSection = (
<div className="mx_Login_error mx_Login_serverError">
{this.state.serverDeadError}
</div>
);
}

const signIn = <a className="mx_AuthBody_changeFlow" onClick={this.onLoginClick} href="#">
{ _t('Sign in instead') }
</a>;
Expand All @@ -480,6 +510,7 @@ module.exports = React.createClass({
<AuthBody>
<h2>{ _t('Create your account') }</h2>
{ errorText }
{ serverDeadSection }
{ this.renderServerComponent() }
{ this.renderRegisterComponent() }
{ goBack }
Expand Down
Loading