Don't swallow errors coming from the shareSession call

spec/unit/crypto/algorithms/megolm.spec.ts

@@ -490,6 +490,26 @@ describe("MegolmDecryption", function () {
 
                 expect(mockBaseApis.queueToDevice).not.toHaveBeenCalled();
             });
+
+            it("sholdn't wedge the setup promise if sharing a room key fails", async () => {
+                // @ts-ignore - private field access
+                const initialSetupPromise = await megolmEncryption.setupPromise;
+                expect(initialSetupPromise).toBe(null);
+
+                // @ts-ignore - private field access
+                megolmEncryption.shareSession = () => {
+                    throw new Error("Can't share session");
+                };
+
+                await expect(() =>
+                    // @ts-ignore - private field access
+                    megolmEncryption.ensureOutboundSession(mockRoom, {}, {}, true),
+                ).rejects.toThrow();
+
+                // @ts-ignore - private field access
+                const finalSetupPromise = await megolmEncryption.setupPromise;
+                expect(finalSetupPromise).toBe(null);
+            });
         });
     });
 

src/client.ts

@@ -9302,13 +9302,13 @@ export function fixNotificationCountOnDecryption(cli: MatrixClient, event: Matri
             const thread = room.getThread(event.threadRootId);
             hasReadEvent = thread
                 ? thread.hasUserReadEvent(cli.getUserId()!, event.getId()!)
-                // If the thread object does not exist in the room yet, we don't
-                // want to calculate notification for this event yet. We have not
-                // restored the read receipts yet and can't accurately calculate
-                // highlight notifications at this stage.
-                //
-                // This issue can likely go away when MSC3874 is implemented
-                : true;
+                : // If the thread object does not exist in the room yet, we don't
+                  // want to calculate notification for this event yet. We have not
+                  // restored the read receipts yet and can't accurately calculate
+                  // highlight notifications at this stage.
+                  //
+                  // This issue can likely go away when MSC3874 is implemented
+                  true;
         } else {
             hasReadEvent = room.hasUserReadEvent(cli.getUserId()!, event.getId()!);
         }

src/crypto/algorithms/megolm.ts

@@ -248,6 +248,23 @@ export class MegolmEncryption extends EncryptionAlgorithm {
      * @param singleOlmCreationPhase - Only perform one round of olm
      *     session creation
      *
+     * This method updates the setupPromise field of the class by chaining a new
+     * call on top of the existing promise, and then catching and discarding any
+     * errors that might happen while setting up the outbound group session. This
+     * is done to ensure that `setupPromise` always resolves to `null` or the
+     * `OutboundSessionInfo`.
+     *
+     * Using `>>=` to represent the promise chaining operation, it does the
+     * following:
+     *
+     * ```
+     * setupPromise = previousSetupPromise >>= setup >>= discardErrors
+     * ```
+     *
+     * The initial value for the `setupPromise` is a promise that resolves to
+     * `null`. The forceDiscardSession() resets setupPromise to this initial
+     * promise.
+     *
      * @returns Promise which resolves to the
      *    OutboundSessionInfo when setup is complete.
      */
@@ -260,36 +277,31 @@ export class MegolmEncryption extends EncryptionAlgorithm {
         // takes the previous OutboundSessionInfo, and considers whether to create
         // a new one. Also shares the key with any (new) devices in the room.
         //
-        // Returns the successful session whether keyshare succeeds or not.
-        //
         // returns a promise which resolves once the keyshare is successful.
         const setup = async (oldSession: OutboundSessionInfo | null): Promise<OutboundSessionInfo> => {
             const sharedHistory = isRoomSharedHistory(room);
-
             const session = await this.prepareSession(devicesInRoom, sharedHistory, oldSession);
 
-            try {
-                await this.shareSession(devicesInRoom, sharedHistory, singleOlmCreationPhase, blocked, session);
-            } catch (e) {
-                logger.error(`Failed to ensure outbound session in ${this.roomId}`, e);
-            }
+            await this.shareSession(devicesInRoom, sharedHistory, singleOlmCreationPhase, blocked, session);
 
             return session;
         };
 
         // first wait for the previous share to complete
-        const prom = this.setupPromise.then(setup);
+        const fallible = this.setupPromise.then(setup);
 
-        // Ensure any failures are logged for debugging
-        prom.catch((e) => {
+        // Ensure any failures are logged for debugging and make sure that the
+        // promise chain remains unbroken
+        //
+        // setupPromise resolves to `null` or the `OutboundSessionInfo` whether
+        // or not the share succeeds
+        this.setupPromise = fallible.catch((e) => {
             logger.error(`Failed to setup outbound session in ${this.roomId}`, e);
+            return null;
         });
 
-        // setupPromise resolves to `session` whether or not the share succeeds
-        this.setupPromise = prom;
-
         // but we return a promise which only resolves if the share was successful.
-        return prom;
+        return fallible;
     }
 
     private async prepareSession(