Feed events to widgets as they are decrypted (even if out of order)

The code that feeds events to widgets tries to enforce that only events from the end of the timeline will be passed through. This is to prevent old, irrelevant events from being passed to widgets as the timeline is back-filled. However, since encrypted events need to be decrypted asynchronously, it's not possible to feed them to a widget in a strictly linear order without introducing some kind of blocking or unreliable delivery. This code has been dropping events when they're decrypted out of order, which we consider to be an undesirable behavior.

The solution provided here is that, to reflect the asynchronous nature of decryption, encrypted events that arrive at the end of the timeline will be fed to a widget whenever they finish decrypting, even if this means feeding them out of order. For now we're not aware of any widgets that care about knowing the exact order of events in the timeline, but if such a need reveals itself later, we can explore adding ordering information to this part of the widget API.

Author: robintown

src/stores/widgets/StopGapWidget.ts

@@ -154,7 +154,10 @@ export class StopGapWidget extends EventEmitter {
     private kind: WidgetKind;
     private readonly virtual: boolean;
     private readUpToMap: { [roomId: string]: string } = {}; // room ID to event ID
-    private stickyPromise?: () => Promise<void>; // This promise will be called and needs to resolve before the widget will actually become sticky.
+    // This promise will be called and needs to resolve before the widget will actually become sticky.
+    private stickyPromise?: () => Promise<void>;
+    // Holds events that should be fed to the widget once they finish decrypting
+    private readonly eventsToFeed = new WeakSet<MatrixEvent>();
 
     public constructor(private appTileProps: IAppTileProps) {
         super();
@@ -465,12 +468,10 @@ export class StopGapWidget extends EventEmitter {
 
     private onEvent = (ev: MatrixEvent): void => {
         this.client.decryptEventIfNeeded(ev);
-        if (ev.isBeingDecrypted() || ev.isDecryptionFailure()) return;
         this.feedEvent(ev);
     };
 
     private onEventDecrypted = (ev: MatrixEvent): void => {
-        if (ev.isDecryptionFailure()) return;
         this.feedEvent(ev);
     };
 
@@ -546,6 +547,9 @@ export class StopGapWidget extends EventEmitter {
     private feedEvent(ev: MatrixEvent): void {
         if (this.messaging === null) return;
         if (
+            // If we had decided earlier to feed this event to the widget, but
+            // it just wasn't ready, give it another try
+            this.eventsToFeed.delete(ev) ||
             // Skip marker timeline check for events with relations to unknown parent because these
             // events are not added to the timeline here and will be ignored otherwise:
             // https://github.com/matrix-org/matrix-js-sdk/blob/d3dfcd924201d71b434af3d77343b5229b6ed75e/src/models/room.ts#L2207-L2213
@@ -563,10 +567,16 @@ export class StopGapWidget extends EventEmitter {
             // receiving ancient events from backfill and such.
             this.advanceReadUpToMarker(ev)
         ) {
-            const raw = ev.getEffectiveEvent();
-            this.messaging.feedEvent(raw as IRoomEvent, this.eventListenerRoomId!).catch((e) => {
-                logger.error("Error sending event to widget: ", e);
-            });
+            // If the event is still being decrypted, remember that we want to
+            // feed it to the widget (even if not strictly in the order given by
+            // the timeline) and get back to it later
+            if (ev.isBeingDecrypted() || ev.isDecryptionFailure()) this.eventsToFeed.add(ev);
+            else {
+                const raw = ev.getEffectiveEvent();
+                this.messaging.feedEvent(raw as IRoomEvent, this.eventListenerRoomId!).catch((e) => {
+                    logger.error("Error sending event to widget: ", e);
+                });
+            }
         }
     }
 }

test/unit-tests/stores/widgets/StopGapWidget-test.ts

@@ -8,7 +8,14 @@ Please see LICENSE files in the repository root for full details.
 
 import { mocked, MockedObject } from "jest-mock";
 import { last } from "lodash";
-import { MatrixEvent, MatrixClient, ClientEvent, EventTimeline } from "matrix-js-sdk/src/matrix";
+import {
+    MatrixEvent,
+    MatrixClient,
+    ClientEvent,
+    EventTimeline,
+    EventType,
+    MatrixEventEvent,
+} from "matrix-js-sdk/src/matrix";
 import { ClientWidgetApi, WidgetApiFromWidgetAction } from "matrix-widget-api";
 import { waitFor } from "jest-matrix-react";
 
@@ -134,6 +141,46 @@ describe("StopGapWidget", () => {
             expect(messaging.feedEvent).toHaveBeenLastCalledWith(event2.getEffectiveEvent(), "!1:example.org");
         });
 
+        it("feeds decrypted events asynchronously", async () => {
+            const event1Encrypted = new MatrixEvent({
+                event_id: event1.getId(),
+                type: EventType.RoomMessageEncrypted,
+                sender: event1.sender?.userId,
+                room_id: event1.getRoomId(),
+                content: {},
+            });
+            const decryptingSpy1 = jest.spyOn(event1Encrypted, "isBeingDecrypted").mockReturnValue(true);
+            client.emit(ClientEvent.Event, event1Encrypted);
+            const event2Encrypted = new MatrixEvent({
+                event_id: event2.getId(),
+                type: EventType.RoomMessageEncrypted,
+                sender: event2.sender?.userId,
+                room_id: event2.getRoomId(),
+                content: {},
+            });
+            const decryptingSpy2 = jest.spyOn(event2Encrypted, "isBeingDecrypted").mockReturnValue(true);
+            client.emit(ClientEvent.Event, event2Encrypted);
+            expect(messaging.feedEvent).not.toHaveBeenCalled();
+
+            // "Decrypt" the events, but in reverse order; first event 2…
+            event2Encrypted.event.type = event2.getType();
+            event2Encrypted.event.content = event2.getContent();
+            decryptingSpy2.mockReturnValue(false);
+            client.emit(MatrixEventEvent.Decrypted, event2Encrypted);
+            expect(messaging.feedEvent).toHaveBeenCalledTimes(1);
+            expect(messaging.feedEvent).toHaveBeenLastCalledWith(event2Encrypted.getEffectiveEvent(), "!1:example.org");
+            // …then event 1
+            event1Encrypted.event.type = event1.getType();
+            event1Encrypted.event.content = event1.getContent();
+            decryptingSpy1.mockReturnValue(false);
+            client.emit(MatrixEventEvent.Decrypted, event1Encrypted);
+            // The events should be fed in that same order so that event 2
+            // doesn't have to be blocked on the decryption of event 1 (or
+            // worse, dropped)
+            expect(messaging.feedEvent).toHaveBeenCalledTimes(2);
+            expect(messaging.feedEvent).toHaveBeenLastCalledWith(event1Encrypted.getEffectiveEvent(), "!1:example.org");
+        });
+
         it("should not feed incoming event if not in timeline", () => {
             const event = mkEvent({
                 event: true,