From 1403cd851aa7af13158619ebb5a958528e0296b8 Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 26 Feb 2024 15:30:32 +0000 Subject: [PATCH] Fix spurious session corruption error (#12280) * Fix spurious session corruption error Move the server versions check to each time we reconnect to the server rather than the first time,although, as per comment it will still only trigger the first time, but it will avoid us awaiting and mean we know we're connected to the server when we try, and get automatic retries. Fixes https://github.com/element-hq/element-web/issues/26967 * Move test & add regression test * Write some more tests * More comments & catch exceptions in server versions check * Note caching behaviour * Typo Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> * Remove the bit of the comment that might be wrong --------- Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- src/Lifecycle.ts | 34 ------------ src/stores/LifecycleStore.ts | 57 ++++++++++++++++++++ test/Lifecycle-test.ts | 12 +---- test/stores/LifecycleStore-test.ts | 86 ++++++++++++++++++++++++++++++ 4 files changed, 145 insertions(+), 44 deletions(-) create mode 100644 test/stores/LifecycleStore-test.ts diff --git a/src/Lifecycle.ts b/src/Lifecycle.ts index 980fd9ae1a..b6fce3b636 100644 --- a/src/Lifecycle.ts +++ b/src/Lifecycle.ts @@ -23,7 +23,6 @@ import { InvalidStoreError } from "matrix-js-sdk/src/errors"; import { IEncryptedPayload } from "matrix-js-sdk/src/crypto/aes"; import { QueryDict } from "matrix-js-sdk/src/utils"; import { logger } from "matrix-js-sdk/src/logger"; -import { MINIMUM_MATRIX_VERSION, SUPPORTED_MATRIX_VERSIONS } from "matrix-js-sdk/src/version-support"; import { IMatrixClientCreds, MatrixClientPeg } from "./MatrixClientPeg"; import SecurityCustomisations from "./customisations/Security"; @@ -74,7 +73,6 @@ import { getStoredOidcTokenIssuer, persistOidcAuthenticatedSettings, } from "./utils/oidc/persistOidcSettings"; -import GenericToast from "./components/views/toasts/GenericToast"; import { ACCESS_TOKEN_IV, ACCESS_TOKEN_STORAGE_KEY, @@ -647,7 +645,6 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): }, false, ); - await checkServerVersions(); return true; } else { logger.log("No previous session found."); @@ -655,37 +652,6 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }): } } -async function checkServerVersions(): Promise { - const client = MatrixClientPeg.get(); - if (!client) return; - for (const version of SUPPORTED_MATRIX_VERSIONS) { - // Check if the server supports this spec version. (`isVersionSupported` caches the response, so this loop will - // only make a single HTTP request). - if (await client.isVersionSupported(version)) { - // we found a compatible spec version - return; - } - } - - const toastKey = "LEGACY_SERVER"; - ToastStore.sharedInstance().addOrReplaceToast({ - key: toastKey, - title: _t("unsupported_server_title"), - props: { - description: _t("unsupported_server_description", { - version: MINIMUM_MATRIX_VERSION, - brand: SdkConfig.get().brand, - }), - acceptLabel: _t("action|ok"), - onAccept: () => { - ToastStore.sharedInstance().dismissToast(toastKey); - }, - }, - component: GenericToast, - priority: 98, - }); -} - async function handleLoadSessionFailure(e: unknown): Promise { logger.error("Unable to load session", e); diff --git a/src/stores/LifecycleStore.ts b/src/stores/LifecycleStore.ts index e4da964553..decef11881 100644 --- a/src/stores/LifecycleStore.ts +++ b/src/stores/LifecycleStore.ts @@ -14,11 +14,20 @@ See the License for the specific language governing permissions and limitations under the License. */ +import { SyncState } from "matrix-js-sdk/src/matrix"; +import { MINIMUM_MATRIX_VERSION, SUPPORTED_MATRIX_VERSIONS } from "matrix-js-sdk/src/version-support"; +import { logger } from "matrix-js-sdk/src/logger"; + import { Action } from "../dispatcher/actions"; import dis from "../dispatcher/dispatcher"; import { ActionPayload } from "../dispatcher/payloads"; import { DoAfterSyncPreparedPayload } from "../dispatcher/payloads/DoAfterSyncPreparedPayload"; import { AsyncStore } from "./AsyncStore"; +import { MatrixClientPeg } from "../MatrixClientPeg"; +import ToastStore from "./ToastStore"; +import { _t } from "../languageHandler"; +import SdkConfig from "../SdkConfig"; +import GenericToast from "../components/views/toasts/GenericToast"; interface IState { deferredAction: ActionPayload | null; @@ -51,6 +60,12 @@ class LifecycleStore extends AsyncStore { }); break; case "MatrixActions.sync": { + if (payload.state === SyncState.Syncing && payload.prevState !== SyncState.Syncing) { + // We've reconnected to the server: update server version support + // This is async but we don't care about the result, so just fire & forget. + checkServerVersions(); + } + if (payload.state !== "PREPARED") { break; } @@ -70,6 +85,48 @@ class LifecycleStore extends AsyncStore { } } +async function checkServerVersions(): Promise { + try { + const client = MatrixClientPeg.get(); + if (!client) return; + for (const version of SUPPORTED_MATRIX_VERSIONS) { + // Check if the server supports this spec version. (`isVersionSupported` caches the response, so this loop will + // only make a single HTTP request). + // Note that although we do this on a reconnect, we cache the server's versions in memory + // indefinitely, so it will only ever trigger the toast on the first connection after a fresh + // restart of the client. + if (await client.isVersionSupported(version)) { + // we found a compatible spec version + return; + } + } + + // This is retrospective doc having debated about the exactly what this toast is for, but + // our guess is that it's a nudge to update, or ask your HS admin to update your Homeserver + // after a new version of Element has come out, in a way that doesn't lock you out of all + // your messages. + const toastKey = "LEGACY_SERVER"; + ToastStore.sharedInstance().addOrReplaceToast({ + key: toastKey, + title: _t("unsupported_server_title"), + props: { + description: _t("unsupported_server_description", { + version: MINIMUM_MATRIX_VERSION, + brand: SdkConfig.get().brand, + }), + acceptLabel: _t("action|ok"), + onAccept: () => { + ToastStore.sharedInstance().dismissToast(toastKey); + }, + }, + component: GenericToast, + priority: 98, + }); + } catch (e) { + logger.warn("Failed to check server versions", e); + } +} + let singletonLifecycleStore: LifecycleStore | null = null; if (!singletonLifecycleStore) { singletonLifecycleStore = new LifecycleStore(); diff --git a/test/Lifecycle-test.ts b/test/Lifecycle-test.ts index 9c998734bf..fac59b235a 100644 --- a/test/Lifecycle-test.ts +++ b/test/Lifecycle-test.ts @@ -28,7 +28,6 @@ import { MatrixClientPeg } from "../src/MatrixClientPeg"; import Modal from "../src/Modal"; import * as StorageManager from "../src/utils/StorageManager"; import { flushPromises, getMockClientWithEventEmitter, mockClientMethodsUser, mockPlatformPeg } from "./test-utils"; -import ToastStore from "../src/stores/ToastStore"; import { OidcClientStore } from "../src/stores/oidc/OidcClientStore"; import { makeDelegatedAuthConfig } from "./test-utils/oidc"; import { persistOidcAuthenticatedSettings } from "../src/utils/oidc/persistOidcSettings"; @@ -452,17 +451,10 @@ describe("Lifecycle", () => { }); }); - it("should show a toast if the matrix server version is unsupported", async () => { - const toastSpy = jest.spyOn(ToastStore.sharedInstance(), "addOrReplaceToast"); - mockClient.isVersionSupported.mockImplementation(async (version) => version == "r0.6.0"); - initLocalStorageMock({ ...localStorageSession }); + it("should proceed if server is not accessible", async () => { + mockClient.isVersionSupported.mockRejectedValue(new Error("Oh, noes, the server is down!")); expect(await restoreFromLocalStorage()).toEqual(true); - expect(toastSpy).toHaveBeenCalledWith( - expect.objectContaining({ - title: "Your server is unsupported", - }), - ); }); }); }); diff --git a/test/stores/LifecycleStore-test.ts b/test/stores/LifecycleStore-test.ts new file mode 100644 index 0000000000..e9801980d9 --- /dev/null +++ b/test/stores/LifecycleStore-test.ts @@ -0,0 +1,86 @@ +/* +Copyright 2024 The Matrix.org Foundation C.I.C. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +import { mocked } from "jest-mock"; +import { SyncState } from "matrix-js-sdk/src/matrix"; + +import { MatrixClientPeg } from "../../src/MatrixClientPeg"; +import ToastStore from "../../src/stores/ToastStore"; +import { stubClient } from "../test-utils"; +import LifecycleStore from "../../src/stores/LifecycleStore"; + +describe("LifecycleStore", () => { + stubClient(); + const client = MatrixClientPeg.safeGet(); + let addOrReplaceToast: jest.SpyInstance; + + beforeEach(() => { + addOrReplaceToast = jest.spyOn(ToastStore.sharedInstance(), "addOrReplaceToast"); + }); + + it("should do nothing if the matrix server version is supported", async () => { + mocked(client).isVersionSupported.mockResolvedValue(true); + + (LifecycleStore as any).onDispatch({ + action: "MatrixActions.sync", + state: SyncState.Syncing, + prevState: SyncState.Prepared, + }); + + await new Promise(setImmediate); + + expect(addOrReplaceToast).not.toHaveBeenCalledWith( + expect.objectContaining({ + title: "Your server is unsupported", + }), + ); + }); + + it("should show a toast if the matrix server version is unsupported", async () => { + mocked(client).isVersionSupported.mockResolvedValue(false); + + (LifecycleStore as any).onDispatch({ + action: "MatrixActions.sync", + state: SyncState.Syncing, + prevState: SyncState.Prepared, + }); + + await new Promise(setImmediate); + + expect(addOrReplaceToast).toHaveBeenCalledWith( + expect.objectContaining({ + title: "Your server is unsupported", + }), + ); + }); + + it("dismisses toast on accept button", async () => { + const dismissToast = jest.spyOn(ToastStore.sharedInstance(), "dismissToast"); + mocked(client).isVersionSupported.mockResolvedValue(false); + + (LifecycleStore as any).onDispatch({ + action: "MatrixActions.sync", + state: SyncState.Syncing, + prevState: SyncState.Prepared, + }); + + await new Promise(setImmediate); + + addOrReplaceToast.mock.calls[0][0].props.onAccept(); + + expect(dismissToast).toHaveBeenCalledWith(addOrReplaceToast.mock.calls[0][0].key); + }); +});