Skip to content

Commit b64dbdc

Browse files
Timeline needs to refresh when we see a MSC2716 marker event (#2299)
Inform the client that historical messages were imported in the timeline and they should refresh the timeline in order to see the new events. Companion `matrix-react-sdk` PR: matrix-org/matrix-react-sdk#8354 The `marker` events are being used as state now because this way they can't be lost in a timeline gap. Regardless of when they were sent, we will still have the latest version of the state to compare against. Any time we see our latest state value change for marker events, prompt the user that the timeline needs to refresh. > In a [sync meeting with @ara4n](https://docs.google.com/document/d/1KCEmpnGr4J-I8EeaVQ8QJZKBDu53ViI7V62y5BzfXr0/edit#bookmark=id.67nio1ka8znc), we came up with the idea to make the `marker` events as state events. When the client sees that the `m.room.marker` state changed to a different event ID, it can throw away all of the timeline and re-fetch as needed. > > For homeservers where the [same problem](matrix-org/matrix-spec-proposals#2716 (comment)) can happen, we probably don't want to throw away the whole timeline but it can go up the `unsigned.replaces_state` chain of the `m.room.marker` state events to get them all. > > In terms of state performance, there could be thousands of `marker` events in a room but it's no different than room members joining and leaving over and over like an IRC room. > > *-- matrix-org/matrix-spec-proposals#2716 (comment) ### Why are we just setting `timlineNeedsRefresh` (and [prompting the user](matrix-org/matrix-react-sdk#8354)) instead of automatically refreshing the timeline for the user? If we refreshed the timeline automatically, someone could cause your Element client to constantly refresh the timeline by just sending marker events over and over. Granted, you probably want to leave a room like this 🤷. Perhaps also some sort of DOS vector since everyone will be refreshing and hitting the server at the exact same time. In order to avoid the timeline maybe going blank during the refresh, we could re-fetch the new events first, then replace the timeline. But the points above still stand on why we shouldn't.
1 parent 2e27a41 commit b64dbdc

16 files changed

+1635
-125
lines changed

spec/integ/matrix-client-event-timeline.spec.js

+71
Original file line numberDiff line numberDiff line change
@@ -540,6 +540,77 @@ describe("MatrixClient event timelines", function() {
540540
});
541541
});
542542

543+
describe("getLatestTimeline", function() {
544+
it("should create a new timeline for new events", function() {
545+
const room = client.getRoom(roomId);
546+
const timelineSet = room.getTimelineSets()[0];
547+
548+
const latestMessageId = 'event1:bar';
549+
550+
httpBackend.when("GET", "/rooms/!foo%3Abar/messages")
551+
.respond(200, function() {
552+
return {
553+
chunk: [{
554+
event_id: latestMessageId,
555+
}],
556+
};
557+
});
558+
559+
httpBackend.when("GET", `/rooms/!foo%3Abar/context/${encodeURIComponent(latestMessageId)}`)
560+
.respond(200, function() {
561+
return {
562+
start: "start_token",
563+
events_before: [EVENTS[1], EVENTS[0]],
564+
event: EVENTS[2],
565+
events_after: [EVENTS[3]],
566+
state: [
567+
ROOM_NAME_EVENT,
568+
USER_MEMBERSHIP_EVENT,
569+
],
570+
end: "end_token",
571+
};
572+
});
573+
574+
return Promise.all([
575+
client.getLatestTimeline(timelineSet).then(function(tl) {
576+
// Instead of this assertion logic, we could just add a spy
577+
// for `getEventTimeline` and make sure it's called with the
578+
// correct parameters. This doesn't feel too bad to make sure
579+
// `getLatestTimeline` is doing the right thing though.
580+
expect(tl.getEvents().length).toEqual(4);
581+
for (let i = 0; i < 4; i++) {
582+
expect(tl.getEvents()[i].event).toEqual(EVENTS[i]);
583+
expect(tl.getEvents()[i].sender.name).toEqual(userName);
584+
}
585+
expect(tl.getPaginationToken(EventTimeline.BACKWARDS))
586+
.toEqual("start_token");
587+
expect(tl.getPaginationToken(EventTimeline.FORWARDS))
588+
.toEqual("end_token");
589+
}),
590+
httpBackend.flushAllExpected(),
591+
]);
592+
});
593+
594+
it("should throw error when /messages does not return a message", () => {
595+
const room = client.getRoom(roomId);
596+
const timelineSet = room.getTimelineSets()[0];
597+
598+
httpBackend.when("GET", "/rooms/!foo%3Abar/messages")
599+
.respond(200, () => {
600+
return {
601+
chunk: [
602+
// No messages to return
603+
],
604+
};
605+
});
606+
607+
return Promise.all([
608+
expect(client.getLatestTimeline(timelineSet)).rejects.toThrow(),
609+
httpBackend.flushAllExpected(),
610+
]);
611+
});
612+
});
613+
543614
describe("paginateEventTimeline", function() {
544615
it("should allow you to paginate backwards", function() {
545616
const room = client.getRoom(roomId);

spec/integ/matrix-client-room-timeline.spec.js

+269-1
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import * as utils from "../test-utils/test-utils";
22
import { EventStatus } from "../../src/models/event";
3+
import { RoomEvent } from "../../src";
34
import { TestClient } from "../TestClient";
45

56
describe("MatrixClient room timelines", function() {
@@ -579,7 +580,7 @@ describe("MatrixClient room timelines", function() {
579580
});
580581
});
581582

582-
it("should emit a 'Room.timelineReset' event", function() {
583+
it("should emit a `RoomEvent.TimelineReset` event when the sync response is `limited`", function() {
583584
const eventData = [
584585
utils.mkMessage({ user: userId, room: roomId }),
585586
];
@@ -608,4 +609,271 @@ describe("MatrixClient room timelines", function() {
608609
});
609610
});
610611
});
612+
613+
describe('Refresh live timeline', () => {
614+
const initialSyncEventData = [
615+
utils.mkMessage({ user: userId, room: roomId }),
616+
utils.mkMessage({ user: userId, room: roomId }),
617+
utils.mkMessage({ user: userId, room: roomId }),
618+
];
619+
620+
const contextUrl = `/rooms/${encodeURIComponent(roomId)}/context/` +
621+
`${encodeURIComponent(initialSyncEventData[2].event_id)}`;
622+
const contextResponse = {
623+
start: "start_token",
624+
events_before: [initialSyncEventData[1], initialSyncEventData[0]],
625+
event: initialSyncEventData[2],
626+
events_after: [],
627+
state: [
628+
USER_MEMBERSHIP_EVENT,
629+
],
630+
end: "end_token",
631+
};
632+
633+
let room;
634+
beforeEach(async () => {
635+
setNextSyncData(initialSyncEventData);
636+
637+
// Create a room from the sync
638+
await Promise.all([
639+
httpBackend.flushAllExpected(),
640+
utils.syncPromise(client, 1),
641+
]);
642+
643+
// Get the room after the first sync so the room is created
644+
room = client.getRoom(roomId);
645+
expect(room).toBeTruthy();
646+
});
647+
648+
it('should clear and refresh messages in timeline', async () => {
649+
// `/context` request for `refreshLiveTimeline()` -> `getEventTimeline()`
650+
// to construct a new timeline from.
651+
httpBackend.when("GET", contextUrl)
652+
.respond(200, function() {
653+
// The timeline should be cleared at this point in the refresh
654+
expect(room.timeline.length).toEqual(0);
655+
656+
return contextResponse;
657+
});
658+
659+
// Refresh the timeline.
660+
await Promise.all([
661+
room.refreshLiveTimeline(),
662+
httpBackend.flushAllExpected(),
663+
]);
664+
665+
// Make sure the message are visible
666+
const resultantEventsInTimeline = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents();
667+
const resultantEventIdsInTimeline = resultantEventsInTimeline.map((event) => event.getId());
668+
expect(resultantEventIdsInTimeline).toEqual([
669+
initialSyncEventData[0].event_id,
670+
initialSyncEventData[1].event_id,
671+
initialSyncEventData[2].event_id,
672+
]);
673+
});
674+
675+
it('Perfectly merges timelines if a sync finishes while refreshing the timeline', async () => {
676+
// `/context` request for `refreshLiveTimeline()` ->
677+
// `getEventTimeline()` to construct a new timeline from.
678+
//
679+
// We only resolve this request after we detect that the timeline
680+
// was reset(when it goes blank) and force a sync to happen in the
681+
// middle of all of this refresh timeline logic. We want to make
682+
// sure the sync pagination still works as expected after messing
683+
// the refresh timline logic messes with the pagination tokens.
684+
httpBackend.when("GET", contextUrl)
685+
.respond(200, () => {
686+
// Now finally return and make the `/context` request respond
687+
return contextResponse;
688+
});
689+
690+
// Wait for the timeline to reset(when it goes blank) which means
691+
// it's in the middle of the refrsh logic right before the
692+
// `getEventTimeline()` -> `/context`. Then simulate a racey `/sync`
693+
// to happen in the middle of all of this refresh timeline logic. We
694+
// want to make sure the sync pagination still works as expected
695+
// after messing the refresh timline logic messes with the
696+
// pagination tokens.
697+
//
698+
// We define this here so the event listener is in place before we
699+
// call `room.refreshLiveTimeline()`.
700+
const racingSyncEventData = [
701+
utils.mkMessage({ user: userId, room: roomId }),
702+
];
703+
const waitForRaceySyncAfterResetPromise = new Promise((resolve, reject) => {
704+
let eventFired = false;
705+
// Throw a more descriptive error if this part of the test times out.
706+
const failTimeout = setTimeout(() => {
707+
if (eventFired) {
708+
reject(new Error(
709+
'TestError: `RoomEvent.TimelineReset` fired but we timed out trying to make' +
710+
'a `/sync` happen in time.',
711+
));
712+
} else {
713+
reject(new Error(
714+
'TestError: Timed out while waiting for `RoomEvent.TimelineReset` to fire.',
715+
));
716+
}
717+
}, 4000 /* FIXME: Is there a way to reference the current timeout of this test in Jest? */);
718+
719+
room.on(RoomEvent.TimelineReset, async () => {
720+
try {
721+
eventFired = true;
722+
723+
// The timeline should be cleared at this point in the refresh
724+
expect(room.getUnfilteredTimelineSet().getLiveTimeline().getEvents().length).toEqual(0);
725+
726+
// Then make a `/sync` happen by sending a message and seeing that it
727+
// shows up (simulate a /sync naturally racing with us).
728+
setNextSyncData(racingSyncEventData);
729+
httpBackend.when("GET", "/sync").respond(200, function() {
730+
return NEXT_SYNC_DATA;
731+
});
732+
await Promise.all([
733+
httpBackend.flush("/sync", 1),
734+
utils.syncPromise(client, 1),
735+
]);
736+
// Make sure the timeline has the racey sync data
737+
const afterRaceySyncTimelineEvents = room
738+
.getUnfilteredTimelineSet()
739+
.getLiveTimeline()
740+
.getEvents();
741+
const afterRaceySyncTimelineEventIds = afterRaceySyncTimelineEvents
742+
.map((event) => event.getId());
743+
expect(afterRaceySyncTimelineEventIds).toEqual([
744+
racingSyncEventData[0].event_id,
745+
]);
746+
747+
clearTimeout(failTimeout);
748+
resolve();
749+
} catch (err) {
750+
reject(err);
751+
}
752+
});
753+
});
754+
755+
// Refresh the timeline. Just start the function, we will wait for
756+
// it to finish after the racey sync.
757+
const refreshLiveTimelinePromise = room.refreshLiveTimeline();
758+
759+
await waitForRaceySyncAfterResetPromise;
760+
761+
await Promise.all([
762+
refreshLiveTimelinePromise,
763+
// Then flush the remaining `/context` to left the refresh logic complete
764+
httpBackend.flushAllExpected(),
765+
]);
766+
767+
// Make sure sync pagination still works by seeing a new message show up
768+
// after refreshing the timeline.
769+
const afterRefreshEventData = [
770+
utils.mkMessage({ user: userId, room: roomId }),
771+
];
772+
setNextSyncData(afterRefreshEventData);
773+
httpBackend.when("GET", "/sync").respond(200, function() {
774+
return NEXT_SYNC_DATA;
775+
});
776+
await Promise.all([
777+
httpBackend.flushAllExpected(),
778+
utils.syncPromise(client, 1),
779+
]);
780+
781+
// Make sure the timeline includes the the events from the `/sync`
782+
// that raced and beat us in the middle of everything and the
783+
// `/sync` after the refresh. Since the `/sync` beat us to create
784+
// the timeline, `initialSyncEventData` won't be visible unless we
785+
// paginate backwards with `/messages`.
786+
const resultantEventsInTimeline = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents();
787+
const resultantEventIdsInTimeline = resultantEventsInTimeline.map((event) => event.getId());
788+
expect(resultantEventIdsInTimeline).toEqual([
789+
racingSyncEventData[0].event_id,
790+
afterRefreshEventData[0].event_id,
791+
]);
792+
});
793+
794+
it('Timeline recovers after `/context` request to generate new timeline fails', async () => {
795+
// `/context` request for `refreshLiveTimeline()` -> `getEventTimeline()`
796+
// to construct a new timeline from.
797+
httpBackend.when("GET", contextUrl)
798+
.respond(500, function() {
799+
// The timeline should be cleared at this point in the refresh
800+
expect(room.timeline.length).toEqual(0);
801+
802+
return {
803+
errcode: 'TEST_FAKE_ERROR',
804+
error: 'We purposely intercepted this /context request to make it fail ' +
805+
'in order to test whether the refresh timeline code is resilient',
806+
};
807+
});
808+
809+
// Refresh the timeline and expect it to fail
810+
const settledFailedRefreshPromises = await Promise.allSettled([
811+
room.refreshLiveTimeline(),
812+
httpBackend.flushAllExpected(),
813+
]);
814+
// We only expect `TEST_FAKE_ERROR` here. Anything else is
815+
// unexpected and should fail the test.
816+
if (settledFailedRefreshPromises[0].status === 'fulfilled') {
817+
throw new Error('Expected the /context request to fail with a 500');
818+
} else if (settledFailedRefreshPromises[0].reason.errcode !== 'TEST_FAKE_ERROR') {
819+
throw settledFailedRefreshPromises[0].reason;
820+
}
821+
822+
// The timeline will be empty after we refresh the timeline and fail
823+
// to construct a new timeline.
824+
expect(room.timeline.length).toEqual(0);
825+
826+
// `/messages` request for `refreshLiveTimeline()` ->
827+
// `getLatestTimeline()` to construct a new timeline from.
828+
httpBackend.when("GET", `/rooms/${encodeURIComponent(roomId)}/messages`)
829+
.respond(200, function() {
830+
return {
831+
chunk: [{
832+
// The latest message in the room
833+
event_id: initialSyncEventData[2].event_id,
834+
}],
835+
};
836+
});
837+
// `/context` request for `refreshLiveTimeline()` ->
838+
// `getLatestTimeline()` -> `getEventTimeline()` to construct a new
839+
// timeline from.
840+
httpBackend.when("GET", contextUrl)
841+
.respond(200, function() {
842+
// The timeline should be cleared at this point in the refresh
843+
expect(room.timeline.length).toEqual(0);
844+
845+
return contextResponse;
846+
});
847+
848+
// Refresh the timeline again but this time it should pass
849+
await Promise.all([
850+
room.refreshLiveTimeline(),
851+
httpBackend.flushAllExpected(),
852+
]);
853+
854+
// Make sure sync pagination still works by seeing a new message show up
855+
// after refreshing the timeline.
856+
const afterRefreshEventData = [
857+
utils.mkMessage({ user: userId, room: roomId }),
858+
];
859+
setNextSyncData(afterRefreshEventData);
860+
httpBackend.when("GET", "/sync").respond(200, function() {
861+
return NEXT_SYNC_DATA;
862+
});
863+
await Promise.all([
864+
httpBackend.flushAllExpected(),
865+
utils.syncPromise(client, 1),
866+
]);
867+
868+
// Make sure the message are visible
869+
const resultantEventsInTimeline = room.getUnfilteredTimelineSet().getLiveTimeline().getEvents();
870+
const resultantEventIdsInTimeline = resultantEventsInTimeline.map((event) => event.getId());
871+
expect(resultantEventIdsInTimeline).toEqual([
872+
initialSyncEventData[0].event_id,
873+
initialSyncEventData[1].event_id,
874+
initialSyncEventData[2].event_id,
875+
afterRefreshEventData[0].event_id,
876+
]);
877+
});
878+
});
611879
});

0 commit comments

Comments
 (0)