Skip to content

Commit a52304a

Browse files
Mounix99Mounix99
committed
fix(message_widget): guard MessageCard._updateWidthLimit against unlaid RenderBox
`_MessageCardState._updateWidthLimit` is scheduled via `WidgetsBinding.instance.addPostFrameCallback` from `didChangeDependencies`, and reads `renderBox.size.width` when it fires. If the attachments subtree has been detached from the tree between scheduling and firing (e.g. the parent removed the message because the list was rebuilt, the chat was disposed, or the attachment was deleted), `RenderBox.size` throws `StateError: RenderBox was not laid out` per the framework contract. The throw is caught by `FlutterError.onError`, so users see a dropped frame rather than a crash — but Sentry still records it as a fatal error. In one production Flutter app the two corresponding Sentry issues have generated 1500+ events across 1100+ unique users in a single 2-week release. The error appears under any view path that has chat messages with attachments (chat, groupPost, splash, root). Fix is a `hasSize` guard before reading `.size`, plus moving the existing `mounted` check to the top of the function for symmetry (it was previously only checked before `setState`, so we still touched the unmounted render object first). The render-box-was-not-laid-out error is the canonical signature of this mistake in Flutter; the `hasSize` check is exactly what the framework docs recommend for this case.
1 parent 7f0804d commit a52304a

3 files changed

Lines changed: 187 additions & 5 deletions

File tree

‎packages/stream_chat_flutter/CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66
until the server confirmed it.
77
- Fixed a `FlutterError` ("A RenderViewport exceeded its maximum number of layout cycles") that
88
could occur when fast-scrolling through the message list.
9+
- Fixed `RenderBox was not laid out` thrown by `MessageCard._updateWidthLimit` when the attachments
10+
subtree was detached between scheduling the post-frame callback and it firing (e.g. the list was
11+
rebuilt or the message removed). Added a `hasSize` guard before reading `RenderBox.size`.
912

1013
## 9.24.0
1114

‎packages/stream_chat_flutter/lib/src/message_widget/message_card.dart‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -133,15 +133,19 @@ class _MessageCardState extends State<MessageCard> {
133133
}
134134

135135
void _updateWidthLimit() {
136+
if (!mounted) return;
137+
136138
final attachmentContext = attachmentsKey.currentContext;
137139
final renderBox = attachmentContext?.findRenderObject() as RenderBox?;
138-
final attachmentsWidth = renderBox?.size.width;
140+
// The attachments subtree may have been detached between scheduling this
141+
// post-frame callback and it firing. Reading [RenderBox.size] without
142+
// checking [RenderBox.hasSize] throws `RenderBox was not laid out`.
143+
if (renderBox == null || !renderBox.hasSize) return;
144+
final attachmentsWidth = renderBox.size.width;
139145

140-
if (attachmentsWidth == null || attachmentsWidth == 0) return;
146+
if (attachmentsWidth == 0) return;
141147

142-
if (mounted) {
143-
setState(() => widthLimit = attachmentsWidth);
144-
}
148+
setState(() => widthLimit = attachmentsWidth);
145149
}
146150

147151
@override

‎packages/stream_chat_flutter/test/src/message_widget/message_card_test.dart‎

Lines changed: 175 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
import 'package:flutter/material.dart';
2+
import 'package:flutter_test/flutter_test.dart';
3+
import 'package:mocktail/mocktail.dart';
4+
import 'package:stream_chat_flutter/stream_chat_flutter.dart';
5+
6+
import '../mocks.dart';
7+
8+
/// Always-handles attachment builder used to inject a known-size widget into
9+
/// the attachments slot of [MessageCard]. Lets each test control the size that
10+
/// `_updateWidthLimit` will read off the rendered RenderBox.
11+
class _FixedSizeAttachmentBuilder extends StreamAttachmentWidgetBuilder {
12+
const _FixedSizeAttachmentBuilder({required this.size});
13+
14+
final Size size;
15+
16+
@override
17+
bool canHandle(Message message, Map<String, List<Attachment>> attachments) =>
18+
true;
19+
20+
@override
21+
Widget build(
22+
BuildContext context,
23+
Message message,
24+
Map<String, List<Attachment>> attachments,
25+
) {
26+
return SizedBox.fromSize(size: size);
27+
}
28+
}
29+
30+
MessageCard _buildCard({
31+
required bool hasNonUrlAttachments,
32+
StreamAttachmentWidgetBuilder? attachmentBuilder,
33+
}) {
34+
return MessageCard(
35+
message: Message(
36+
id: 'm1',
37+
text: 'hi',
38+
attachments: [Attachment(type: AttachmentType.file)],
39+
),
40+
isFailedState: false,
41+
showUserAvatar: DisplayWidget.show,
42+
messageTheme: const StreamMessageThemeData(),
43+
hasQuotedMessage: false,
44+
hasUrlAttachments: false,
45+
hasNonUrlAttachments: hasNonUrlAttachments,
46+
hasPoll: false,
47+
isOnlyEmoji: false,
48+
isGiphy: false,
49+
attachmentBuilders: attachmentBuilder == null ? null : [attachmentBuilder],
50+
attachmentPadding: const EdgeInsets.all(4),
51+
attachmentShape: null,
52+
onAttachmentTap: (_, __) {},
53+
onShowMessage: (_, __) {},
54+
onReplyTap: (_) {},
55+
attachmentActionsModalBuilder: null,
56+
textPadding: const EdgeInsets.all(8),
57+
reverse: false,
58+
);
59+
}
60+
61+
/// Wraps [child] in a minimal [StreamChat] + [MaterialApp] so descendant
62+
/// widgets like [StreamMessageText] can resolve `StreamChat.of(context)`.
63+
Widget _wrap(Widget child) {
64+
final client = MockClient();
65+
final clientState = MockClientState();
66+
final user = OwnUser(id: 'user-id');
67+
when(() => client.state).thenReturn(clientState);
68+
when(() => clientState.currentUser).thenReturn(user);
69+
when(() => clientState.currentUserStream)
70+
.thenAnswer((_) => Stream.value(user));
71+
72+
return MaterialApp(
73+
home: StreamChat(
74+
client: client,
75+
child: Scaffold(body: Center(child: child)),
76+
),
77+
);
78+
}
79+
80+
void main() {
81+
group('MessageCard._updateWidthLimit', () {
82+
testWidgets(
83+
'applies width limit from laid-out attachments render box',
84+
(tester) async {
85+
const attachmentSize = Size(220, 80);
86+
87+
await tester.pumpWidget(
88+
_wrap(
89+
_buildCard(
90+
hasNonUrlAttachments: true,
91+
attachmentBuilder:
92+
const _FixedSizeAttachmentBuilder(size: attachmentSize),
93+
),
94+
),
95+
);
96+
// Allow the post-frame callback scheduled in didChangeDependencies to
97+
// fire and apply the width limit via setState.
98+
await tester.pump();
99+
100+
final container = tester.widget<Container>(
101+
find.descendant(
102+
of: find.byType(MessageCard),
103+
matching: find.byType(Container),
104+
),
105+
);
106+
expect(container.constraints?.maxWidth, attachmentSize.width);
107+
},
108+
);
109+
110+
testWidgets(
111+
'does not throw when the message card is unmounted before '
112+
'the post-frame callback fires',
113+
(tester) async {
114+
const attachmentSize = Size(120, 60);
115+
116+
await tester.pumpWidget(
117+
_wrap(
118+
_buildCard(
119+
hasNonUrlAttachments: true,
120+
attachmentBuilder:
121+
const _FixedSizeAttachmentBuilder(size: attachmentSize),
122+
),
123+
),
124+
);
125+
// Replace the MessageCard with an empty tree BEFORE pumping the next
126+
// frame. The post-frame callback queued in didChangeDependencies will
127+
// fire against an unmounted state — the new `if (!mounted) return;`
128+
// guard must keep it from throwing.
129+
await tester.pumpWidget(_wrap(const SizedBox.shrink()));
130+
131+
expect(tester.takeException(), isNull);
132+
},
133+
);
134+
135+
testWidgets(
136+
'leaves width limit unset when attachment reports zero width',
137+
(tester) async {
138+
await tester.pumpWidget(
139+
_wrap(
140+
_buildCard(
141+
hasNonUrlAttachments: true,
142+
attachmentBuilder:
143+
const _FixedSizeAttachmentBuilder(size: Size.zero),
144+
),
145+
),
146+
);
147+
await tester.pump();
148+
149+
final container = tester.widget<Container>(
150+
find.descendant(
151+
of: find.byType(MessageCard),
152+
matching: find.byType(Container),
153+
),
154+
);
155+
// The early-return on `attachmentsWidth == 0` means widthLimit stays
156+
// null and the constraints stay unconstrained on the width axis.
157+
expect(container.constraints?.maxWidth, double.infinity);
158+
},
159+
);
160+
161+
testWidgets(
162+
'skips width measurement entirely when there are no attachments',
163+
(tester) async {
164+
await tester.pumpWidget(
165+
_wrap(_buildCard(hasNonUrlAttachments: false)),
166+
);
167+
await tester.pump();
168+
169+
// No exception — the post-frame callback is not scheduled at all when
170+
// hasAttachments is false.
171+
expect(tester.takeException(), isNull);
172+
},
173+
);
174+
});
175+
}

0 commit comments

Comments
 (0)