matrix-react-sdk
matrix-react-sdk copied to clipboard
matrix-org
Ask to refresh timeline when historical messages are imported (MSC2716)
Ask to refresh timeline when historical messages are imported (MSC2716)
Associated matrix-js-sdk PR: https://github.com/matrix-org/matrix-js-sdk/pull/2299
Loading state:
Error state for network errors (no need to submit debug logs):
Error state for other errors you can submit debug logs:
Beware: edge case
There is one edge case where the "History import detected" banner can re-appear after you have refreshed the timeline. matrix-js-sdk only persists /sync data to IndexDB every 5 minutes which means it can have a stale next_batch pagination token compared to the latest events you might be seeing on screen. So if you receive a marker event, it will show the banner to refresh the timeline. You can then use the button to refresh your timeline and the banner will disappear as expected. Then if you restart your entire Element client (like refreshing the entire browser), Element will syncFromCache with the /sync response from 5 minutes ago and query /sync with that old pagination token which will fetch the marker event again and show the banner again.
Why are we just setting
timlineNeedsRefresh(and prompting the user) 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.
-- https://github.com/matrix-org/matrix-js-sdk/pull/2299
Testing strategy
- In your Synapse
homeserver.yaml, enableexperimental_features->msc2716_enabled: true - Have the room open in Element with this PR checked out locally
- Create a room with a MSC2716 room version,
POST http://localhost:8008/_matrix/client/v3/createRoomwith{ "preset": "public_chat", "name": "test-msc2716", "room_version": "org.matrix.msc2716v3", "invite": ["@erictroupetester:my.synapse.server"] } - Send some messages in the room just so we have something to import in the middle of. Let's just say you sent 10 messages (1 though 10)
- Pick one of those previous messages (lets just pick 5) and use it in the
?prev_event_idparameter for the batch send request:POST http://localhost:8008/_matrix/client/unstable/org.matrix.msc2716/rooms/{roomId}/batch_send?prev_event_id={someEventId}with{ "state_events_at_start": [ { "content": { "displayname": "some-display-name-for-@maria-01234:my.synapse.server", "membership": "join" }, "origin_server_ts": 1642055555924, "sender": "@maria-01234:my.synapse.server", "state_key": "@maria-01234:my.synapse.server", "type": "m.room.member" } ], "events": [ { "content": { "body": "Historical 0 (batch=0)", "msgtype": "m.text", "org.matrix.msc2716.historical": true }, "origin_server_ts": 1642055555924, "sender": "@maria-01234:my.synapse.server", "type": "m.room.message" }, { "content": { "body": "Historical 1 (batch=0)", "msgtype": "m.text", "org.matrix.msc2716.historical": true }, "origin_server_ts": 1642055555925, "sender": "@maria-01234:my.synapse.server", "type": "m.room.message" }, { "content": { "body": "Historical 2 (batch=0)", "msgtype": "m.text", "org.matrix.msc2716.historical": true }, "origin_server_ts": 1642055555926, "sender": "@maria-01234:my.synapse.server", "type": "m.room.message" }, { "content": { "body": "Historical 3 (batch=0)", "msgtype": "m.text", "org.matrix.msc2716.historical": true }, "origin_server_ts": 1642055555927, "sender": "@maria-01234:my.synapse.server", "type": "m.room.message" } ] } - Send a marker event in the room to expose the history to everyone. Use the
base_insertion_event_idfrom the batch send response in theorg.matrix.msc2716.marker.insertionfield in the JSON body:PUT http://localhost:8008/_matrix/client/r0/rooms/{roomId}/state/org.matrix.msc2716.marker{ "org.matrix.msc2716.marker.insertion": "$jxIsfEXmnUtl6KroclFmselixa1FEzQfBgKWlqV3Bfg", } - Notice the history import detected banner
- Press the
Refresh timelinebutton - Notice the room timeline refreshes and now you can see the historical messages in the room
Dev notes
First reviewed in https://github.com/matrix-org/matrix-react-sdk/pull/8303
loadTimeline
refreshTimeline
onRoomTimelineReset
onRoomTimeline
onMessageListFillRequest
onMessageListUnfillRequest
Generic apply data-xxx attributes to to child components (based off of https://stackoverflow.com/a/65656589/796832):
function getDataAttributes(inputProps = {}) {
return Object.keys(inputProps).reduce((dataAttrs, key) => {
if (key.startsWith('data-')) {
dataAttrs[key] = inputProps[key];
}
return dataAttrs;
}, {});
}
<SomeJSXComponent {...getDataAttributes(restProps)}>
Cypress
Clean up Synapse Docker instances from Cypress
docker ps --filter "name=react-sdk-cypress-synapse-*" -aq | xargs docker stop
docker ps --filter "name=react-sdk-cypress-synapse-*" -aq | xargs docker rm
Can't queue within cy.intercept callbacks
Error:
Cypress detected that you returned a promise from a command while also invoking one or more cy commands in that promise.
Snippet without async still fails ❌
cy.intercept(contextUrl, (req) => {
console.log('intercepted');
cy.get('.mx_BasicMessageComposer').click()
// Now continue the request
req.reply();
}).as('contextRequestThatWillMakeNewTimeline');
Snippet with async use case fails too ❌
cy.intercept(contextUrl, async (req) => {
console.log('intercepted');
const {event_id: eventIdWhileRefrshingTimeline } = await asMatrixClient.sendMessage(roomId, null, {
body: `live_event while trying to refresh timeline`,
msgtype: "m.text",
});
// Wait for the message to show up for the logged in user
// indicating that a sync happened
waitForEventIdsInClient([eventIdWhileRefrshingTimeline]);
// Now continue the request
req.continue();
//req.reply();
}).as('contextRequestThatWillMakeNewTimeline');
Related issues:
- https://github.com/cypress-io/cypress/discussions/17341
- https://github.com/cypress-io/cypress/discussions/15578
- https://github.com/cypress-io/cypress/issues/3514
- https://github.com/cypress-io/cypress/issues/16472
cy.intercept(..., { forceNetworkError: true }) does not play well with multiple matching requests or overriding
cy.intercept()routes are matched in reverse order of definition, except for routes which are defined with{ middleware: true }, which always run first. This allows you to override existingcy.intercept()declarations by defining an overlappingcy.intercept():https://docs.cypress.io/api/commands/intercept#Interception-lifecycle
But if you try to override a cy.intercept that uses forceNetworkError, the second match will always fail. Both of the following examples work if you just switch it out for { statusCode: 500 } instead of the network failure
Snippet trying to override intercepts ❌
cy.intercept('/context', { times: 1 }, { forceNetworkError: true }).as('contextRequestThatWillTryToMakeNewTimeline');
cy.wait('@contextRequestThatWillMakeNewTimeline').should('have.property', 'error');
// This will fail on two different fronts:
// 1. This won't have a "response" because the first intercept is overriding
// 2. Not specifying a StaticResponse will cause it to be overriden as well even if we use `statusCode: 500` above instead of a network error
cy.intercept('/context').as('contextRequestThatWillMakeNewTimeline');
cy.wait('@contextRequestThatWillMakeNewTimeline').its('response.statusCode').should('eq', 200);
Snippet trying to conditionally fail/passthrough request ❌
let failContextRequests = true;
cy.wrap(null).then(function() {
cy.intercept('/context', async (req) => {
// Force a network error the first time so we have to retry
console.log('intercept', failContextRequests);
if (failContextRequests) {
console.log('intercept error');
req.reply({ forceNetworkError: true });
} else {
// Otherwise, respond as normal
console.log('intercept normal');
req.reply();
}
}).as('contextRequestThatWillMakeNewTimeline');
});
// Press "Refresh timeline"
cy.get(`[data-cy="refresh-timeline-button"]`).click();
// Make sure the request was intercepted and thew an network error
cy.wait('@contextRequestThatWillMakeNewTimeline').should('have.property', 'error');
// Make sure we tell the user that an error happened
cy.get(`[data-cy="historical-import-detected-error-content"]`).should("exist");
// Allow the requests to succeed now
cy.wrap(null).then(() => {
failContextRequests = false;
});
// Press "Refresh timeline" again, this time the network request should succeed
cy.get(`[data-cy="refresh-timeline-button"]`).click();
// Make sure the request was intercepted and succeeded
// (this will fail with never having a response)
cy.wait('@contextRequestThatWillMakeNewTimeline').its('response.statusCode').should('eq', 200);
Related issues:
- https://github.com/cypress-io/cypress/issues/15317
- https://github.com/cypress-io/cypress/issues/20697
- https://github.com/cypress-io/cypress/issues/20728
Debugging Synapse in the Cypress e2e tests
In cypress/plugins/synapsedocker/index.ts#L124, change the image that the synapsedocker Cypress plugin is using to spin up Synapse instances to matrixdotorg/synapse:latest. Restart your Cypress process.
In your local Synapse checkout, make any changes and build the image: docker build -t matrixdotorg/synapse -f docker/Dockerfile .
Logging code in Synapse to generate the tabulated events from the room you're interested in:
logger.info(
"asdf_get_debug_events_in_room_ordered_by_depth\n%s",
await self.store.asdf_get_debug_events_in_room_ordered_by_depth(room_id),
)
async def asdf_get_debug_events_in_room_ordered_by_depth(self, room_id: str) -> Any:
"""Gets the topological token in a room after or at the given stream
ordering.
Args:
room_id
"""
sql = (
"SELECT depth, stream_ordering, type, state_key, event_id FROM events"
" WHERE events.room_id = ?"
" ORDER BY depth DESC, stream_ordering DESC;"
)
rows = await self.db_pool.execute(
"asdf_get_debug_events_in_room_ordered_by_depth", None, sql, room_id
)
headerColumnLengthMap = {
"depth": 4,
"stream_ordering": 12,
"type": 30,
"state_key": 28,
# event_ids are 44 long but we don't need the extra 5 padding
# because it's the last item and we're just cheating by building
# this into the value instead of the format code.
"event_id": 39,
}
output = ""
row_format = "".join(
[
"{:<" + str(columnLength + 5) + "}"
for columnLength in headerColumnLengthMap.values()
]
)
output += row_format.format(*headerColumnLengthMap.keys()) + "\n"
for row in rows:
output += row_format.format(*[str(x) for x in row]) + "\n"
return output
Example output:
depth stream_ordering type state_key event_id
12 13 org.matrix.msc2716.marker marker_state_key_2087 $bokn_JFlThduvpLCDQcbzSo3UUIdzM3s3ldS5VdFyYQ
11 12 m.room.message None $U0_8sLrLF3pqcKnEg-3ewUbzyfHDS1EB6YfKVh-ZnQc
10 11 m.room.message None $0CIrzvW7ZTh0x4gtI5d3mXyDXvt1kuCTb7iISfNtHlQ
10 -3 org.matrix.msc2716.insertion None $dWZvpd2rEuImJdZ4aSk97Dj8cYXR-b2xMUMWG7FYm6E
10 -4 org.matrix.msc2716.batch None $h1deiGVBrEV8QCsAwbwxHkjFO-sHdwvP251N8GYaYIw
10 -5 m.room.message None $BrgyUUoD8ITK3BRaOGPUGMWiH78dhs9FrjCwH0wgwfI
10 -6 m.room.message None $1OOGZAlyTehMK7p3mJhVXRvs7qnJ4MRXtBMFHTONLHw
10 -7 m.room.message None $drnLdMjvo5e_NsfqBGVsPQcejyOZvbTrNXKvD2KZ_QE
10 -8 org.matrix.msc2716.insertion None $T6u8-XmI-ZFAnbxHx1HFXXTgCJ82lVtd71oulCc0Uo8
9 10 m.room.message None $Cu7LaaoGrCMnoXPn7uVsHWJ5W-XG4WSG-MiBWlY24z0
8 9 m.room.message None $AkJKzohur-0DNkrd6w3u9hzvdzYFIeEX31a1IwX7evo
7 8 m.room.member @userid_1961:localhost $jw6pB2SEBqlOhKiwIrUsGiH79aRHLGum1pJVY-t3L4o
6 7 m.room.name $3krkuB2G3MrzEBS3zGRvldzYpGAGsLmoIuMPvXXZE2w
5 6 m.room.history_visibility $pNca6LVT6x6vtG4AITsbMrxuO0AoEdnNsDb3jbFsCNw
4 5 m.room.join_rules $BYCJnsez0Ic1ZGMSvZa2NwLjZIigI_A-2amN3JkTkEY
3 4 m.room.power_levels $5D5WzmPfY2wwVu0YU6A815h6bw4Pb1L5NLlSbq9z-Og
2 3 m.room.member @gitter-badger:localhost $0aP1Sh_mIB4eLnA2vXaCfwS8wSNAWyugIo3N_yFYevc
1 2 m.room.create $dNbk-MSdtXsPH7PYr3q6ZWy1z87B4wrMJ6SSf2rtvm8
1 -2 m.room.member @maria-01234:localhost $Xz7QxLkgDRtaTOse1oAw0wpFBmM58Oqdkj08Q3wGQ5k
Todo
- [x] Add tests
- [x] Add e2e test, https://github.com/matrix-org/matrix-react-sdk/pull/8354#discussion_r852588155
- [x] Integrate method from
matrix-js-sdkto actually refresh the timeline, https://github.com/matrix-org/matrix-react-sdk/pull/8303#discussion_r848999579
Here's what your changelog entry will look like:
✨ Features
- Ask to refresh timeline when historical messages are imported (MSC2716) (#8354). Contributed by @MadLittleMods.
Preview: https://pr8354--matrix-react-sdk.netlify.app ⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.
Codecov Report
Merging #8354 (f571107) into develop (27118a9) will increase coverage by
0.07%. The diff coverage is40.62%.
@@ Coverage Diff @@
## develop #8354 +/- ##
===========================================
+ Coverage 30.01% 30.09% +0.07%
===========================================
Files 879 879
Lines 50193 50220 +27
Branches 12783 12791 +8
===========================================
+ Hits 15064 15112 +48
+ Misses 35129 35108 -21
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/components/structures/FilePanel.tsx | 1.12% <0.00%> (ø) |
|
| src/components/structures/TimelinePanel.tsx | 1.63% <0.00%> (-0.03%) |
:arrow_down: |
| src/indexing/EventIndex.ts | 0.59% <0.00%> (ø) |
|
| src/components/structures/RoomStatusBar.tsx | 52.88% <72.22%> (+44.93%) |
:arrow_up: |
![codecov[bot] avatar](https://avatars.githubusercontent.com/in/254?v=4 "Published by a pub.dev verified publisher"){kind=small}
What's the state of this change? It looks like a huge amount of work has gone into it, and it's been around for ~6 months!
@andybalaam The implementation on this side has been ready to go since April 20th when it had the review pass. And I've kept it up to date at various points (whenever I merge develop).
It's been blocked behind the refresh timeline function in the matrix-js-sdk which has an edge case with threads. I initially fixed this in a v1 fix but it didn't pass review (see comments there) and now a v2 change has been made but still requires more cycles to conform.
I'm focusing on other things so I guess it will continue to languish 🗻
OK so @MadLittleMods it doesn't look like you're blocked on us for anything here - just needing to find time to respond to review requests on https://github.com/matrix-org/matrix-js-sdk/pull/2852 . Let me know if we can help.