diff --git a/spec/unit/crypto/algorithms/megolm.spec.ts b/spec/unit/crypto/algorithms/megolm.spec.ts index 5b00466d8..2dca6765d 100644 --- a/spec/unit/crypto/algorithms/megolm.spec.ts +++ b/spec/unit/crypto/algorithms/megolm.spec.ts @@ -490,6 +490,26 @@ describe("MegolmDecryption", function () { expect(mockBaseApis.queueToDevice).not.toHaveBeenCalled(); }); + + it("shouldn'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.prepareSession = () => { + throw new Error("Can't prepare 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); + }); }); }); diff --git a/src/crypto/algorithms/megolm.ts b/src/crypto/algorithms/megolm.ts index aed0ab28f..e002ffa3d 100644 --- a/src/crypto/algorithms/megolm.ts +++ b/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. */ @@ -278,18 +295,20 @@ export class MegolmEncryption extends EncryptionAlgorithm { }; // 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(