Skip to content

Commit

Permalink
Properly translate errors in AddThreepid.ts (#10432)
Browse files Browse the repository at this point in the history 
* Properly translate errors in AddThreepid.ts

Part of element-hq/element-web#9597

* Use translated message

* Avoid returning undefined ever

* More usage

* Introduce UserFriendlyError

* Use UserFriendlyError

* Add more usage instead of normal error

* Use types and translatedMessage

* Fix lints

* Update i18n although it's wrong

* Use unknown for easier creation from try/catch

* Use types

* Use error types

* Use types

* Update i18n strings

* Remove generic re-label of HTTPError

See #10432 (comment)

The HTTPError already has a good label and it isn't even translated if we re-label it here in this way generically

Probably best to just remove in favor of thinking about a translations in general from the `matrix-js-sdk`, see matrix-org/matrix-js-sdk#1309

* Make error message extraction generic

* Update i18n strings

* Add tests for email addresses

* More consistent error logging to actually see error in logs

* Consistent error handling

* Any is okay because we have a fallback

* Check error type

* Use dedicated mockResolvedValue function

See #10432 (comment)
  • Loading branch information
@MadLittleMods
MadLittleMods committed Apr 14, 2023
1 parent 8f8b74b commit c1e7905
Show file tree
Hide file tree
Showing 9 changed files with 300 additions and 132 deletions.
165 changes: 83 additions & 82 deletions src/AddThreepid.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,21 @@ limitations under the License.
*/

import { IAuthData, IRequestMsisdnTokenResponse, IRequestTokenResponse } from "matrix-js-sdk/src/matrix";
import { MatrixError, HTTPError } from "matrix-js-sdk/src/matrix";

import { MatrixClientPeg } from "./MatrixClientPeg";
import Modal from "./Modal";
import { _t } from "./languageHandler";
import { _t, UserFriendlyError } from "./languageHandler";
import IdentityAuthClient from "./IdentityAuthClient";
import { SSOAuthEntry } from "./components/views/auth/InteractiveAuthEntryComponents";
import InteractiveAuthDialog from "./components/views/dialogs/InteractiveAuthDialog";

function getIdServerDomain(): string {
const idBaseUrl = MatrixClientPeg.get().idBaseUrl;
const idBaseUrl = MatrixClientPeg.get().getIdentityServerUrl(true);
if (!idBaseUrl) {
throw new Error("Identity server not set");
throw new UserFriendlyError("Identity server not set");
}
return idBaseUrl.split("://")[1];
return idBaseUrl;
}

export type Binding = {
Expand Down Expand Up @@ -67,23 +68,18 @@ export default class AddThreepid {
* @param {string} emailAddress The email address to add
* @return {Promise} Resolves when the email has been sent. Then call checkEmailLinkClicked().
*/
public addEmailAddress(emailAddress: string): Promise<IRequestTokenResponse> {
return MatrixClientPeg.get()
.requestAdd3pidEmailToken(emailAddress, this.clientSecret, 1)
.then(
(res) => {
this.sessionId = res.sid;
return res;
},
function (err) {
if (err.errcode === "M_THREEPID_IN_USE") {
err.message = _t("This email address is already in use");
} else if (err.httpStatus) {
err.message = err.message + ` (Status ${err.httpStatus})`;
}
throw err;
},
);
public async addEmailAddress(emailAddress: string): Promise<IRequestTokenResponse> {
try {
const res = await MatrixClientPeg.get().requestAdd3pidEmailToken(emailAddress, this.clientSecret, 1);
this.sessionId = res.sid;
return res;
} catch (err) {
if (err instanceof MatrixError && err.errcode === "M_THREEPID_IN_USE") {
throw new UserFriendlyError("This email address is already in use", { cause: err });
}
// Otherwise, just blurt out the same error
throw err;
}
}

/**
Expand All @@ -98,22 +94,23 @@ export default class AddThreepid {
// For separate bind, request a token directly from the IS.
const authClient = new IdentityAuthClient();
const identityAccessToken = (await authClient.getAccessToken()) ?? undefined;
return MatrixClientPeg.get()
.requestEmailToken(emailAddress, this.clientSecret, 1, undefined, identityAccessToken)
.then(
(res) => {
this.sessionId = res.sid;
return res;
},
function (err) {
if (err.errcode === "M_THREEPID_IN_USE") {
err.message = _t("This email address is already in use");
} else if (err.httpStatus) {
err.message = err.message + ` (Status ${err.httpStatus})`;
}
throw err;
},
try {
const res = await MatrixClientPeg.get().requestEmailToken(
emailAddress,
this.clientSecret,
1,
undefined,
identityAccessToken,
);
this.sessionId = res.sid;
return res;
} catch (err) {
if (err instanceof MatrixError && err.errcode === "M_THREEPID_IN_USE") {
throw new UserFriendlyError("This email address is already in use", { cause: err });
}
// Otherwise, just blurt out the same error
throw err;
}
} else {
// For tangled bind, request a token via the HS.
return this.addEmailAddress(emailAddress);
Expand All @@ -127,24 +124,24 @@ export default class AddThreepid {
* @param {string} phoneNumber The national or international formatted phone number to add
* @return {Promise} Resolves when the text message has been sent. Then call haveMsisdnToken().
*/
public addMsisdn(phoneCountry: string, phoneNumber: string): Promise<IRequestMsisdnTokenResponse> {
return MatrixClientPeg.get()
.requestAdd3pidMsisdnToken(phoneCountry, phoneNumber, this.clientSecret, 1)
.then(
(res) => {
this.sessionId = res.sid;
this.submitUrl = res.submit_url;
return res;
},
function (err) {
if (err.errcode === "M_THREEPID_IN_USE") {
err.message = _t("This phone number is already in use");
} else if (err.httpStatus) {
err.message = err.message + ` (Status ${err.httpStatus})`;
}
throw err;
},
public async addMsisdn(phoneCountry: string, phoneNumber: string): Promise<IRequestMsisdnTokenResponse> {
try {
const res = await MatrixClientPeg.get().requestAdd3pidMsisdnToken(
phoneCountry,
phoneNumber,
this.clientSecret,
1,
);
this.sessionId = res.sid;
this.submitUrl = res.submit_url;
return res;
} catch (err) {
if (err instanceof MatrixError && err.errcode === "M_THREEPID_IN_USE") {
throw new UserFriendlyError("This phone number is already in use", { cause: err });
}
// Otherwise, just blurt out the same error
throw err;
}
}

/**
Expand All @@ -160,22 +157,24 @@ export default class AddThreepid {
// For separate bind, request a token directly from the IS.
const authClient = new IdentityAuthClient();
const identityAccessToken = (await authClient.getAccessToken()) ?? undefined;
return MatrixClientPeg.get()
.requestMsisdnToken(phoneCountry, phoneNumber, this.clientSecret, 1, undefined, identityAccessToken)
.then(
(res) => {
this.sessionId = res.sid;
return res;
},
function (err) {
if (err.errcode === "M_THREEPID_IN_USE") {
err.message = _t("This phone number is already in use");
} else if (err.httpStatus) {
err.message = err.message + ` (Status ${err.httpStatus})`;
}
throw err;
},
try {
const res = await MatrixClientPeg.get().requestMsisdnToken(
phoneCountry,
phoneNumber,
this.clientSecret,
1,
undefined,
identityAccessToken,
);
this.sessionId = res.sid;
return res;
} catch (err) {
if (err instanceof MatrixError && err.errcode === "M_THREEPID_IN_USE") {
throw new UserFriendlyError("This phone number is already in use", { cause: err });
}
// Otherwise, just blurt out the same error
throw err;
}
} else {
// For tangled bind, request a token via the HS.
return this.addMsisdn(phoneCountry, phoneNumber);
Expand All @@ -195,7 +194,7 @@ export default class AddThreepid {
const authClient = new IdentityAuthClient();
const identityAccessToken = await authClient.getAccessToken();
if (!identityAccessToken) {
throw new Error("No identity access token found");
throw new UserFriendlyError("No identity access token found");
}
await MatrixClientPeg.get().bindThreePid({
sid: this.sessionId,
Expand All @@ -210,10 +209,10 @@ export default class AddThreepid {
// The spec has always required this to use UI auth but synapse briefly
// implemented it without, so this may just succeed and that's OK.
return [true];
} catch (e) {
if (e.httpStatus !== 401 || !e.data || !e.data.flows) {
} catch (err) {
if (!(err instanceof MatrixError) || err.httpStatus !== 401 || !err.data || !err.data.flows) {
// doesn't look like an interactive-auth failure
throw e;
throw err;
}

const dialogAesthetics = {
Expand All @@ -235,7 +234,7 @@ export default class AddThreepid {
const { finished } = Modal.createDialog(InteractiveAuthDialog, {
title: _t("Add Email Address"),
matrixClient: MatrixClientPeg.get(),
authData: e.data,
authData: err.data,
makeRequest: this.makeAddThreepidOnlyRequest,
aestheticsForStagePhases: {
[SSOAuthEntry.LOGIN_TYPE]: dialogAesthetics,
Expand All @@ -256,11 +255,13 @@ export default class AddThreepid {
);
}
} catch (err) {
if (err.httpStatus === 401) {
err.message = _t("Failed to verify email address: make sure you clicked the link in the email");
} else if (err.httpStatus) {
err.message += ` (Status ${err.httpStatus})`;
if (err instanceof HTTPError && err.httpStatus === 401) {
throw new UserFriendlyError(
"Failed to verify email address: make sure you clicked the link in the email",
{ cause: err },
);
}
// Otherwise, just blurt out the same error
throw err;
}
return [];
Expand Down Expand Up @@ -308,7 +309,7 @@ export default class AddThreepid {
await authClient.getAccessToken(),
);
} else {
throw new Error("The add / bind with MSISDN flow is misconfigured");
throw new UserFriendlyError("The add / bind with MSISDN flow is misconfigured");
}
if (result.errcode) {
throw result;
Expand All @@ -329,10 +330,10 @@ export default class AddThreepid {
// The spec has always required this to use UI auth but synapse briefly
// implemented it without, so this may just succeed and that's OK.
return;
} catch (e) {
if (e.httpStatus !== 401 || !e.data || !e.data.flows) {
} catch (err) {
if (!(err instanceof MatrixError) || err.httpStatus !== 401 || !err.data || !err.data.flows) {
// doesn't look like an interactive-auth failure
throw e;
throw err;
}

const dialogAesthetics = {
Expand All @@ -354,7 +355,7 @@ export default class AddThreepid {
const { finished } = Modal.createDialog(InteractiveAuthDialog, {
title: _t("Add Phone Number"),
matrixClient: MatrixClientPeg.get(),
authData: e.data,
authData: err.data,
makeRequest: this.makeAddThreepidOnlyRequest,
aestheticsForStagePhases: {
[SSOAuthEntry.LOGIN_TYPE]: dialogAesthetics,
Expand Down
19 changes: 18 additions & 1 deletion src/components/views/dialogs/ErrorDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,26 @@ limitations under the License.

import React from "react";

import { _t } from "../../../languageHandler";
import { _t, UserFriendlyError } from "../../../languageHandler";
import BaseDialog from "./BaseDialog";

/**
* Get a user friendly error message string from a given error. Useful for the
* `description` prop of the `ErrorDialog`
* @param err Error object in question to extract a useful message from. To make it easy
* to use with try/catch, this is typed as `any` because try/catch will type
* the error as `unknown`. And in any case we can use the fallback message.
* @param translatedFallbackMessage The fallback message to be used if the error doesn't have any message
* @returns a user friendly error message string from a given error
*/
export function extractErrorMessageFromError(err: any, translatedFallbackMessage: string): string {
return (
(err instanceof UserFriendlyError && err.translatedMessage) ||
(err instanceof Error && err.message) ||
translatedFallbackMessage
);
}

interface IProps {
onFinished: (success?: boolean) => void;
title?: string;
Expand Down
17 changes: 12 additions & 5 deletions src/components/views/dialogs/SetEmailDialog.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ limitations under the License.

import React from "react";
import { logger } from "matrix-js-sdk/src/logger";
import { MatrixError } from "matrix-js-sdk/src/matrix";

import * as Email from "../../../email";
import AddThreepid from "../../../AddThreepid";
import { _t } from "../../../languageHandler";
import { _t, UserFriendlyError } from "../../../languageHandler";
import Modal from "../../../Modal";
import Spinner from "../elements/Spinner";
import ErrorDialog from "./ErrorDialog";
import ErrorDialog, { extractErrorMessageFromError } from "./ErrorDialog";
import QuestionDialog from "./QuestionDialog";
import BaseDialog from "./BaseDialog";
import EditableText from "../elements/EditableText";
Expand Down Expand Up @@ -88,7 +89,7 @@ export default class SetEmailDialog extends React.Component<IProps, IState> {
logger.error("Unable to add email address " + emailAddress + " " + err);
Modal.createDialog(ErrorDialog, {
title: _t("Unable to add email address"),
description: err && err.message ? err.message : _t("Operation failed"),
description: extractErrorMessageFromError(err, _t("Operation failed")),
});
},
);
Expand All @@ -114,7 +115,13 @@ export default class SetEmailDialog extends React.Component<IProps, IState> {
},
(err) => {
this.setState({ emailBusy: false });
if (err.errcode == "M_THREEPID_AUTH_FAILED") {

let underlyingError = err;
if (err instanceof UserFriendlyError) {
underlyingError = err.cause;
}

if (underlyingError instanceof MatrixError && underlyingError.errcode === "M_THREEPID_AUTH_FAILED") {
const message =
_t("Unable to verify email address.") +
" " +
Expand All @@ -131,7 +138,7 @@ export default class SetEmailDialog extends React.Component<IProps, IState> {
logger.error("Unable to verify email address: " + err);
Modal.createDialog(ErrorDialog, {
title: _t("Unable to verify email address."),
description: err && err.message ? err.message : _t("Operation failed"),
description: extractErrorMessageFromError(err, _t("Operation failed")),
});
}
},
Expand Down
Loading

0 comments on commit c1e7905

Please sign in to comment.