ChangeRecoveryKey: error handling (#29262)

* CreateSecretStorageDialog: error handling

I'm fed up with setup operations in EW failing silently. Rather than leaving
the user with a mysteriously broken client, let's at least tell them that
something has gone wrong, so that they can report the issue and we can
investigate.

Obviously, showing an unactionable Error dialog is a last resort: ideally, we
should handle the error ourselves, or give the user actionable steps to resolve
the problem. But that takes significant design and engineering.

Just swallowing errors is the worst of all possible options.

* Fix typo in test name

* Improve test coverage

src/components/views/settings/encryption/ChangeRecoveryKey.tsx

@@ -19,7 +19,6 @@ import {
 } from "@vector-im/compound-web";
 import CopyIcon from "@vector-im/compound-design-tokens/assets/web/icons/copy";
 import KeyIcon from "@vector-im/compound-design-tokens/assets/web/icons/key-solid";
-import { logger } from "matrix-js-sdk/src/logger";
 
 import { _t } from "../../../../languageHandler";
 import { EncryptionCard } from "./EncryptionCard";
@@ -28,6 +27,7 @@ import { useAsyncMemo } from "../../../../hooks/useAsyncMemo";
 import { copyPlaintext } from "../../../../utils/strings";
 import { initialiseDehydrationIfEnabled } from "../../../../utils/device/dehydration.ts";
 import { withSecretStorageKeyCache } from "../../../../SecurityManager";
+import { logErrorAndShowErrorDialog } from "../../../../utils/ErrorUtils.tsx";
 
 /**
  * The possible states of the component.
@@ -132,7 +132,7 @@ export function ChangeRecoveryKey({
                             });
                             onFinish();
                         } catch (e) {
-                            logger.error("Failed to bootstrap secret storage", e);
+                            logErrorAndShowErrorDialog("Failed to set up secret storage", e);
                         }
                     }}
                     submitButtonLabel={

src/utils/ErrorUtils.tsx

@@ -8,11 +8,14 @@ Please see LICENSE files in the repository root for full details.
 
 import React, { type ReactNode } from "react";
 import { MatrixError, ConnectionError } from "matrix-js-sdk/src/matrix";
+import { logger } from "matrix-js-sdk/src/logger";
 
 import { _t, _td, lookupString, type Tags, type TranslatedString, type TranslationKey } from "../languageHandler";
 import SdkConfig from "../SdkConfig";
 import { type ValidatedServerConfig } from "./ValidatedServerConfig";
 import ExternalLink from "../components/views/elements/ExternalLink";
+import Modal from "../Modal.tsx";
+import ErrorDialog from "../components/views/dialogs/ErrorDialog.tsx";
 
 export const resourceLimitStrings = {
     "monthly_active_user": _td("error|mau"),
@@ -191,3 +194,27 @@ export function messageForConnectionError(
 
     return errorText;
 }
+
+/**
+ * Utility for handling unexpected errors: pops up the error dialog.
+ *
+ * Example usage:
+ * ```
+ * try {
+ *     /// complicated operation
+ * } catch (e) {
+ *     logErrorAndShowErrorDialog("Failed complicated operation", e);
+ * }
+ * ```
+ *
+ * This isn't particularly intended to be pretty; rather it lets the user know that *something* has gone wrong so that
+ * they can report a bug. The general idea is that it's better to let the user know of a failure, even if they
+ * can't do anything about it, than it is to fail silently with the appearance of success.
+ *
+ * @param title - Title for the error dialog.
+ * @param error - The thrown error. Becomes the content of the error dialog.
+ */
+export function logErrorAndShowErrorDialog(title: string, error: any): void {
+    logger.error(`${title}:`, error);
+    Modal.createDialog(ErrorDialog, { title, description: `${error}` });
+}

test/unit-tests/components/views/settings/encryption/ChangeRecoveryKey-test.tsx

@@ -9,6 +9,7 @@ import React from "react";
 import { render, screen, waitFor } from "jest-matrix-react";
 import { type MatrixClient } from "matrix-js-sdk/src/matrix";
 import userEvent from "@testing-library/user-event";
+import { mocked } from "jest-mock";
 
 import { ChangeRecoveryKey } from "../../../../../../src/components/views/settings/encryption/ChangeRecoveryKey";
 import { createTestClient, withClientContextRenderOptions } from "../../../../../test-utils";
@@ -18,6 +19,10 @@ jest.mock("../../../../../../src/utils/strings", () => ({
     copyPlaintext: jest.fn(),
 }));
 
+afterEach(() => {
+    jest.restoreAllMocks();
+});
+
 describe("<ChangeRecoveryKey />", () => {
     let matrixClient: MatrixClient;
 
@@ -36,7 +41,7 @@ describe("<ChangeRecoveryKey />", () => {
         );
     }
 
-    describe("flow to setup a recovery key", () => {
+    describe("flow to set up a recovery key", () => {
         it("should display information about the recovery key", async () => {
             const user = userEvent.setup();
 
@@ -107,6 +112,33 @@ describe("<ChangeRecoveryKey />", () => {
             await user.click(finishButton);
             expect(onFinish).toHaveBeenCalledWith();
         });
+
+        it("should display errors from bootstrapSecretStorage", async () => {
+            const consoleErrorSpy = jest.spyOn(console, "error").mockReturnValue(undefined);
+            mocked(matrixClient.getCrypto()!).bootstrapSecretStorage.mockRejectedValue(new Error("can't bootstrap"));
+
+            const user = userEvent.setup();
+            renderComponent(false);
+
+            // Display the recovery key to save
+            await waitFor(() => user.click(screen.getByRole("button", { name: "Continue" })));
+            // Display the form to confirm the recovery key
+            await waitFor(() => user.click(screen.getByRole("button", { name: "Continue" })));
+
+            await waitFor(() => expect(screen.getByText("Enter your recovery key to confirm")).toBeInTheDocument());
+
+            const finishButton = screen.getByRole("button", { name: "Finish set up" });
+            const input = screen.getByRole("textbox");
+            await userEvent.type(input, "encoded private key");
+            await user.click(finishButton);
+
+            await screen.findByText("Failed to set up secret storage");
+            await screen.findByText("Error: can't bootstrap");
+            expect(consoleErrorSpy).toHaveBeenCalledWith(
+                "Failed to set up secret storage:",
+                new Error("can't bootstrap"),
+            );
+        });
     });
 
     describe("flow to change the recovery key", () => {

test/unit-tests/components/views/settings/encryption/__snapshots__/ChangeRecoveryKey-test.tsx.snap

@@ -160,7 +160,7 @@ exports[`<ChangeRecoveryKey /> flow to change the recovery key should display th
 </DocumentFragment>
 `;
 
-exports[`<ChangeRecoveryKey /> flow to setup a recovery key should ask the user to enter the recovery key 1`] = `
+exports[`<ChangeRecoveryKey /> flow to set up a recovery key should ask the user to enter the recovery key 1`] = `
 <DocumentFragment>
   <nav
     class="_breadcrumb_ikpbb_17"
@@ -293,7 +293,7 @@ exports[`<ChangeRecoveryKey /> flow to setup a recovery key should ask the user
 </DocumentFragment>
 `;
 
-exports[`<ChangeRecoveryKey /> flow to setup a recovery key should ask the user to enter the recovery key 2`] = `
+exports[`<ChangeRecoveryKey /> flow to set up a recovery key should ask the user to enter the recovery key 2`] = `
 <DocumentFragment>
   <nav
     class="_breadcrumb_ikpbb_17"
@@ -448,7 +448,7 @@ exports[`<ChangeRecoveryKey /> flow to setup a recovery key should ask the user
 </DocumentFragment>
 `;
 
-exports[`<ChangeRecoveryKey /> flow to setup a recovery key should display information about the recovery key 1`] = `
+exports[`<ChangeRecoveryKey /> flow to set up a recovery key should display information about the recovery key 1`] = `
 <DocumentFragment>
   <nav
     class="_breadcrumb_ikpbb_17"
@@ -564,7 +564,7 @@ exports[`<ChangeRecoveryKey /> flow to setup a recovery key should display infor
 </DocumentFragment>
 `;
 
-exports[`<ChangeRecoveryKey /> flow to setup a recovery key should display the recovery key 1`] = `
+exports[`<ChangeRecoveryKey /> flow to set up a recovery key should display the recovery key 1`] = `
 <DocumentFragment>
   <nav
     class="_breadcrumb_ikpbb_17"