Skip to content

Commit

Permalink
Don't swallow up errors coming from the shareSession call
Browse files Browse the repository at this point in the history
A call to ensureSession() has two steps:
    1. prepareSession(), where an outbound group session might get created
       or rotated
    2. shareSession(), where an outbound group session might get
       encrypted and queued up to be sent to other devices

Both of those calls may mostly fail due to storage errors, yet only the
errors from prepareSession get propagated to the caller.

Errors from prepareSession will mean that you can't get an
outbound group session so you can't encrypt an event.

Errors from shareSession, especially if the error happens in the part
where the to-device requests are queued up to be sent out, mean that
other people will not be able to decrypt the events that will get
encrypted using the outbound group session.

Both of those cases are catastrophic, the second case is just much
harder to debug, since the error happens on another device at some
arbitrary point in the future.

Let's just return the error instead, people can then retry and the
storage issue might have been resolved, or at least the error becomes
visible when it happens.
  • Loading branch information
poljar committed Nov 24, 2022
1 parent 7ba71f7 commit 093901a
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions src/crypto/algorithms/megolm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,8 +292,6 @@ 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);
Expand All @@ -302,11 +300,11 @@ class MegolmEncryption extends EncryptionAlgorithm {

try {
await this.shareSession(devicesInRoom, sharedHistory, singleOlmCreationPhase, blocked, session);
return session;
} catch (e) {
logger.error(`Failed to ensure outbound session in ${this.roomId}`, e);
throw e;
}

return session;
};

// first wait for the previous share to complete
Expand Down

0 comments on commit 093901a

Please sign in to comment.