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

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

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

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<MatrixClient>;
@@ -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(SettingsStore, "getFeatureSettingNames").mockReturnValue(someFeatures);
-            jest.spyOn(mockSettingsStore, "getValue").mockImplementation((settingName): any => {
+            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", () => {