Support for UTD error codes in `[Sync]TimelineEvent` #4070

richvdh · 2024-10-04T10:42:36Z

This PR takes the TimelineEventKind that was added in #4082, and adds a third enum variant which is specific to unable-to-decrypt events. We update the two methods which are responsible for decrypting encrypted events that have been received from the network (decrypt_sync_room_event and decrypt_room_event), and make them return a [Sync]TimelineEvent of the relevant kind instead of a megolm error.

Since pretty much everything downstream accesses UTD events via TimelineEventKind::raw(), we don't have to change much else to keep things working.

Part of #4048.

crates/matrix-sdk-ui/src/timeline/tests/edit.rs

codecov · 2024-10-04T11:39:00Z

Codecov Report

Attention: Patch coverage is 68.62745% with 16 lines in your changes missing coverage. Please review.

Project coverage is 84.80%. Comparing base (0c26988) to head (1cc4629).
Report is 7 commits behind head on main.

Files with missing lines	Patch %	Lines
crates/matrix-sdk-ui/src/notification_client.rs	0.00%	13 Missing ⚠️
...es/matrix-sdk-common/src/deserialized_responses.rs	66.66%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4070      +/-   ##
==========================================
- Coverage   84.84%   84.80%   -0.04%     
==========================================
  Files         269      269              
  Lines       28800    28827      +27     
==========================================
+ Hits        24434    24446      +12     
- Misses       4366     4381      +15

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

When `decrypt_sync_room_event` fails to decrypt an event, return the UTD as a `SyncTimelineEvent` instead of an Error.

stefanceriu

Nice, looks good to me 👍

stefanceriu · 2024-10-21T13:03:30Z

crates/matrix-sdk-ui/src/notification_client.rs

+                                    // key; retry in a few.
+                                    wait *= 2;
+                                } else {
+                                    trace!("Event could not be decrypted, but waiting longer is unlikely to help: {:?}", utd_info.reason);


This feels useful on the nse level, should we maybe promote it to at least info?

Well, the failure will already be logged at warning in the crypto layer, so it's not immediately obvious that logging it again is right.

I'll bump this one to debug as a compromise for now.

stefanceriu · 2024-10-21T13:04:56Z

crates/matrix-sdk-ui/src/notification_client.rs

+                        matrix_sdk::deserialized_responses::TimelineEventKind::UnableToDecrypt {
+                            utd_info, ..
+                        } => {
+                            trace!(


Maybe here too?

Again, the actual error will be logged at warning, and in this case it's just replicating the previous behaviour (note the trace!("Encryption sync failed to decrypt the event: {err}"); just below).

I can change this if you feel strongly but it's not obvious to me that we should.

No no, that's quite okay, I didn't check the lower levels to see if we print it anywhere else.

When `decrypt_room_event` fails to decrypt an event, return the UTD as a `TimelineEvent` instead of an Error.

Make the tests behave the same way as the network code, by returning UTDs as `TimelineEventKind::UnableToDecrypt` instead of `TimelineEventKind::PlainText`.

richvdh force-pushed the rav/utds_in_timeline_event branch 3 times, most recently from 455bbd4 to c737e86 Compare October 4, 2024 11:24

richvdh commented Oct 4, 2024

View reviewed changes

crates/matrix-sdk-ui/src/timeline/tests/edit.rs Outdated Show resolved Hide resolved

richvdh force-pushed the rav/utds_in_timeline_event branch 8 times, most recently from bcd57e8 to ab28964 Compare October 10, 2024 17:08

richvdh force-pushed the rav/utds_in_timeline_event branch 2 times, most recently from 03d4148 to 8fa3bf9 Compare October 11, 2024 15:36

richvdh mentioned this pull request Oct 15, 2024

Timeline: Extend UtdCause with new reason codes #4126

Merged

richvdh force-pushed the rav/utds_in_timeline_event branch from f702bc9 to bad2be9 Compare October 15, 2024 13:23

richvdh changed the title ~~WIP: support for UTD error codes in the timeline~~ WIP: support for UTD error codes in [Sync]TimelineEvent Oct 15, 2024

richvdh force-pushed the rav/utds_in_timeline_event branch 3 times, most recently from 06eff7f to d00c1a2 Compare October 16, 2024 16:55

richvdh changed the title ~~WIP: support for UTD error codes in [Sync]TimelineEvent~~ Support for UTD error codes in [Sync]TimelineEvent Oct 16, 2024

richvdh force-pushed the rav/utds_in_timeline_event branch 3 times, most recently from 0997324 to d838ed5 Compare October 17, 2024 15:59

richvdh added 2 commits October 21, 2024 12:52

refactor(common): add TimelineEventKind::UnableToDecrypt

e227926

refactor(timeline): store UTDs in decrypt_sync_room_event

172b9fc

When `decrypt_sync_room_event` fails to decrypt an event, return the UTD as a `SyncTimelineEvent` instead of an Error.

richvdh force-pushed the rav/utds_in_timeline_event branch from d838ed5 to 63b94a3 Compare October 21, 2024 11:53

richvdh marked this pull request as ready for review October 21, 2024 11:54

richvdh requested a review from a team as a code owner October 21, 2024 11:54

richvdh requested review from stefanceriu and removed request for a team October 21, 2024 11:54

stefanceriu approved these changes Oct 21, 2024

View reviewed changes

richvdh added 2 commits October 21, 2024 17:11

refactor(timeline): store UTDs in decrypt_room_event

f4e8222

When `decrypt_room_event` fails to decrypt an event, return the UTD as a `TimelineEvent` instead of an Error.

test: Update tests to use new UTD TimelineEventKind variant

1cc4629

Make the tests behave the same way as the network code, by returning UTDs as `TimelineEventKind::UnableToDecrypt` instead of `TimelineEventKind::PlainText`.

richvdh force-pushed the rav/utds_in_timeline_event branch from 89bd8c1 to 1cc4629 Compare October 21, 2024 16:11

richvdh enabled auto-merge (rebase) October 21, 2024 16:11

richvdh merged commit 3887c10 into main Oct 21, 2024
40 checks passed

richvdh deleted the rav/utds_in_timeline_event branch October 21, 2024 16:26

richvdh mentioned this pull request Oct 23, 2024

Add support in timeline for the new MegolmError SenderIdentityNotTrusted when cross-signed TrustRequirement is enabled #4048

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support for UTD error codes in `[Sync]TimelineEvent` #4070

Support for UTD error codes in `[Sync]TimelineEvent` #4070

richvdh commented Oct 4, 2024 •

edited

Loading

codecov bot commented Oct 4, 2024 •

edited

Loading

stefanceriu left a comment

stefanceriu Oct 21, 2024

richvdh Oct 21, 2024

stefanceriu Oct 21, 2024

richvdh Oct 21, 2024

stefanceriu Oct 22, 2024

Support for UTD error codes in [Sync]TimelineEvent #4070

Support for UTD error codes in [Sync]TimelineEvent #4070

Conversation

richvdh commented Oct 4, 2024 • edited Loading

codecov bot commented Oct 4, 2024 • edited Loading

Codecov Report

stefanceriu left a comment

Choose a reason for hiding this comment

stefanceriu Oct 21, 2024

Choose a reason for hiding this comment

richvdh Oct 21, 2024

Choose a reason for hiding this comment

stefanceriu Oct 21, 2024

Choose a reason for hiding this comment

richvdh Oct 21, 2024

Choose a reason for hiding this comment

stefanceriu Oct 22, 2024

Choose a reason for hiding this comment

Support for UTD error codes in `[Sync]TimelineEvent` #4070

Support for UTD error codes in `[Sync]TimelineEvent` #4070

richvdh commented Oct 4, 2024 •

edited

Loading

codecov bot commented Oct 4, 2024 •

edited

Loading