Skip to content

Commit ba3890f

Browse files
TW-2834 Fix missing messages when open chat room by notification
1 parent 040478a commit ba3890f

File tree

4 files changed

+139
-11
lines changed

4 files changed

+139
-11
lines changed
Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
# 34. Fix Notification Redirection Race Condition
2+
3+
Date: 2026-01-15
4+
5+
## Status
6+
7+
Accepted
8+
9+
## Context
10+
11+
Users reported that when clicking a notification to navigate to a room, sometimes the messages would fail to load, and the "load more" function would not trigger.
12+
Investigation revealed a race condition in `ChatController`:
13+
14+
- The timeline loading logic (`_tryLoadTimeline`) was called in `initState` but used the `room` variable.
15+
- The `room` variable was previously initialized in the `build` method of `ChatView`.
16+
- When navigating via deep link or notification, the build method might not have run before `initState` tried to access `room`, leading to initialization with a null or stale room, or simply failing to load the correct timeline for the context event.
17+
18+
Additionally, when switching between rooms via notifications (e.g., being in Room A and clicking a notification for Room B), the `ChatController` did not cleanly detect the change and re-initialize the timeline, leading to mixed state.
19+
20+
## Decision
21+
22+
We decided to move the room initialization logic to the earliest possible point in the `ChatController` lifecycle and explicitly handle room updates.
23+
24+
### 1. Early Room Initialization
25+
26+
Moved `room` initialization from `ChatView.build` to `ChatController.initState`.
27+
28+
- A new method `_initRoom()` was created to initialize `matrix` client and `room`.
29+
- `_initRoom()` is called at the beginning of `initState`.
30+
31+
### 2. Explicit Update Handling
32+
33+
Updated `didUpdateWidget` in `ChatController`:
34+
35+
- Checks if `widget.roomId` has changed compared to `oldWidget.roomId`.
36+
- If changed, it performs a full room state reset and re-initialization:
37+
- Unsubscribes existing room event listeners (canceling any active timers or streams).
38+
- Clears and re-fetches pinned events.
39+
- Resets cached presence and participants data.
40+
- Calls `_initRoom()` to switch the internal matrix room reference.
41+
- Re-subscribes to room listeners for the new room.
42+
- Calls `_tryLoadTimeline()` to load the initial messages for the new room.
43+
- This ensures all room-scoped resources are fully refreshed and correctly isolated on room switch.
44+
45+
### 3. Removal of Side-Effects from Build
46+
47+
Removed the assignment of `controller.room` inside `ChatView.build`. The build method now strictly reads from the controller and returns early if `room` is null (though `initState` protections make this unlikely to happen in a visible way).
48+
49+
## Consequences
50+
51+
### Positive
52+
53+
- **Reliability:** Notifications and deep links now reliably load the correct room and timeline.
54+
- **Correctness:** Timeline loading (`_tryLoadTimeline`) now is guaranteed to have a valid `room` instance.
55+
- **Maintainability:** `ChatView.build` is purer and has fewer side effects. State initialization is where it belongs, in `initState`.
56+
57+
### Negative
58+
59+
- **Boilerplate:** We have to manually handle `didUpdateWidget` to detect changes that `build` would have picked up automatically (though incorrectly for this use case).
60+
61+
### Files Modified
62+
63+
1. `lib/pages/chat/chat.dart` - `initState`, `_initRoom`, `didUpdateWidget`.
64+
2. `lib/pages/chat/chat_view.dart` - Removed initialization logic from `build`.

lib/pages/chat/chat.dart

Lines changed: 69 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,12 @@ class ChatController extends State<Chat>
622622
scrollToEventId(fullyRead, highlight: false);
623623
}
624624

625-
void _tryLoadTimeline() async {
625+
Future<void> _tryLoadTimeline() async {
626+
final currentRoomId = roomId;
627+
if (room == null && currentRoomId != null) {
628+
room = matrix?.client.getRoomById(currentRoomId);
629+
}
630+
626631
_updateOpeningChatViewStateNotifier(ViewEventListLoading());
627632
loadTimelineFuture = _getTimeline();
628633
try {
@@ -649,6 +654,8 @@ class ChatController extends State<Chat>
649654
scrollToEventIdAndHighlight(eventId);
650655
}
651656
});
657+
if (!mounted || roomId != currentRoomId) return;
658+
652659
await _tryRequestHistory();
653660
final fullyRead = room?.fullyRead;
654661
if (fullyRead == null || fullyRead.isEmpty || fullyRead == '') {
@@ -657,7 +664,6 @@ class ChatController extends State<Chat>
657664
if (room?.hasNewMessages == true) {
658665
_initUnreadLocation(fullyRead);
659666
}
660-
if (!mounted) return;
661667
} catch (e, s) {
662668
Logs().e('Failed to load timeline', e, s);
663669
rethrow;
@@ -1225,8 +1231,12 @@ class ChatController extends State<Chat>
12251231
}
12261232

12271233
Future<void> _getTimeline({String? eventContextId}) async {
1234+
final currentRoomId = roomId;
12281235
await Matrix.of(context).client.roomsLoading;
12291236
await Matrix.of(context).client.accountDataLoading;
1237+
1238+
if (!mounted || roomId != currentRoomId) return;
1239+
12301240
if (eventContextId != null &&
12311241
(!eventContextId.isValidMatrixId || eventContextId.sigil != '\$')) {
12321242
eventContextId = null;
@@ -1241,12 +1251,14 @@ class ChatController extends State<Chat>
12411251
);
12421252
} catch (e, s) {
12431253
Logs().w('Unable to load timeline on event ID $eventContextId', e, s);
1244-
if (!mounted) return;
1254+
if (!mounted || roomId != currentRoomId) return;
12451255
timeline = await room?.getTimeline(onUpdate: updateView);
1246-
if (!mounted) return;
1256+
if (!mounted || roomId != currentRoomId) return;
12471257
}
12481258
timeline?.requestKeys(onlineKeyBackupOnly: false);
1249-
if (room!.markedUnread) room?.markUnread(false);
1259+
1260+
// Safely check room state
1261+
if (room?.markedUnread == true) room?.markUnread(false);
12501262

12511263
return;
12521264
}
@@ -2968,9 +2980,37 @@ class ChatController extends State<Chat>
29682980
EventTypes.Sticker,
29692981
];
29702982

2983+
void _resetState() {
2984+
// Cancel any pending operations or subscriptions for the old room
2985+
onUpdateEventStreamSubcription?.cancel();
2986+
cachedPresenceStreamSubscription?.cancel();
2987+
_jumpToEventIdSubscription?.cancel();
2988+
ignoredUsersStreamSub?.cancel();
2989+
_timestampTimer?.cancel();
2990+
2991+
// Clear notifiers that might hold old room state
2992+
replyEventNotifier.value = null;
2993+
editEventNotifier.value = null;
2994+
stickyTimestampNotifier.value = null;
2995+
selectedEvents.clear();
2996+
pinnedEventsController.reset();
2997+
2998+
// Reset timeline and futures
2999+
timeline?.cancelSubscriptions();
3000+
timeline = null;
3001+
loadTimelineFuture = null;
3002+
3003+
// Reset other state variables
3004+
_pendingReadMarkerEventId = null;
3005+
_currentChatScrollState = ChatScrollState.endScroll;
3006+
}
3007+
29713008
Future<void> _tryRequestHistory() async {
29723009
if (timeline == null) return;
29733010

3011+
// Check mounted after async gap if any (none here but good practice)
3012+
if (!mounted) return;
3013+
29743014
final allMembershipEvents = timeline!.events.every(
29753015
(event) => event.type == EventTypes.RoomMember,
29763016
);
@@ -2987,6 +3027,7 @@ class ChatController extends State<Chat>
29873027
await requestHistory(historyCount: _defaultEventCountDisplay).then((
29883028
response,
29893029
) async {
3030+
if (!mounted) return;
29903031
Logs().v('Chat::_tryRequestHistory():: Try request history success');
29913032
if (allMembershipEvents) {
29923033
await requestHistory(
@@ -3001,7 +3042,9 @@ class ChatController extends State<Chat>
30013042
} catch (e) {
30023043
Logs().e('Chat::_tryRequestHistory():: Error - $e');
30033044
} finally {
3004-
_updateOpeningChatViewStateNotifier(ViewEventListSuccess());
3045+
if (mounted) {
3046+
_updateOpeningChatViewStateNotifier(ViewEventListSuccess());
3047+
}
30053048
}
30063049
} else {
30073050
_updateOpeningChatViewStateNotifier(ViewEventListSuccess());
@@ -3594,6 +3637,7 @@ class ChatController extends State<Chat>
35943637
@override
35953638
void initState() {
35963639
super.initState();
3640+
_initRoom();
35973641
_initializePinnedEvents();
35983642
_listenOnJumpToEventFromSearch();
35993643
registerPasteShortcutListeners();
@@ -3622,6 +3666,14 @@ class ChatController extends State<Chat>
36223666
showEmojiPickerComposerNotifier.addListener(_emojiPickerListener);
36233667
}
36243668

3669+
void _initRoom() {
3670+
// Only init if not already initialized or needed
3671+
matrix ??= Matrix.read(context);
3672+
final client = matrix!.client;
3673+
sendingClient = client;
3674+
room = client.getRoomById(roomId!);
3675+
}
3676+
36253677
@override
36263678
void didChangeDependencies() {
36273679
super.didChangeDependencies();
@@ -3637,6 +3689,17 @@ class ChatController extends State<Chat>
36373689
if (PlatformInfos.isMobile) {
36383690
highlightEventId = GoRouterState.of(context).uri.queryParameters['event'];
36393691
} else {
3692+
if (oldWidget.roomId != widget.roomId) {
3693+
_resetState();
3694+
_initRoom();
3695+
_tryLoadTimeline();
3696+
_initializePinnedEvents();
3697+
_listenRoomUpdateEvent();
3698+
// Re-initialize other room-specific items
3699+
initCachedPresence();
3700+
_requestParticipants();
3701+
listenIgnoredUser();
3702+
}
36403703
final currentLocation = html.window.location.href;
36413704
highlightEventId = Uri.tryParse(
36423705
Uri.tryParse(currentLocation)?.fragment ?? '',

lib/pages/chat/chat_pinned_events/pinned_events_controller.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,12 @@ class PinnedEventsController {
152152
jumpToPinnedMessageCallback?.call(index);
153153
}
154154

155+
void reset() {
156+
currentPinnedEventNotifier.value = null;
157+
getPinnedMessageNotifier.value = Right(ChatGetPinnedEventsInitial());
158+
_pinnedEventsSubscription?.cancel();
159+
}
160+
155161
void dispose() {
156162
currentPinnedEventNotifier.dispose();
157163
getPinnedMessageNotifier.dispose();

lib/pages/chat/chat_view.dart

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import 'package:fluffychat/pages/chat/events/message_content_mixin.dart';
77
import 'package:fluffychat/presentation/mixins/audio_mixin.dart';
88
import 'package:fluffychat/resource/image_paths.dart';
99
import 'package:fluffychat/utils/stream_extension.dart';
10-
import 'package:fluffychat/widgets/matrix.dart';
1110
import 'package:fluffychat/widgets/twake_components/twake_icon_button.dart';
1211
import 'package:flutter/material.dart';
1312
import 'package:fluffychat/generated/l10n/app_localizations.dart';
@@ -92,10 +91,6 @@ class ChatView extends StatelessWidget with MessageContentMixin {
9291

9392
@override
9493
Widget build(BuildContext context) {
95-
controller.matrix ??= Matrix.of(context);
96-
final client = controller.matrix!.client;
97-
controller.sendingClient ??= client;
98-
controller.room = controller.sendingClient!.getRoomById(controller.roomId!);
9994
if (controller.room == null) {
10095
return const SizedBox.shrink();
10196
}

0 commit comments

Comments
 (0)