Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retry processing potential poll events after decryption #3246

Merged
merged 5 commits into from
Apr 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 32 additions & 2 deletions spec/unit/models/poll.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,16 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { IEvent, MatrixEvent, PollEvent, Room } from "../../../src";
import { M_POLL_START } from "matrix-events-sdk";

import { EventType, IEvent, MatrixEvent, PollEvent, Room } from "../../../src";
import { REFERENCE_RELATION } from "../../../src/@types/extensible_events";
import { M_POLL_END, M_POLL_KIND_DISCLOSED, M_POLL_RESPONSE } from "../../../src/@types/polls";
import { PollStartEvent } from "../../../src/extensible_events_v1/PollStartEvent";
import { Poll } from "../../../src/models/poll";
import { isPollEvent, Poll } from "../../../src/models/poll";
import { getMockClientWithEventEmitter, mockClientMethodsUser } from "../../test-utils/client";
import { flushPromises } from "../../test-utils/flushPromises";
import { mkEvent } from "../../test-utils/test-utils";

jest.useFakeTimers();

Expand Down Expand Up @@ -453,4 +456,31 @@ describe("Poll", () => {
expect(responses.getRelations()).toEqual([responseEvent]);
});
});

describe("isPollEvent", () => {
it("should return »false« for a non-poll event", () => {
const messageEvent = mkEvent({
event: true,
type: EventType.RoomMessage,
content: {},
user: mockClient.getSafeUserId(),
room: room.roomId,
});
expect(isPollEvent(messageEvent)).toBe(false);
});

it.each([[M_POLL_START.name], [M_POLL_RESPONSE.name], [M_POLL_END.name]])(
"should return »true« for a »%s« event",
(type: string) => {
const pollEvent = mkEvent({
event: true,
type,
content: {},
user: mockClient.getSafeUserId(),
room: room.roomId,
});
expect(isPollEvent(pollEvent)).toBe(true);
},
);
});
});
49 changes: 46 additions & 3 deletions spec/unit/room.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ limitations under the License.
*/

import { mocked } from "jest-mock";
import { M_POLL_KIND_DISCLOSED, M_POLL_RESPONSE, PollStartEvent } from "matrix-events-sdk";
import { M_POLL_KIND_DISCLOSED, M_POLL_RESPONSE, M_POLL_START, PollStartEvent } from "matrix-events-sdk";

import * as utils from "../test-utils/test-utils";
import { emitPromise } from "../test-utils/test-utils";
Expand Down Expand Up @@ -53,6 +53,7 @@ import { FeatureSupport, Thread, THREAD_RELATION_TYPE, ThreadEvent } from "../..
import { Crypto } from "../../src/crypto";
import { mkThread } from "../test-utils/thread";
import { getMockClientWithEventEmitter, mockClientMethodsUser } from "../test-utils/client";
import { logger } from "../../src/logger";

describe("Room", function () {
const roomId = "!foo:bar";
Expand Down Expand Up @@ -171,6 +172,8 @@ describe("Room", function () {
room.oldState = room.getLiveTimeline().startState = utils.mock(RoomState, "oldState");
// @ts-ignore
room.currentState = room.getLiveTimeline().endState = utils.mock(RoomState, "currentState");

jest.spyOn(logger, "warn");
});

describe("getCreator", () => {
Expand Down Expand Up @@ -3261,7 +3264,7 @@ describe("Room", function () {
expect(room.emit).toHaveBeenCalledWith(PollEvent.New, pollInstance);
});

it("adds related events to poll models", async () => {
it("adds related events to poll models and log errors", async () => {
const pollStartEvent = makePollStart("1");
const pollStartEvent2 = makePollStart("2");
const events = [pollStartEvent, pollStartEvent2];
Expand All @@ -3274,13 +3277,27 @@ describe("Room", function () {
},
},
});

const messageEvent = new MatrixEvent({
type: "m.room.messsage",
content: {
text: "hello",
},
});

const errorEvent = new MatrixEvent({
type: M_POLL_START.name,
content: {
text: "Error!!!!",
},
});

const error = new Error("Test error");

mocked(client.decryptEventIfNeeded).mockImplementation(async (event: MatrixEvent) => {
if (event === errorEvent) throw error;
});

// init poll
await room.processPollEvents(events);

Expand All @@ -3289,14 +3306,40 @@ describe("Room", function () {
jest.spyOn(poll, "onNewRelation");
jest.spyOn(poll2, "onNewRelation");

await room.processPollEvents([pollResponseEvent, messageEvent]);
await room.processPollEvents([errorEvent, messageEvent, pollResponseEvent]);

// only called for relevant event
expect(poll.onNewRelation).toHaveBeenCalledTimes(1);
expect(poll.onNewRelation).toHaveBeenCalledWith(pollResponseEvent);

// only called on poll with relation
expect(poll2.onNewRelation).not.toHaveBeenCalled();

expect(logger.warn).toHaveBeenCalledWith("Error processing poll event", errorEvent.getId(), error);
});

it("should retry on decryption", async () => {
const pollStartEventId = "poll1";
const pollStartEvent = makePollStart(pollStartEventId);
// simulate decryption failure
const isDecryptionFailureSpy = jest.spyOn(pollStartEvent, "isDecryptionFailure").mockReturnValue(true);

await room.processPollEvents([pollStartEvent]);
// do not expect a poll to show up for the room
expect(room.polls.get(pollStartEventId)).toBeUndefined();

// now emit a Decrypted event but keep the decryption failure
pollStartEvent.emit(MatrixEventEvent.Decrypted, pollStartEvent);
// still do not expect a poll to show up for the room
expect(room.polls.get(pollStartEventId)).toBeUndefined();

// clear decryption failure and emit a Decrypted event again
isDecryptionFailureSpy.mockRestore();
pollStartEvent.emit(MatrixEventEvent.Decrypted, pollStartEvent);

// the poll should now show up in the room's polls
const poll = room.polls.get(pollStartEventId);
expect(poll?.pollId).toBe(pollStartEventId);
});
});

Expand Down
13 changes: 13 additions & 0 deletions src/models/poll.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import { M_POLL_START } from "matrix-events-sdk";

import { M_POLL_END, M_POLL_RESPONSE } from "../@types/polls";
import { MatrixClient } from "../client";
import { PollStartEvent } from "../extensible_events_v1/PollStartEvent";
Expand Down Expand Up @@ -266,3 +268,14 @@ export class Poll extends TypedEventEmitter<Exclude<PollEvent, PollEvent.New>, P
);
}
}

/**
* Tests whether the event is a start, response or end poll event.
*
* @param event - Event to test
* @returns true if the event is a poll event, else false
*/
export const isPollEvent = (event: MatrixEvent): boolean => {
const eventType = event.getType();
return M_POLL_START.matches(eventType) || M_POLL_RESPONSE.matches(eventType) || M_POLL_END.matches(eventType);
};
69 changes: 48 additions & 21 deletions src/models/room.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import {
import { IStateEventWithRoomId } from "../@types/search";
import { RelationsContainer } from "./relations-container";
import { ReadReceipt, synthesizeReceipt } from "./read-receipt";
import { Poll, PollEvent } from "./poll";
import { isPollEvent, Poll, PollEvent } from "./poll";

// These constants are used as sane defaults when the homeserver doesn't support
// the m.room_versions capability. In practice, KNOWN_SAFE_ROOM_VERSION should be
Expand Down Expand Up @@ -1897,35 +1897,62 @@ export class Room extends ReadReceipt<RoomEmittedEvents, RoomEventHandlerMap> {
this.threadsReady = true;
}

/**
* Calls {@link processPollEvent} for a list of events.
*
* @param events - List of events
*/
public async processPollEvents(events: MatrixEvent[]): Promise<void> {
const processPollStartEvent = (event: MatrixEvent): void => {
if (!M_POLL_START.matches(event.getType())) return;
for (const event of events) {
weeman1337 marked this conversation as resolved.
Show resolved Hide resolved
try {
// Continue if the event is a clear text, non-poll event.
if (!event.isEncrypted() && !isPollEvent(event)) continue;

/**
* Try to decrypt the event. Promise resolution does not guarantee a successful decryption.
* Retry is handled in {@link processPollEvent}.
*/
await this.client.decryptEventIfNeeded(event);
this.processPollEvent(event);
} catch (err) {
logger.warn("Error processing poll event", event.getId(), err);
}
}
}

/**
* Processes poll events:
* If the event has a decryption failure, it will listen for decryption and tries again.
* If it is a poll start event ({@link M_POLL_START}),
* it creates and stores a Poll model and emits a PollEvent.New event.
* If the event is related to a poll, it will add it to the poll.
* Noop for other cases.
*
* @param event - Event that could be a poll event
*/
private async processPollEvent(event: MatrixEvent): Promise<void> {
if (event.isDecryptionFailure()) {
event.once(MatrixEventEvent.Decrypted, (maybeDecryptedEvent: MatrixEvent) => {
this.processPollEvent(maybeDecryptedEvent);
});
return;
}

if (M_POLL_START.matches(event.getType())) {
try {
const poll = new Poll(event, this.client, this);
this.polls.set(event.getId()!, poll);
this.emit(PollEvent.New, poll);
} catch {}
// poll creation can fail for malformed poll start events
};

const processPollRelationEvent = (event: MatrixEvent): void => {
const relationEventId = event.relationEventId;
if (relationEventId && this.polls.has(relationEventId)) {
const poll = this.polls.get(relationEventId);
poll?.onNewRelation(event);
}
};
return;
}

const processPollEvent = (event: MatrixEvent): void => {
processPollStartEvent(event);
processPollRelationEvent(event);
};
const relationEventId = event.relationEventId;

for (const event of events) {
try {
await this.client.decryptEventIfNeeded(event);
processPollEvent(event);
} catch {}
if (relationEventId && this.polls.has(relationEventId)) {
const poll = this.polls.get(relationEventId);
poll?.onNewRelation(event);
}
}

Expand Down