-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Show an error message if resetting identity takes too long #34715
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
Changes from all commits
e6f43f0
e09bbf0
573a39d
7ecceb5
8c98442
d987084
94bfeb6
4cce460
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,8 @@ import { stubClient } from "test-utils"; | |
| import CompleteSecurity from "./CompleteSecurity"; | ||
| import { Phase, SetupEncryptionStore } from "../../../stores/SetupEncryptionStore"; | ||
| import SdkConfig from "../../../SdkConfig"; | ||
| import { sleep } from "matrix-js-sdk/src/utils"; | ||
| import { MatrixClientPeg } from "../../../MatrixClientPeg"; | ||
|
|
||
| class MockSetupEncryptionStore extends EventEmitter { | ||
| public phase: Phase = Phase.Intro; | ||
|
|
@@ -98,6 +100,26 @@ describe("CompleteSecurity", () => { | |
| expect(panel.getByRole("button", { name: "Continue" })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("Shows an error if reset times out", async () => { | ||
| const client = MatrixClientPeg.safeGet(); | ||
|
|
||
| // Given reset will freeze forever when we do it | ||
| client.getCrypto()!.resetEncryption = vi.fn().mockImplementation(() => sleep(20000)); | ||
| const store = new SetupEncryptionStore(); | ||
| vi.spyOn(SetupEncryptionStore, "sharedInstance").mockReturnValue(store); | ||
| const panel = await act(() => render(<CompleteSecurity onFinished={() => {}} resetTimeoutMs={1} />)); | ||
|
|
||
| // When we hit reset, then continue | ||
| await act(async () => panel.getByRole("button", { name: "Can't confirm?" }).click()); | ||
| await act(async () => panel.getByRole("button", { name: "Continue" }).click()); | ||
|
|
||
| // And wait more than the timeout | ||
| await sleep(10); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. rather than sleeping in a test, and threading a timeout value through 10 layers of components, could we use fake timers? |
||
|
|
||
| // Then an error dialog appears | ||
| expect(screen.getByRole("heading", { name: "Identity reset failed" })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it("Allows verifying with another device if one is available", async () => { | ||
| // Given a store and a dialog based on it | ||
| const store = new SetupEncryptionStore(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,13 +17,24 @@ | |
| import { EncryptionCardButtons } from "./EncryptionCardButtons"; | ||
| import { EncryptionCardEmphasisedContent } from "./EncryptionCardEmphasisedContent"; | ||
| import { useMatrixClientContext } from "../../../../contexts/MatrixClientContext"; | ||
| import { timeout } from "../../../../utils/promise"; | ||
|
|
||
| interface ResetIdentityBodyProps { | ||
| /** | ||
| * Called when the identity is reset. | ||
| */ | ||
| onReset: () => void; | ||
|
|
||
| // How long to wait for an identity reset before we assume it failed. | ||
| // | ||
| // Defaults to 5000ms if omitted. | ||
| resetTimeoutMs?: number; | ||
|
|
||
| /** | ||
| * Called when the identity reset fails. | ||
| */ | ||
| onFail: (failureReason: string) => void; | ||
|
|
||
| /** | ||
| * Called when the cancel button is clicked. | ||
| */ | ||
|
|
@@ -60,13 +71,45 @@ | |
| * | ||
| * Used by {@link ResetIdentityPanel}. | ||
| */ | ||
| export function ResetIdentityBody({ onCancelClick, onReset, variant }: ResetIdentityBodyProps): JSX.Element { | ||
| export function ResetIdentityBody({ | ||
| onCancelClick, | ||
| onReset, | ||
| resetTimeoutMs, | ||
| onFail, | ||
| variant, | ||
| }: ResetIdentityBodyProps): JSX.Element { | ||
|
Check warning on line 80 in apps/web/src/components/views/settings/encryption/ResetIdentityBody.tsx
|
||
| const matrixClient = useMatrixClientContext(); | ||
|
|
||
| // After the user clicks "Continue", we disable the button so it can't be | ||
| // clicked again, and warn the user not to close the window. | ||
| const [inProgress, setInProgress] = useState(false); | ||
|
|
||
| async function onClick(): Promise<void> { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't this and |
||
| setInProgress(true); | ||
|
|
||
| try { | ||
| const timedOut = "timed_out"; | ||
| const result = await timeout(doOnClick(), timedOut, resetTimeoutMs ?? 5000); | ||
|
|
||
| if (result === timedOut) { | ||
| onFail("Timed out"); | ||
| } else { | ||
| onReset(); | ||
| } | ||
| } catch (e: any) { | ||
| onFail(e.toString()); | ||
| } | ||
| } | ||
|
|
||
| async function doOnClick(): Promise<void> { | ||
| const crypto = matrixClient.getCrypto(); | ||
| if (!crypto) { | ||
| throw new Error("Crypto is not set up"); | ||
| } | ||
|
|
||
| await crypto.resetEncryption((makeRequest) => uiAuthCallback(matrixClient, makeRequest)); | ||
| } | ||
|
|
||
| return ( | ||
| <EncryptionCard Icon={ErrorIcon} destructive={true} title={titleForVariant(variant)}> | ||
| <EncryptionCardEmphasisedContent> | ||
|
|
@@ -84,17 +127,7 @@ | |
| {variant === "compromised" && <span>{_t("settings|encryption|advanced|breadcrumb_warning")}</span>} | ||
| </EncryptionCardEmphasisedContent> | ||
| <EncryptionCardButtons> | ||
| <Button | ||
| destructive={true} | ||
| disabled={inProgress} | ||
| onClick={async () => { | ||
| setInProgress(true); | ||
| await matrixClient | ||
| .getCrypto() | ||
| ?.resetEncryption((makeRequest) => uiAuthCallback(matrixClient, makeRequest)); | ||
| onReset(); | ||
| }} | ||
| > | ||
| <Button destructive={true} disabled={inProgress} onClick={onClick}> | ||
| {inProgress ? ( | ||
| <> | ||
| <InlineSpinner /> {_t("settings|encryption|advanced|reset_in_progress")} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"A promise that never resolves" can also be spelt
new Promise(() => {}), which might be better than having a dangling timeout?