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

@@ -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<void> {
-    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<boolean> {
     logger.error("Unable to load session", e);
 

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<IState> {
                 });
                 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<IState> {
     }
 }
 
+async function checkServerVersions(): Promise<void> {
+    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();

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 () => {
+            it("should proceed if server is not accessible", async () => {
-                const toastSpy = jest.spyOn(ToastStore.sharedInstance(), "addOrReplaceToast");
+                mockClient.isVersionSupported.mockRejectedValue(new Error("Oh, noes, the server is down!"));
-                mockClient.isVersionSupported.mockImplementation(async (version) => version == "r0.6.0");
-                initLocalStorageMock({ ...localStorageSession });
 
                 expect(await restoreFromLocalStorage()).toEqual(true);
-                expect(toastSpy).toHaveBeenCalledWith(
-                    expect.objectContaining({
-                        title: "Your server is unsupported",
-                    }),
-                );
             });
         });
     });

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);
+    });
+});