diff --git a/src/AddThreepid.ts b/src/AddThreepid.ts index 2c37d002374..5121fdb51a8 100644 --- a/src/AddThreepid.ts +++ b/src/AddThreepid.ts @@ -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 = { @@ -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 { - 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 { + 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; + } } /** @@ -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); @@ -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 { - 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 { + 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; + } } /** @@ -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); @@ -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, @@ -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 = { @@ -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, @@ -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 []; @@ -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; @@ -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 = { @@ -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, diff --git a/src/components/views/dialogs/ErrorDialog.tsx b/src/components/views/dialogs/ErrorDialog.tsx index 4a82941767a..b4cdf20fc06 100644 --- a/src/components/views/dialogs/ErrorDialog.tsx +++ b/src/components/views/dialogs/ErrorDialog.tsx @@ -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; diff --git a/src/components/views/dialogs/SetEmailDialog.tsx b/src/components/views/dialogs/SetEmailDialog.tsx index ec41240c469..b5b148ea203 100644 --- a/src/components/views/dialogs/SetEmailDialog.tsx +++ b/src/components/views/dialogs/SetEmailDialog.tsx @@ -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"; @@ -88,7 +89,7 @@ export default class SetEmailDialog extends React.Component { 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")), }); }, ); @@ -114,7 +115,13 @@ export default class SetEmailDialog extends React.Component { }, (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.") + " " + @@ -131,7 +138,7 @@ export default class SetEmailDialog extends React.Component { 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")), }); } }, diff --git a/src/components/views/settings/account/EmailAddresses.tsx b/src/components/views/settings/account/EmailAddresses.tsx index 0d5405d93fb..5605bfacb66 100644 --- a/src/components/views/settings/account/EmailAddresses.tsx +++ b/src/components/views/settings/account/EmailAddresses.tsx @@ -18,15 +18,16 @@ limitations under the License. import React from "react"; import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids"; import { logger } from "matrix-js-sdk/src/logger"; +import { MatrixError } from "matrix-js-sdk/src/matrix"; -import { _t } from "../../../../languageHandler"; +import { _t, UserFriendlyError } from "../../../../languageHandler"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; import Field from "../../elements/Field"; import AccessibleButton from "../../elements/AccessibleButton"; import * as Email from "../../../../email"; import AddThreepid from "../../../../AddThreepid"; import Modal from "../../../../Modal"; -import ErrorDialog from "../../dialogs/ErrorDialog"; +import ErrorDialog, { extractErrorMessageFromError } from "../../dialogs/ErrorDialog"; /* TODO: Improve the UX for everything in here. @@ -190,7 +191,7 @@ export default class EmailAddresses extends React.Component { this.setState({ verifying: false, continueDisabled: false, addTask: null }); 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")), }); }); }; @@ -218,8 +219,16 @@ export default class EmailAddresses extends React.Component { }); }) .catch((err) => { + logger.error("Unable to verify email address: ", err); + this.setState({ continueDisabled: 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") { Modal.createDialog(ErrorDialog, { title: _t("Your email address hasn't been verified yet"), description: _t( @@ -227,10 +236,9 @@ export default class EmailAddresses extends React.Component { ), }); } else { - 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")), }); } }); diff --git a/src/components/views/settings/account/PhoneNumbers.tsx b/src/components/views/settings/account/PhoneNumbers.tsx index ac8e2ab244b..52803d98c3c 100644 --- a/src/components/views/settings/account/PhoneNumbers.tsx +++ b/src/components/views/settings/account/PhoneNumbers.tsx @@ -19,14 +19,14 @@ import React from "react"; import { IThreepid, ThreepidMedium } from "matrix-js-sdk/src/@types/threepids"; import { logger } from "matrix-js-sdk/src/logger"; -import { _t } from "../../../../languageHandler"; +import { _t, UserFriendlyError } from "../../../../languageHandler"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; import Field from "../../elements/Field"; import AccessibleButton from "../../elements/AccessibleButton"; import AddThreepid from "../../../../AddThreepid"; import CountryDropdown from "../../auth/CountryDropdown"; import Modal from "../../../../Modal"; -import ErrorDialog from "../../dialogs/ErrorDialog"; +import ErrorDialog, { extractErrorMessageFromError } from "../../dialogs/ErrorDialog"; import { PhoneNumberCountryDefinition } from "../../../../phonenumber"; /* @@ -81,7 +81,7 @@ export class ExistingPhoneNumber extends React.Component { this.setState({ verifying: false, continueDisabled: false, addTask: null }); Modal.createDialog(ErrorDialog, { title: _t("Error"), - description: err && err.message ? err.message : _t("Operation failed"), + description: extractErrorMessageFromError(err, _t("Operation failed")), }); }); }; @@ -224,12 +224,18 @@ export default class PhoneNumbers extends React.Component { }); }) .catch((err) => { + logger.error("Unable to verify phone number: " + err); this.setState({ continueDisabled: false }); - if (err.errcode !== "M_THREEPID_AUTH_FAILED") { - logger.error("Unable to verify phone number: " + err); + + let underlyingError = err; + if (err instanceof UserFriendlyError) { + underlyingError = err.cause; + } + + if (underlyingError.errcode !== "M_THREEPID_AUTH_FAILED") { Modal.createDialog(ErrorDialog, { title: _t("Unable to verify phone number."), - description: err && err.message ? err.message : _t("Operation failed"), + description: extractErrorMessageFromError(err, _t("Operation failed")), }); } else { this.setState({ verifyError: _t("Incorrect verification code") }); diff --git a/src/components/views/settings/discovery/EmailAddresses.tsx b/src/components/views/settings/discovery/EmailAddresses.tsx index 18cf49a1629..7c524266a4e 100644 --- a/src/components/views/settings/discovery/EmailAddresses.tsx +++ b/src/components/views/settings/discovery/EmailAddresses.tsx @@ -18,12 +18,13 @@ limitations under the License. import React from "react"; import { IThreepid } from "matrix-js-sdk/src/@types/threepids"; import { logger } from "matrix-js-sdk/src/logger"; +import { MatrixError } from "matrix-js-sdk/src/matrix"; -import { _t } from "../../../../languageHandler"; +import { _t, UserFriendlyError } from "../../../../languageHandler"; import { MatrixClientPeg } from "../../../../MatrixClientPeg"; import Modal from "../../../../Modal"; import AddThreepid, { Binding } from "../../../../AddThreepid"; -import ErrorDialog from "../../dialogs/ErrorDialog"; +import ErrorDialog, { extractErrorMessageFromError } from "../../dialogs/ErrorDialog"; import AccessibleButton from "../../elements/AccessibleButton"; /* @@ -98,7 +99,7 @@ export class EmailAddress extends React.Component + jest.fn().mockImplementation(() => ({ + getAccessToken: mockGetAccessToken, + })), +); + +const emailThreepidFixture: IThreepid = { + medium: ThreepidMedium.Email, + address: "foo@bar.com", + validated_at: 12345, + added_at: 12342, + bound: false, +}; describe("", () => { + const mockClient = getMockClientWithEventEmitter({ + getIdentityServerUrl: jest.fn().mockReturnValue("https://fake-identity-server"), + generateClientSecret: jest.fn(), + doesServerSupportSeparateAddAndBind: jest.fn(), + requestEmailToken: jest.fn(), + bindThreePid: jest.fn(), + }); + + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(async () => { + jest.useRealTimers(); + await clearAllModals(); + }); + it("should track props.email.bound changes", async () => { - const email: IThreepid = { - medium: ThreepidMedium.Email, - address: "foo@bar.com", - validated_at: 12345, - added_at: 12342, - bound: false, - }; - - const { rerender } = render(); + const { rerender } = render(); await screen.findByText("Share"); - email.bound = true; - rerender(); + rerender( + , + ); await screen.findByText("Revoke"); }); + + describe("Email verification share phase", () => { + it("shows translated error message", async () => { + render(); + mockClient.doesServerSupportSeparateAddAndBind.mockResolvedValue(true); + mockClient.requestEmailToken.mockRejectedValue( + new MatrixError( + { errcode: "M_THREEPID_IN_USE", error: "Some fake MatrixError occured" }, + 400, + "https://fake-url/", + ), + ); + fireEvent.click(screen.getByText("Share")); + + // Expect error dialog/modal to be shown. We have to wait for the UI to transition. + expect(await screen.findByText("This email address is already in use")).toBeInTheDocument(); + }); + }); + + describe("Email verification complete phase", () => { + beforeEach(async () => { + // Start these tests out at the "Complete" phase + render(); + mockClient.requestEmailToken.mockResolvedValue({ sid: "123-fake-sid" } satisfies IRequestTokenResponse); + mockClient.doesServerSupportSeparateAddAndBind.mockResolvedValue(true); + fireEvent.click(screen.getByText("Share")); + // Then wait for the completion screen to come up + await screen.findByText("Complete"); + }); + + it("Shows error dialog when share completion fails (email not verified yet)", async () => { + mockClient.bindThreePid.mockRejectedValue( + new MatrixError( + { errcode: "M_THREEPID_AUTH_FAILED", error: "Some fake MatrixError occured" }, + 403, + "https://fake-url/", + ), + ); + fireEvent.click(screen.getByText("Complete")); + + // Expect error dialog/modal to be shown. We have to wait for the UI to transition. + // Check the title + expect(await screen.findByText("Your email address hasn't been verified yet")).toBeInTheDocument(); + // Check the description + expect( + await screen.findByText( + "Click the link in the email you received to verify and then click continue again.", + ), + ).toBeInTheDocument(); + }); + + it("Shows error dialog when share completion fails (UserFriendlyError)", async () => { + const fakeErrorText = "Fake UserFriendlyError error in test"; + mockClient.bindThreePid.mockRejectedValue(new UserFriendlyError(fakeErrorText)); + fireEvent.click(screen.getByText("Complete")); + + // Expect error dialog/modal to be shown. We have to wait for the UI to transition. + // Check the title + expect(await screen.findByText("Unable to verify email address.")).toBeInTheDocument(); + // Check the description + expect(await screen.findByText(fakeErrorText)).toBeInTheDocument(); + }); + + it("Shows error dialog when share completion fails (generic error)", async () => { + const fakeErrorText = "Fake plain error in test"; + mockClient.bindThreePid.mockRejectedValue(new Error(fakeErrorText)); + fireEvent.click(screen.getByText("Complete")); + + // Expect error dialog/modal to be shown. We have to wait for the UI to transition. + // Check the title + expect(await screen.findByText("Unable to verify email address.")).toBeInTheDocument(); + // Check the description + expect(await screen.findByText(fakeErrorText)).toBeInTheDocument(); + }); + }); });