diff --git a/src/stores/widgets/StopGapWidget.ts b/src/stores/widgets/StopGapWidget.ts index e5c46e6a6af..8362f1048a0 100644 --- a/src/stores/widgets/StopGapWidget.ts +++ b/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; // 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; + // Holds events that should be fed to the widget once they finish decrypting + private readonly eventsToFeed = new WeakSet(); 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); }; @@ -480,72 +481,103 @@ export class StopGapWidget extends EventEmitter { await this.messaging?.feedToDevice(ev.getEffectiveEvent() as IRoomEvent, ev.isEncrypted()); }; - private feedEvent(ev: MatrixEvent): void { - if (!this.messaging) return; - - // Check to see if this event would be before or after our "read up to" marker. If it's - // before, or we can't decide, then we assume the widget will have already seen the event. - // If the event is after, or we don't have a marker for the room, then we'll send it through. - // - // This approach of "read up to" prevents widgets receiving decryption spam from startup or - // receiving out-of-order events from backfill and such. - // - // 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 - let isRelationToUnknown: boolean | undefined = undefined; - const upToEventId = this.readUpToMap[ev.getRoomId()!]; - if (upToEventId) { - // Small optimization for exact match (prevent search) - if (upToEventId === ev.getId()) { - return; - } + /** + * Determines whether the event has a relation to an unknown parent. + */ + private relatesToUnknown(ev: MatrixEvent): boolean { + // Replies to unknown events don't count + if (!ev.relationEventId || ev.replyEventId) return false; + const room = this.client.getRoom(ev.getRoomId()); + return room === null || !room.findEventById(ev.relationEventId); + } - // should be true to forward the event to the widget - let shouldForward = false; - - const room = this.client.getRoom(ev.getRoomId()!); - if (!room) return; - // Timelines are most recent last, so reverse the order and limit ourselves to 100 events - // to avoid overusing the CPU. - const timeline = room.getLiveTimeline(); - const events = arrayFastClone(timeline.getEvents()).reverse().slice(0, 100); - - for (const timelineEvent of events) { - if (timelineEvent.getId() === upToEventId) { - break; - } else if (timelineEvent.getId() === ev.getId()) { - shouldForward = true; - break; - } - } + /** + * Determines whether the event comes from a room that we've been invited to + * (in which case we likely don't have the full timeline). + */ + private isFromInvite(ev: MatrixEvent): boolean { + const room = this.client.getRoom(ev.getRoomId()); + return room?.getMyMembership() === KnownMembership.Invite; + } - if (!shouldForward) { - // checks that the event has a relation to unknown event - isRelationToUnknown = - !ev.replyEventId && !!ev.relationEventId && !room.findEventById(ev.relationEventId); - if (!isRelationToUnknown) { - // Ignore the event: it is before our interest. - return; - } - } + /** + * Advances the "read up to" marker for a room to a certain event. No-ops if + * the event is before the marker. + * @returns Whether the "read up to" marker was advanced. + */ + private advanceReadUpToMarker(ev: MatrixEvent): boolean { + const evId = ev.getId(); + if (evId === undefined) return false; + const roomId = ev.getRoomId(); + if (roomId === undefined) return false; + const room = this.client.getRoom(roomId); + if (room === null) return false; + + const upToEventId = this.readUpToMap[ev.getRoomId()!]; + if (!upToEventId) { + // There's no marker yet; start it at this event + this.readUpToMap[roomId] = evId; + return true; } - // Skip marker assignment if membership is 'invite', otherwise 'm.room.member' from - // invitation room will assign it and new state events will be not forwarded to the widget - // because of empty timeline for invitation room and assigned marker. - const evRoomId = ev.getRoomId(); - const evId = ev.getId(); - if (evRoomId && evId) { - const room = this.client.getRoom(evRoomId); - if (room && room.getMyMembership() === KnownMembership.Join && !isRelationToUnknown) { - this.readUpToMap[evRoomId] = evId; + // Small optimization for exact match (skip the search) + if (upToEventId === evId) return false; + + // Timelines are most recent last, so reverse the order and limit ourselves to 100 events + // to avoid overusing the CPU. + const timeline = room.getLiveTimeline(); + const events = arrayFastClone(timeline.getEvents()).reverse().slice(0, 100); + + for (const timelineEvent of events) { + if (timelineEvent.getId() === upToEventId) { + // The event must be somewhere before the "read up to" marker + return false; + } else if (timelineEvent.getId() === ev.getId()) { + // The event is after the marker; advance it + this.readUpToMap[roomId] = evId; + return true; } } - const raw = ev.getEffectiveEvent(); - this.messaging.feedEvent(raw as IRoomEvent, this.eventListenerRoomId!).catch((e) => { - logger.error("Error sending event to widget: ", e); - }); + // We can't say for sure whether the widget has seen the event; let's + // just assume that it has + return false; + } + + 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 + this.relatesToUnknown(ev) || + // Skip marker timeline check for rooms where membership is + // 'invite', otherwise the membership event from the invitation room + // will advance the marker and new state events will not be + // forwarded to the widget. + this.isFromInvite(ev) || + // Check whether this event would be before or after our "read up to" marker. If it's + // before, or we can't decide, then we assume the widget will have already seen the event. + // If the event is after, or we don't have a marker for the room, then the marker will advance and we'll + // send it through. + // This approach of "read up to" prevents widgets receiving decryption spam from startup or + // receiving ancient events from backfill and such. + this.advanceReadUpToMarker(ev) + ) { + // 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); + }); + } + } } } diff --git a/test/unit-tests/stores/widgets/StopGapWidget-test.ts b/test/unit-tests/stores/widgets/StopGapWidget-test.ts index 84cc84ee83a..397c289d224 100644 --- a/test/unit-tests/stores/widgets/StopGapWidget-test.ts +++ b/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,