Properly translate errors in ChangePassword.tsx so they show up translated to the user but not in our logs (#10615)

* Properly translate errors in `ChangePassword.tsx`

So they show up translated to the user but not in our logs.

Part of https://github.com/vector-im/element-web/issues/9597 and also fixes it
since it's the last piece mentioned (there could be other cases we log translated strings)

Fix https://github.com/vector-im/element-web/issues/9597

* Make more useful

* Update i18n strings

* No need to checkPassword since field validation already covers this

See https://github.com/matrix-org/matrix-react-sdk/pull/10615#discussion_r1167363765

Both of the error cases are covered by the logic in `verifyFieldsBeforeSubmit()` just above
and there is no way `checkPassword` would ever throw one of these errors since they are
already valid by the time it reaches here.

* Update i18n strings

* Revert "No need to checkPassword since field validation already covers this"

This reverts commit 7786dd151028e6fbf04d1a38a9c2cd47a3fbfc4b.

* Update i18n strings

* Add todo context to note that we can remove this logic in the future

* Ensure is an error

* Remove else

See https://github.com/matrix-org/matrix-react-sdk/pull/10615#discussion_r1173477053
This commit is contained in:
Eric Eastwood 2023-04-24 03:41:09 -05:00 committed by GitHub
parent e24e85f7a5
commit 16ab5e9db0
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 74 additions and 27 deletions

View file

@ -24,7 +24,7 @@ import { MatrixClientPeg } from "../../../MatrixClientPeg";
import AccessibleButton from "../elements/AccessibleButton"; import AccessibleButton from "../elements/AccessibleButton";
import Spinner from "../elements/Spinner"; import Spinner from "../elements/Spinner";
import withValidation, { IFieldState, IValidationResult } from "../elements/Validation"; import withValidation, { IFieldState, IValidationResult } from "../elements/Validation";
import { _t, _td } from "../../../languageHandler"; import { UserFriendlyError, _t, _td } from "../../../languageHandler";
import Modal from "../../../Modal"; import Modal from "../../../Modal";
import PassphraseField from "../auth/PassphraseField"; import PassphraseField from "../auth/PassphraseField";
import { PASSWORD_MIN_SCORE } from "../auth/RegistrationForm"; import { PASSWORD_MIN_SCORE } from "../auth/RegistrationForm";
@ -48,7 +48,7 @@ interface IProps {
/** Was one or more other devices logged out whilst changing the password */ /** Was one or more other devices logged out whilst changing the password */
didLogoutOutOtherDevices: boolean; didLogoutOutOtherDevices: boolean;
}) => void; }) => void;
onError: (error: { error: string }) => void; onError: (error: Error) => void;
rowClassName?: string; rowClassName?: string;
buttonClassName?: string; buttonClassName?: string;
buttonKind?: string; buttonKind?: string;
@ -183,7 +183,16 @@ export default class ChangePassword extends React.Component<IProps, IState> {
} }
}, },
(err) => { (err) => {
if (err instanceof Error) {
this.props.onError(err); this.props.onError(err);
} else {
this.props.onError(
new UserFriendlyError("Error while changing password: %(error)s", {
error: String(err),
cause: undefined,
}),
);
}
}, },
) )
.finally(() => { .finally(() => {
@ -196,15 +205,19 @@ export default class ChangePassword extends React.Component<IProps, IState> {
}); });
} }
private checkPassword(oldPass: string, newPass: string, confirmPass: string): { error: string } | undefined { /**
* Checks the `newPass` and throws an error if it is unacceptable.
* @param oldPass The old password
* @param newPass The new password that the user is trying to be set
* @param confirmPass The confirmation password where the user types the `newPass`
* again for confirmation and should match the `newPass` before we accept their new
* password.
*/
private checkPassword(oldPass: string, newPass: string, confirmPass: string): void {
if (newPass !== confirmPass) { if (newPass !== confirmPass) {
return { throw new UserFriendlyError("New passwords don't match");
error: _t("New passwords don't match"),
};
} else if (!newPass || newPass.length === 0) { } else if (!newPass || newPass.length === 0) {
return { throw new UserFriendlyError("Passwords can't be empty");
error: _t("Passwords can't be empty"),
};
} }
} }
@ -307,11 +320,24 @@ export default class ChangePassword extends React.Component<IProps, IState> {
const oldPassword = this.state.oldPassword; const oldPassword = this.state.oldPassword;
const newPassword = this.state.newPassword; const newPassword = this.state.newPassword;
const confirmPassword = this.state.newPasswordConfirm; const confirmPassword = this.state.newPasswordConfirm;
const err = this.checkPassword(oldPassword, newPassword, confirmPassword); try {
if (err) { // TODO: We can remove this check (but should add some Cypress tests to
// sanity check this flow). This logic is redundant with the input field
// validation we do and `verifyFieldsBeforeSubmit()` above. See
// https://github.com/matrix-org/matrix-react-sdk/pull/10615#discussion_r1167364214
this.checkPassword(oldPassword, newPassword, confirmPassword);
return this.onChangePassword(oldPassword, newPassword);
} catch (err) {
if (err instanceof Error) {
this.props.onError(err); this.props.onError(err);
} else { } else {
return this.onChangePassword(oldPassword, newPassword); this.props.onError(
new UserFriendlyError("Error while changing password: %(error)s", {
error: String(err),
cause: undefined,
}),
);
}
} }
}; };

View file

@ -21,9 +21,9 @@ import { SERVICE_TYPES } from "matrix-js-sdk/src/service-types";
import { IThreepid } from "matrix-js-sdk/src/@types/threepids"; import { IThreepid } from "matrix-js-sdk/src/@types/threepids";
import { logger } from "matrix-js-sdk/src/logger"; import { logger } from "matrix-js-sdk/src/logger";
import { IDelegatedAuthConfig, M_AUTHENTICATION } from "matrix-js-sdk/src/matrix"; import { IDelegatedAuthConfig, M_AUTHENTICATION } from "matrix-js-sdk/src/matrix";
import { MatrixError } from "matrix-js-sdk/src/matrix"; import { HTTPError } from "matrix-js-sdk/src/matrix";
import { _t } from "../../../../../languageHandler"; import { UserFriendlyError, _t } from "../../../../../languageHandler";
import ProfileSettings from "../../ProfileSettings"; import ProfileSettings from "../../ProfileSettings";
import * as languageHandler from "../../../../../languageHandler"; import * as languageHandler from "../../../../../languageHandler";
import SettingsStore from "../../../../../settings/SettingsStore"; import SettingsStore from "../../../../../settings/SettingsStore";
@ -43,7 +43,7 @@ import Spinner from "../../../elements/Spinner";
import { SettingLevel } from "../../../../../settings/SettingLevel"; import { SettingLevel } from "../../../../../settings/SettingLevel";
import { UIFeature } from "../../../../../settings/UIFeature"; import { UIFeature } from "../../../../../settings/UIFeature";
import { ActionPayload } from "../../../../../dispatcher/payloads"; import { ActionPayload } from "../../../../../dispatcher/payloads";
import ErrorDialog from "../../../dialogs/ErrorDialog"; import ErrorDialog, { extractErrorMessageFromError } from "../../../dialogs/ErrorDialog";
import AccountPhoneNumbers from "../../account/PhoneNumbers"; import AccountPhoneNumbers from "../../account/PhoneNumbers";
import AccountEmailAddresses from "../../account/EmailAddresses"; import AccountEmailAddresses from "../../account/EmailAddresses";
import DiscoveryEmailAddresses from "../../discovery/EmailAddresses"; import DiscoveryEmailAddresses from "../../discovery/EmailAddresses";
@ -260,18 +260,35 @@ export default class GeneralUserSettingsTab extends React.Component<IProps, ISta
PlatformPeg.get()?.setSpellCheckEnabled(spellCheckEnabled); PlatformPeg.get()?.setSpellCheckEnabled(spellCheckEnabled);
}; };
private onPasswordChangeError = (err: { error: string } & MatrixError): void => { private onPasswordChangeError = (err: Error): void => {
// TODO: Figure out a design that doesn't involve replacing the current dialog logger.error("Failed to change password: " + err);
let errMsg = err.error || err.message || "";
if (err.httpStatus === 403) { let underlyingError = err;
errMsg = _t("Failed to change password. Is your password correct?"); if (err instanceof UserFriendlyError && err.cause instanceof Error) {
} else if (!errMsg) { underlyingError = err.cause;
errMsg += ` (HTTP status ${err.httpStatus})`;
} }
logger.error("Failed to change password: " + errMsg);
const errorMessage = extractErrorMessageFromError(
err,
_t("Unknown password change error (%(stringifiedError)s)", {
stringifiedError: String(err),
}),
);
let errorMessageToDisplay = errorMessage;
if (underlyingError instanceof HTTPError && underlyingError.httpStatus === 403) {
errorMessageToDisplay = _t("Failed to change password. Is your password correct?");
} else if (underlyingError instanceof HTTPError) {
errorMessageToDisplay = _t("%(errorMessage)s (HTTP status %(httpStatus)s)", {
errorMessage,
httpStatus: underlyingError.httpStatus,
});
}
// TODO: Figure out a design that doesn't involve replacing the current dialog
Modal.createDialog(ErrorDialog, { Modal.createDialog(ErrorDialog, {
title: _t("Error"), title: _t("Error changing password"),
description: errMsg, description: errorMessageToDisplay,
}); });
}; };

View file

@ -1344,6 +1344,7 @@
"If you want to retain access to your chat history in encrypted rooms you should first export your room keys and re-import them afterwards.": "If you want to retain access to your chat history in encrypted rooms you should first export your room keys and re-import them afterwards.", "If you want to retain access to your chat history in encrypted rooms you should first export your room keys and re-import them afterwards.": "If you want to retain access to your chat history in encrypted rooms you should first export your room keys and re-import them afterwards.",
"You can also ask your homeserver admin to upgrade the server to change this behaviour.": "You can also ask your homeserver admin to upgrade the server to change this behaviour.", "You can also ask your homeserver admin to upgrade the server to change this behaviour.": "You can also ask your homeserver admin to upgrade the server to change this behaviour.",
"Export E2E room keys": "Export E2E room keys", "Export E2E room keys": "Export E2E room keys",
"Error while changing password: %(error)s": "Error while changing password: %(error)s",
"New passwords don't match": "New passwords don't match", "New passwords don't match": "New passwords don't match",
"Passwords can't be empty": "Passwords can't be empty", "Passwords can't be empty": "Passwords can't be empty",
"Do you want to set an email address?": "Do you want to set an email address?", "Do you want to set an email address?": "Do you want to set an email address?",
@ -1546,7 +1547,10 @@
"Set the name of a font installed on your system & %(brand)s will attempt to use it.": "Set the name of a font installed on your system & %(brand)s will attempt to use it.", "Set the name of a font installed on your system & %(brand)s will attempt to use it.": "Set the name of a font installed on your system & %(brand)s will attempt to use it.",
"Customise your appearance": "Customise your appearance", "Customise your appearance": "Customise your appearance",
"Appearance Settings only affect this %(brand)s session.": "Appearance Settings only affect this %(brand)s session.", "Appearance Settings only affect this %(brand)s session.": "Appearance Settings only affect this %(brand)s session.",
"Unknown password change error (%(stringifiedError)s)": "Unknown password change error (%(stringifiedError)s)",
"Failed to change password. Is your password correct?": "Failed to change password. Is your password correct?", "Failed to change password. Is your password correct?": "Failed to change password. Is your password correct?",
"%(errorMessage)s (HTTP status %(httpStatus)s)": "%(errorMessage)s (HTTP status %(httpStatus)s)",
"Error changing password": "Error changing password",
"Your password was successfully changed.": "Your password was successfully changed.", "Your password was successfully changed.": "Your password was successfully changed.",
"You will not receive push notifications on other devices until you sign back in to them.": "You will not receive push notifications on other devices until you sign back in to them.", "You will not receive push notifications on other devices until you sign back in to them.": "You will not receive push notifications on other devices until you sign back in to them.",
"Success": "Success", "Success": "Success",