Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix spurious session corruption error #12280

Merged
merged 7 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 0 additions & 34 deletions src/Lifecycle.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -635,45 +633,13 @@ export async function restoreFromLocalStorage(opts?: { ignoreGuest?: boolean }):
},
false,
);
await checkServerVersions();
return true;
} else {
logger.log("No previous session found.");
return false;
}
}

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

Expand Down
47 changes: 47 additions & 0 deletions src/stores/LifecycleStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,19 @@ 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 { 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;
Expand Down Expand Up @@ -51,6 +59,11 @@ class LifecycleStore extends AsyncStore<IState> {
});
break;
case "MatrixActions.sync": {
if (payload.state === SyncState.Syncing && payload.prevState !== SyncState.Syncing) {
richvdh marked this conversation as resolved.
Show resolved Hide resolved
// We've reconnected to the server: update server version support
checkServerVersions();
richvdh marked this conversation as resolved.
Show resolved Hide resolved
}

if (payload.state !== "PREPARED") {
break;
}
Expand All @@ -70,6 +83,40 @@ class LifecycleStore extends AsyncStore<IState> {
}
}

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).
// 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)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's rather odd that we (sometimes) make another /versions request, rather than using the result of the /versions request that the client has just successfully done, which caused the transition to the Syncing state.

I appreciate that changing that would mostly mean changes in the js-sdk (in the implementation of isVersionSupported, I guess), so I'm not asking you to change this now as part of a quick fix to this issue, but maybe you could record the situation in a comment?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the js-sdk impl also laments the lack of cache expiration which seems roughly in the same category of problems. I can put a comment there?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually I think it does always re-use the result of the /versions request since it caches the response and of course, in this case, we know it's already done one so it never actually does the request. Noted in 8616cc2.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// 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,
});
}

let singletonLifecycleStore: LifecycleStore | null = null;
if (!singletonLifecycleStore) {
singletonLifecycleStore = new LifecycleStore();
Expand Down
12 changes: 2 additions & 10 deletions test/Lifecycle-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -451,17 +450,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",
}),
);
});
});
});
Expand Down
86 changes: 86 additions & 0 deletions test/stores/LifecycleStore-test.ts
Original file line number Diff line number Diff line change
@@ -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);
});
});
Loading