Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed Improvements to error messaging issue #1091 #1166

Closed
wants to merge 13 commits into from
19 changes: 19 additions & 0 deletions src/libs/API.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import CONFIG from '../CONFIG';
import ONYXKEYS from '../ONYXKEYS';
import redirectToSignIn from './actions/SignInRedirect';
import * as Network from './Network';
import authenticationErrorMessages from './ErrorMessage/authenticationErrorMessages';

let isAuthenticating;

Expand Down Expand Up @@ -481,6 +482,23 @@ function SetPassword(parameters) {
return request(commandName, parameters);
}

/**
* Display a error message in the UI when the API returns an authentication error.
*
* @param {String} error
* @returns {String}
*/
function GetSignInErrorMessage(error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah what I meant was that you should probably place this in Authenticate. I also think you don't need a separate authenticationErrorMessages file either. It's overkill for what we're trying to achieve IMO

Copy link
Author

Choose a reason for hiding this comment

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

What I understood where you mentioned place in Authenticate. This means in src\libs\actions\Session.js file. So, This function GetSignInErrorMessage should be in this src\libs\actions\Session.js file, and no need separate file for authenticationErrorMessages. If you confirm me so I can proceed the changes.

Copy link
Author

Choose a reason for hiding this comment

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

@yuwenmemon Could you please confirm so I can push and close it?

const code = error.split(' ')[0];
if (!_.isEmpty(code)) {
const errorMessage = _.filter(authenticationErrorMessages, {errorCode: +code.trim()});
if (!_.isEmpty(errorMessage)) {
return errorMessage[0].errorMessage;
}
}
return error;
}

export {
getAuthToken,
Authenticate,
Expand All @@ -502,4 +520,5 @@ export {
SetGithubUsername,
SetPassword,
User_SignUp,
GetSignInErrorMessage,
};
31 changes: 31 additions & 0 deletions src/libs/ErrorMessage/authenticationErrorMessages.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
const authenticationErrorMessages = [
{
errorCode: 401,
errorMessage: 'Incorrect login or password. Please try again.',
},
{
errorCode: 402,
errorMessage:
'You have 2FA enabled on this account. Please sign in using your email or phone number.',
},
{
errorCode: 403,
errorMessage:
'Invalid login or password. Please try again or reset your password.',
},
{
errorCode: 404,
errorMessage:
'We were unable to change your password. This is likely due to an expired password reset link in an old password reset email. We have emailed you a new link so you can try again. Check your Inbox and your Spam folder; it should arrive in just a few minutes.',
},
{
errorCode: 405,
errorMessage:
'You do not have access to this application. Please add your GitHub username for access.',
},
{
errorCode: 413,
errorMessage: 'Your account has been locked.',
},
];
export default authenticationErrorMessages;
4 changes: 2 additions & 2 deletions src/libs/actions/Session.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,14 @@ function signIn(password, exitTo, twoFactorAuthCode) {
Onyx.merge(ONYXKEYS.CREDENTIALS, {autoGeneratedLogin, autoGeneratedPassword});
})
.catch((error) => {
Onyx.merge(ONYXKEYS.ACCOUNT, {error: error.message});
Onyx.merge(ONYXKEYS.ACCOUNT, {error: API.GetSignInErrorMessage(error.message)});
})
.finally(() => {
Onyx.merge(ONYXKEYS.ACCOUNT, {loading: false});
});
})
.catch((error) => {
Onyx.merge(ONYXKEYS.ACCOUNT, {error: error.message, loading: false});
Onyx.merge(ONYXKEYS.ACCOUNT, {error: API.GetSignInErrorMessage(error.message), loading: false});
});
}

Expand Down