Skip to content

Commit

Permalink
Log when we show, and hide, encryption setup toasts (#29235)
Browse files Browse the repository at this point in the history
It's currently hard to debug when someone sees or hides one of these
toasts. Lets's add some logging.
  • Loading branch information
richvdh authored Feb 11, 2025
1 parent ef69c0d commit f2fae82
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 10 deletions.
10 changes: 8 additions & 2 deletions src/DeviceListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {
type SyncState,
ClientStoppedError,
} from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import { logger as baseLogger } from "matrix-js-sdk/src/logger";
import { CryptoEvent, type KeyBackupInfo } from "matrix-js-sdk/src/crypto-api";
import { type CryptoSessionStateChange } from "@matrix-org/analytics-events/types/typescript/CryptoSessionStateChange";

Expand Down Expand Up @@ -48,6 +48,8 @@ import { asyncSomeParallel } from "./utils/arrays.ts";

const KEY_BACKUP_POLL_INTERVAL = 5 * 60 * 1000;

const logger = baseLogger.getChild("DeviceListener:");

export default class DeviceListener {
private dispatcherRef?: string;
// device IDs for which the user has dismissed the verify toast ('Later')
Expand Down Expand Up @@ -131,7 +133,7 @@ export default class DeviceListener {
* @param {String[]} deviceIds List of device IDs to dismiss notifications for
*/
public async dismissUnverifiedSessions(deviceIds: Iterable<string>): Promise<void> {
logger.log("Dismissing unverified sessions: " + Array.from(deviceIds).join(","));
logger.debug("Dismissing unverified sessions: " + Array.from(deviceIds).join(","));
for (const d of deviceIds) {
this.dismissed.add(d);
}
Expand Down Expand Up @@ -309,16 +311,20 @@ export default class DeviceListener {
if (!crossSigningReady) {
// This account is legacy and doesn't have cross-signing set up at all.
// Prompt the user to set it up.
logger.info("Cross-signing not ready: showing SET_UP_ENCRYPTION toast");
showSetupEncryptionToast(SetupKind.SET_UP_ENCRYPTION);
} else if (!isCurrentDeviceTrusted) {
// cross signing is ready but the current device is not trusted: prompt the user to verify
logger.info("Current device not verified: showing VERIFY_THIS_SESSION toast");
showSetupEncryptionToast(SetupKind.VERIFY_THIS_SESSION);
} else if (!allCrossSigningSecretsCached) {
// cross signing ready & device trusted, but we are missing secrets from our local cache.
// prompt the user to enter their recovery key.
logger.info("Some secrets not cached: showing KEY_STORAGE_OUT_OF_SYNC toast");
showSetupEncryptionToast(SetupKind.KEY_STORAGE_OUT_OF_SYNC);
} else if (defaultKeyId === null) {
// the user just hasn't set up 4S yet: prompt them to do so
logger.info("No default 4S key: showing SET_UP_RECOVERY toast");
showSetupEncryptionToast(SetupKind.SET_UP_RECOVERY);
} else {
// some other condition... yikes! Show the 'set up encryption' toast: this is what we previously did
Expand Down
4 changes: 4 additions & 0 deletions src/stores/ToastStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Please see LICENSE files in the repository root for full details.
*/

import EventEmitter from "events";
import { logger } from "matrix-js-sdk/src/logger";

import type React from "react";
import { type ComponentClass } from "../@types/common";
Expand Down Expand Up @@ -54,10 +55,12 @@ export default class ToastStore extends EventEmitter {
public addOrReplaceToast<C extends ComponentClass>(newToast: IToast<C>): void {
const oldIndex = this.toasts.findIndex((t) => t.key === newToast.key);
if (oldIndex === -1) {
logger.info(`Opening toast with key '${newToast.key}': title '${newToast.title}'`);
let newIndex = this.toasts.length;
while (newIndex > 0 && this.toasts[newIndex - 1].priority < newToast.priority) --newIndex;
this.toasts.splice(newIndex, 0, newToast);
} else {
logger.info(`Replacing existing toast with key '${newToast.key}': title now '${newToast.title}'`);
this.toasts[oldIndex] = newToast;
}
this.emit("update");
Expand All @@ -71,6 +74,7 @@ export default class ToastStore extends EventEmitter {
const length = this.toasts.length;
this.toasts = this.toasts.filter((t) => t.key !== key);
if (length !== this.toasts.length) {
logger.info(`Removed toast with key '${key}'`);
if (this.toasts.length === 0) {
this.countSeen = 0;
}
Expand Down
21 changes: 13 additions & 8 deletions test/unit-tests/DeviceListener-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ Please see LICENSE files in the repository root for full details.

import { type Mocked, mocked } from "jest-mock";
import { MatrixEvent, type Room, type MatrixClient, Device, ClientStoppedError } from "matrix-js-sdk/src/matrix";
import { logger } from "matrix-js-sdk/src/logger";
import {
CryptoEvent,
type CrossSigningStatus,
Expand All @@ -33,9 +32,6 @@ import { UIFeature } from "../../src/settings/UIFeature";
import { isBulkUnverifiedDeviceReminderSnoozed } from "../../src/utils/device/snoozeBulkUnverifiedDeviceReminder";
import { PosthogAnalytics } from "../../src/PosthogAnalytics";

// don't litter test console with logs
jest.mock("matrix-js-sdk/src/logger");

jest.mock("../../src/dispatcher/dispatcher", () => ({
dispatch: jest.fn(),
register: jest.fn(),
Expand Down Expand Up @@ -63,6 +59,12 @@ describe("DeviceListener", () => {
beforeEach(() => {
jest.resetAllMocks();

// don't litter the console with logs
jest.spyOn(console, "debug").mockImplementation(() => {});
jest.spyOn(console, "info").mockImplementation(() => {});
jest.spyOn(console, "warn").mockImplementation(() => {});
jest.spyOn(console, "error").mockImplementation(() => {});

// spy on various toasts' hide and show functions
// easier than mocking
jest.spyOn(SetupEncryptionToast, "showToast").mockReturnValue(undefined);
Expand Down Expand Up @@ -158,14 +160,17 @@ describe("DeviceListener", () => {
});

it("catches error and logs when saving client information fails", async () => {
const errorLogSpy = jest.spyOn(logger, "error");
const error = new Error("oups");
mockClient!.setAccountData.mockRejectedValue(error);

// doesn't throw
await createAndStart();

expect(errorLogSpy).toHaveBeenCalledWith("Failed to update client information", error);
expect(console.error).toHaveBeenCalledWith(
"DeviceListener:",
"Failed to update client information",
error,
);
});

it("saves client information on logged in action", async () => {
Expand Down Expand Up @@ -275,14 +280,14 @@ describe("DeviceListener", () => {
throw new ClientStoppedError();
});
await createAndStart();
expect(logger.error).not.toHaveBeenCalled();
expect(console.error).not.toHaveBeenCalled();
});
it("correctly handles other errors", async () => {
mockCrypto!.isCrossSigningReady.mockImplementation(() => {
throw new Error("blah");
});
await createAndStart();
expect(logger.error).toHaveBeenCalledTimes(1);
expect(console.error).toHaveBeenCalledTimes(1);
});

describe("set up encryption", () => {
Expand Down

0 comments on commit f2fae82

Please sign in to comment.