From 6cc42b7e2e033c91de570339e9bb24fa1681c357 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 30 Aug 2023 10:35:15 +0100 Subject: [PATCH] Improve error handling for `DeviceListener` (#11484) ... particularly for when it races with a client shutdown. --- src/DeviceListener.ts | 22 ++++++++++++++++++++-- test/DeviceListener-test.ts | 24 +++++++++++++++++++++++- 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/DeviceListener.ts b/src/DeviceListener.ts index 8e8197736b..eb0772154d 100644 --- a/src/DeviceListener.ts +++ b/src/DeviceListener.ts @@ -14,7 +14,15 @@ See the License for the specific language governing permissions and limitations under the License. */ -import { MatrixEvent, ClientEvent, EventType, MatrixClient, RoomStateEvent, SyncState } from "matrix-js-sdk/src/matrix"; +import { + MatrixEvent, + ClientEvent, + EventType, + MatrixClient, + RoomStateEvent, + SyncState, + ClientStoppedError, +} from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; import { IKeyBackupInfo } from "matrix-js-sdk/src/crypto/keybackup"; @@ -260,7 +268,17 @@ export default class DeviceListener { return cli?.getRooms().some((r) => cli.isRoomEncrypted(r.roomId)) ?? false; } - private async recheck(): Promise { + private recheck(): void { + this.doRecheck().catch((e) => { + if (e instanceof ClientStoppedError) { + // the client was stopped while recheck() was running. Nothing left to do. + } else { + logger.error("Error during `DeviceListener.recheck`", e); + } + }); + } + + private async doRecheck(): Promise { if (!this.running || !this.client) return; // we have been stopped const cli = this.client; diff --git a/test/DeviceListener-test.ts b/test/DeviceListener-test.ts index 54fe53594f..0c399b0d26 100644 --- a/test/DeviceListener-test.ts +++ b/test/DeviceListener-test.ts @@ -15,7 +15,15 @@ limitations under the License. */ import { Mocked, mocked } from "jest-mock"; -import { MatrixEvent, Room, MatrixClient, DeviceVerificationStatus, CryptoApi, Device } from "matrix-js-sdk/src/matrix"; +import { + MatrixEvent, + Room, + MatrixClient, + DeviceVerificationStatus, + CryptoApi, + Device, + ClientStoppedError, +} from "matrix-js-sdk/src/matrix"; import { logger } from "matrix-js-sdk/src/logger"; import { CrossSigningInfo } from "matrix-js-sdk/src/crypto/CrossSigning"; import { CryptoEvent } from "matrix-js-sdk/src/crypto"; @@ -257,6 +265,20 @@ describe("DeviceListener", () => { expect(mockCrypto!.isCrossSigningReady).not.toHaveBeenCalled(); }); + it("correctly handles the client being stopped", async () => { + mockCrypto!.isCrossSigningReady.mockImplementation(() => { + throw new ClientStoppedError(); + }); + await createAndStart(); + expect(logger.error).not.toHaveBeenCalled(); + }); + it("correctly handles other errors", async () => { + mockCrypto!.isCrossSigningReady.mockImplementation(() => { + throw new Error("blah"); + }); + await createAndStart(); + expect(logger.error).toHaveBeenCalledTimes(1); + }); describe("set up encryption", () => { const rooms = [{ roomId: "!room1" }, { roomId: "!room2" }] as unknown as Room[];