Skip to content

Commit

Permalink
CONNECTION_FAILED error as object
Browse files Browse the repository at this point in the history
Gradually, we exploded the error of CONNECTION_FAILED in multiple
redux state properties. The explosion makes maintenance harder because
the properties have to be updated in sync. Collect them in an object
resembling an Error instance.
  • Loading branch information
lyubomir committed Sep 24, 2017
1 parent f8b607e commit 4e0761a
Show file tree
Hide file tree
Showing 9 changed files with 89 additions and 85 deletions.
5 changes: 3 additions & 2 deletions connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,9 @@ function connect(id, password, roomName) {
ConnectionEvents.CONNECTION_FAILED,
connectionFailedHandler);

function connectionFailedHandler(error, errMsg) {
APP.store.dispatch(connectionFailed(connection, error, errMsg));
function connectionFailedHandler(error, message, credentials) {
APP.store.dispatch(
connectionFailed(connection, error, message, credentials));

if (isFatalJitsiConnectionError(error)) {
connection.removeEventListener(
Expand Down
19 changes: 17 additions & 2 deletions react/features/authentication/actions.js
Original file line number Diff line number Diff line change
Expand Up @@ -127,10 +127,25 @@ export function stopWaitForOwner() {
* @private
* @returns {{
* type: UPGRADE_ROLE_FINISHED,
* error: Object
* error: ?Object
* }}
*/
function _upgradeRoleFinished(error) {
function _upgradeRoleFinished(error: ?Object) {
if (error) {
// Make the specified error object resemble an Error instance (to the
// extent that jitsi-meet needs it).
const {
authenticationError,
connectionError,
...other
} = error;

error = { // eslint-disable-line no-param-reassign
name: authenticationError || connectionError,
...other
};
}

return {
type: UPGRADE_ROLE_FINISHED,
error
Expand Down
82 changes: 31 additions & 51 deletions react/features/authentication/components/LoginDialog.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,17 +64,7 @@ class LoginDialog extends Component {
/**
* The error which occurred during login/authentication.
*/
_error: PropTypes.string,

/**
* The credential that the user has failed to authenticate with.
*/
_errorCredentials: PropTypes.object,

/**
* Any extra details about the error provided by lib-jitsi-meet.
*/
_errorDetails: PropTypes.string,
_error: PropTypes.object,

/**
* Redux store dispatch method.
Expand Down Expand Up @@ -118,24 +108,34 @@ class LoginDialog extends Component {
const {
_connecting: connecting,
_error: error,
_errorCredentials: errorCredentials,
_errorDetails: errorDetails,
t
} = this.props;

let messageKey = '';
const messageOptions = {};
let messageKey;
let messageOptions;

if (error === JitsiConnectionErrors.PASSWORD_REQUIRED) {
// Show the message if there's been a user ID or password provided.
messageKey
= errorCredentials
&& (errorCredentials.jid || errorCredentials.password)
? 'dialog.incorrectPassword'
: null;
} else if (error) {
messageKey = 'dialog.connectErrorWithMsg';
messageOptions.msg = `${error} ${errorDetails}`;
if (error) {
const { name } = error;

if (name === JitsiConnectionErrors.PASSWORD_REQUIRED) {
// Show a message that the credentials are incorrect only if the
// credentials which have caused the connection to fail are the
// ones which the user sees.
const { credentials } = error;

if (credentials
&& credentials.jid
=== toJid(
this.state.username,
this.props._configHosts)
&& credentials.password === this.state.password) {
messageKey = 'dialog.incorrectPassword';
}
} else if (name) {
messageKey = 'dialog.connectErrorWithMsg';
messageOptions || (messageOptions = {});
messageOptions.msg = `${name} ${error.message}`;
}
}

return (
Expand All @@ -146,6 +146,8 @@ class LoginDialog extends Component {
titleKey = 'dialog.passwordRequired'>
<View style = { styles.loginDialog }>
<TextInput
autoCapitalize = { 'none' }
autoCorrect = { false }
onChangeText = { this._onUsernameChange }
placeholder = { '[email protected]' }
style = { styles.loginDialogTextInput }
Expand All @@ -159,7 +161,7 @@ class LoginDialog extends Component {
<Text style = { styles.loginDialogText }>
{
messageKey
? t(messageKey, messageOptions)
? t(messageKey, messageOptions || {})
: connecting
? t('connection.CONNECTING')
: ''
Expand Down Expand Up @@ -239,9 +241,7 @@ class LoginDialog extends Component {
* _conference: JitsiConference,
* _configHosts: Object,
* _connecting: boolean,
* _error: string,
* _errorCredentials: Object,
* _errorDetails: string
* _error: Object
* }}
*/
function _mapStateToProps(state) {
Expand All @@ -253,34 +253,14 @@ function _mapStateToProps(state) {
const { hosts: configHosts } = state['features/base/config'];
const {
connecting,
credentials,
error: connectionError,
errorMessage: connectionErrorMessage
error: connectionError
} = state['features/base/connection'];

let error;
let errorCredentials;
let errorDetails;

if (connectionError) {
error = connectionError;
errorCredentials = credentials;
errorDetails = connectionErrorMessage;
} else if (upgradeRoleError) {
error
= upgradeRoleError.connectionError
|| upgradeRoleError.authenticationError;
errorCredentials = upgradeRoleError.credentials;
errorDetails = upgradeRoleError.message;
}

return {
_conference: authRequired,
_configHosts: configHosts,
_connecting: Boolean(connecting) || Boolean(upgradeRoleInProgress),
_error: error,
_errorCredentials: errorCredentials,
_errorDetails: errorDetails
_error: connectionError || upgradeRoleError
};
}

Expand Down
8 changes: 6 additions & 2 deletions react/features/authentication/middleware.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,10 +95,14 @@ MiddlewareRegistry.register(store => next => action => {
_hideLoginDialog(store);
break;

case CONNECTION_FAILED:
action.error === JitsiConnectionErrors.PASSWORD_REQUIRED
case CONNECTION_FAILED: {
const { error } = action;

error
&& error.name === JitsiConnectionErrors.PASSWORD_REQUIRED
&& store.dispatch(_openLoginDialog());
break;
}

case STOP_WAIT_FOR_OWNER:
_clearExistingWaitForOwnerTimeout(store);
Expand Down
3 changes: 1 addition & 2 deletions react/features/base/connection/actionTypes.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,7 @@ export const CONNECTION_ESTABLISHED = Symbol('CONNECTION_ESTABLISHED');
* {
* type: CONNECTION_FAILED,
* connection: JitsiConnection,
* error: string,
* message: string
* error: Object | string
* }
*/
export const CONNECTION_FAILED = Symbol('CONNECTION_FAILED');
Expand Down
24 changes: 15 additions & 9 deletions react/features/base/connection/actions.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,8 @@ export function connect(id: ?string, password: ?string) {
* disconnected.
*
* @param {string} message - Disconnect reason.
* @returns {void}
* @private
* @returns {void}
*/
function _onConnectionDisconnected(message: string) {
connection.removeEventListener(
Expand All @@ -69,8 +69,8 @@ export function connect(id: ?string, password: ?string) {
/**
* Resolves external promise when connection is established.
*
* @returns {void}
* @private
* @returns {void}
*/
function _onConnectionEstablished() {
unsubscribe();
Expand All @@ -86,8 +86,8 @@ 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.
* @returns {void}
* @private
* @returns {void}
*/
function _onConnectionFailed(err, msg, credentials) {
unsubscribe();
Expand Down Expand Up @@ -181,9 +181,7 @@ export function connectionEstablished(connection: Object) {
* @returns {{
* type: CONNECTION_FAILED,
* connection: JitsiConnection,
* credentials: Object,
* error: string,
* message: string
* error: Object
* }}
*/
export function connectionFailed(
Expand All @@ -194,9 +192,17 @@ export function connectionFailed(
return {
type: CONNECTION_FAILED,
connection,
credentials,
error,
message

// Make the error resemble an Error instance (to the extent that
// jitsi-meet needs it).
error: {
credentials:
credentials && Object.keys(credentials).length
? credentials
: undefined,
message,
name: error
}
};
}

Expand Down
17 changes: 5 additions & 12 deletions react/features/base/connection/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,7 @@ function _connectionEstablished(
return assign(state, {
connecting: undefined,
connection,
error: undefined,
errorMessage: undefined
error: undefined
});
}

Expand All @@ -93,11 +92,9 @@ function _connectionEstablished(
*/
function _connectionFailed(
state: Object,
{ connection, credentials, error, message }: {
{ connection, error }: {
connection: Object,
credentials: ?Object,
error: string,
message: ?string
error: Object | string
}) {
if (state.connection && state.connection !== connection) {
return state;
Expand All @@ -106,9 +103,7 @@ function _connectionFailed(
return assign(state, {
connecting: undefined,
connection: undefined,
credentials,
error,
errorMessage: message
error
});
}

Expand All @@ -127,9 +122,7 @@ function _connectionWillConnect(
{ connection }: { connection: Object }) {
return assign(state, {
connecting: connection,
credentials: undefined,
error: undefined,
errorMessage: undefined
error: undefined
});
}

Expand Down
10 changes: 7 additions & 3 deletions react/features/base/lib-jitsi-meet/functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,12 +39,16 @@ export function createLocalTrack(type: string, deviceId: string) {
* that category. I've currently named the category fatal because it appears to
* be used in the cases of unrecoverable errors that necessitate a reload.
*
* @param {string} error - The JitsiConnectionErrors instance to
* categorize/classify.
* @param {Object|string} error - The JitsiConnectionErrors instance to
* categorize/classify or an Error-like object.
* @returns {boolean} True if the specified JitsiConnectionErrors instance
* indicates a fatal JitsiConnection error; otherwise, false.
*/
export function isFatalJitsiConnectionError(error: string) {
export function isFatalJitsiConnectionError(error: Object | string) {
if (typeof error !== 'string') {
error = error.name; // eslint-disable-line no-param-reassign
}

return (
error === JitsiConnectionErrors.CONNECTION_DROPPED_ERROR
|| error === JitsiConnectionErrors.OTHER_ERROR
Expand Down
6 changes: 4 additions & 2 deletions react/features/overlay/reducer.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,10 @@ function _connectionEstablished(state) {
* @returns {Object} The new state of the feature overlay after the reduction of
* the specified action.
*/
function _connectionFailed(state, { error, message }) {
function _connectionFailed(state, { error }) {
if (isFatalJitsiConnectionError(error)) {
const { message } = error;

logger.error(`FATAL XMPP connection error: ${message}`);

return assign(state, {
Expand All @@ -99,7 +101,7 @@ function _connectionFailed(state, { error, message }) {
// From all of the cases above only CONNECTION_DROPPED_ERROR is
// considered a network type of failure.
isNetworkFailure:
error === JitsiConnectionErrors.CONNECTION_DROPPED_ERROR,
error.name === JitsiConnectionErrors.CONNECTION_DROPPED_ERROR,
reason: `xmpp-conn-dropped: ${message}`
});
}
Expand Down

0 comments on commit 4e0761a

Please sign in to comment.