From d733ac014cc6c9e04ec01a96822d3d84e475da9e Mon Sep 17 00:00:00 2001 From: Michael Telatynski <7t3chguy@gmail.com> Date: Thu, 22 Jan 2026 17:04:01 +0000 Subject: [PATCH] Fix ability to send rageshake during session restore failure (#31848) * Fix ability to send rageshake during session restore failure This fixes the specific edge case but also hardens the codepath to limit the effect of other similar edges popping up Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Improve coverage Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> * Improve coverage Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --------- Signed-off-by: Michael Telatynski <7t3chguy@gmail.com> --- src/settings/SettingsStore.ts | 7 +++- .../controllers/NotificationControllers.ts | 2 +- test/unit-tests/submit-rageshake-test.ts | 36 ++++++++++++++++--- 3 files changed, 38 insertions(+), 7 deletions(-) diff --git a/src/settings/SettingsStore.ts b/src/settings/SettingsStore.ts index c6a739f1c8..0852583fc1 100644 --- a/src/settings/SettingsStore.ts +++ b/src/settings/SettingsStore.ts @@ -893,7 +893,12 @@ export default class SettingsStore { for (const settingKey of (Object.keys(SETTINGS) as SettingKey[]).filter( (s) => SETTINGS[s].shouldExportToRageshake !== false, )) { - settingMap[settingKey] = SettingsStore.getValue(settingKey); + try { + settingMap[settingKey] = SettingsStore.getValue(settingKey); + } catch (e) { + logger.warn("Failed to read setting", settingKey, e); + settingMap[settingKey] = "Failed to read setting!"; + } } return JSON.stringify(settingMap); } diff --git a/src/settings/controllers/NotificationControllers.ts b/src/settings/controllers/NotificationControllers.ts index 8ec17977b5..08c5cc3c2c 100644 --- a/src/settings/controllers/NotificationControllers.ts +++ b/src/settings/controllers/NotificationControllers.ts @@ -18,7 +18,7 @@ import { type SettingLevel } from "../SettingLevel"; // default action on this rule is dont_notify, but it could be something else export function isPushNotifyDisabled(): boolean { // Return the value of the master push rule as a default - const masterRule = MatrixClientPeg.safeGet().pushProcessor.getPushRuleById(".m.rule.master"); + const masterRule = MatrixClientPeg.get()?.pushProcessor.getPushRuleById(".m.rule.master"); if (!masterRule) { logger.warn("No master push rule! Notifications are disabled for this user."); diff --git a/test/unit-tests/submit-rageshake-test.ts b/test/unit-tests/submit-rageshake-test.ts index e3f623ef6c..0bb2006754 100644 --- a/test/unit-tests/submit-rageshake-test.ts +++ b/test/unit-tests/submit-rageshake-test.ts @@ -25,6 +25,8 @@ import { type FeatureSettingKey, type SettingKey } from "../../src/settings/Sett import { SettingLevel } from "../../src/settings/SettingLevel.ts"; import SdkConfig from "../../src/SdkConfig.ts"; import { BugReportEndpointURLLocal } from "../../src/IConfigOptions.ts"; +import { Notifier } from "../../src/Notifier.ts"; +import { MatrixClientPeg } from "../../src/MatrixClientPeg.ts"; describe("Rageshakes", () => { let mockClient: Mocked; @@ -356,7 +358,9 @@ describe("Rageshakes", () => { }); describe("Settings Store", () => { - const mockSettingsStore = mocked(SettingsStore); + beforeEach(() => { + jest.spyOn(Notifier, "isPossible").mockReturnValue(true); + }); afterEach(() => { jest.restoreAllMocks(); @@ -368,8 +372,8 @@ describe("Rageshakes", () => { "feature_notification_settings2", ] as unknown[] as FeatureSettingKey[]; const enabledFeatures: SettingKey[] = ["feature_video_rooms"]; - jest.spyOn(mockSettingsStore, "getFeatureSettingNames").mockReturnValue(someFeatures); - jest.spyOn(mockSettingsStore, "getValue").mockImplementation((settingName): any => { + jest.spyOn(SettingsStore, "getFeatureSettingNames").mockReturnValue(someFeatures); + jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName): any => { return enabledFeatures.includes(settingName); }); @@ -378,7 +382,7 @@ describe("Rageshakes", () => { }); it("should collect low bandWidth enabled", async () => { - jest.spyOn(mockSettingsStore, "getValue").mockImplementation((settingName): any => { + jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName): any => { if (settingName == "lowBandwidth") { return true; } @@ -388,7 +392,7 @@ describe("Rageshakes", () => { expect(formData.get("lowBandwidth")).toBe("enabled"); }); it("should collect low bandWidth disabled", async () => { - jest.spyOn(mockSettingsStore, "getValue").mockImplementation((settingName): any => { + jest.spyOn(SettingsStore, "getValue").mockImplementation((settingName): any => { if (settingName == "lowBandwidth") { return false; } @@ -397,6 +401,28 @@ describe("Rageshakes", () => { const formData = await collectBugReport(); expect(formData.get("lowBandwidth")).toBeNull(); }); + + it("should handle settings throwing when logged out", async () => { + jest.mocked(MatrixClientPeg.get).mockRestore(); + jest.mocked(MatrixClientPeg.safeGet).mockRestore(); + jest.spyOn(Notifier, "isPossible").mockImplementation(() => { + throw new Error("Test"); + }); + + const formData = await collectBugReport(); + expect(JSON.parse(formData.get("mx_local_settings") as string)["notificationsEnabled"]).toBe( + "Failed to read setting!", + ); + }); + + it("should handle reading notification settings when logged out", async () => { + jest.mocked(MatrixClientPeg.get).mockRestore(); + jest.mocked(MatrixClientPeg.safeGet).mockRestore(); + jest.spyOn(Notifier, "isPossible").mockReturnValue(true); + + const formData = await collectBugReport(); + expect(JSON.parse(formData.get("mx_local_settings") as string)["notificationsEnabled"]).toBe(false); + }); }); describe("Navigator Storage", () => {