Skip to content

Commit 765aa8b

Browse files
buenaflorbuenaflor
authored
fix: prevent false replay config restarts from ScreenshotWidgetStatus equality issues (#3114)
* Fix * Fix * Update test * Update CHANGELOG * Fix tests * Update replay_integration_test.dart * Fix AnalyzE * Add fallback with tolerance based checks due to double precision * Update * Update * Update * bit simpler * update * update * fix analyze
1 parent f761369 commit 765aa8b

File tree

5 files changed

+195
-11
lines changed

5 files changed

+195
-11
lines changed

‎CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
11
# Changelog
22

3+
## Unreleased
4+
5+
### Fixes
6+
7+
- False replay config restarts because of `ScreenshotWidgetStatus` equality issues ([#3114](https://github.com/getsentry/sentry-dart/pull/3114))
8+
39
## 9.6.0-beta.1
410

511
### Fixes

‎flutter/lib/src/replay/integration.dart

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -33,15 +33,20 @@ class ReplayIntegration extends Integration<SentryFlutterOptions> {
3333
}
3434

3535
SentryScreenshotWidget.onBuild((status, prevStatus) {
36-
if (status != prevStatus) {
37-
_native.setReplayConfig(ReplayConfig(
38-
windowWidth: status.size?.width ?? 0.0,
39-
windowHeight: status.size?.height ?? 0.0,
40-
width: replayOptions.quality.resolutionScalingFactor *
41-
(status.size?.width ?? 0.0),
42-
height: replayOptions.quality.resolutionScalingFactor *
43-
(status.size?.height ?? 0.0)));
36+
// Skip config update if the difference is negligible (e.g., due to floating-point precision)
37+
// e.g a size.height of 200.00001 and 200.001 could be treated as equals
38+
if (prevStatus != null && status.matches(prevStatus)) {
39+
return true;
4440
}
41+
42+
_native.setReplayConfig(ReplayConfig(
43+
windowWidth: status.size?.width ?? 0.0,
44+
windowHeight: status.size?.height ?? 0.0,
45+
width: replayOptions.quality.resolutionScalingFactor *
46+
(status.size?.width ?? 0.0),
47+
height: replayOptions.quality.resolutionScalingFactor *
48+
(status.size?.height ?? 0.0)));
49+
4550
return true;
4651
});
4752
}

‎flutter/lib/src/screenshot/sentry_screenshot_widget.dart

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ class _SentryScreenshotWidgetState extends State<SentryScreenshotWidget> {
107107
final status = SentryScreenshotWidgetStatus(
108108
size: mq.size,
109109
pixelRatio: mq.devicePixelRatio,
110-
orientantion: mq.orientation,
110+
orientation: mq.orientation,
111111
);
112112
final prevStatus = SentryScreenshotWidget._status;
113113
SentryScreenshotWidget._status = status;
@@ -180,11 +180,38 @@ class _SentryScreenshotWidgetState extends State<SentryScreenshotWidget> {
180180
class SentryScreenshotWidgetStatus {
181181
final Size? size;
182182
final double? pixelRatio;
183-
final Orientation? orientantion;
183+
final Orientation? orientation;
184+
185+
static const double _pixelRatioTolerance = 1e-6;
186+
static const double _sizeTolerance = 0.05;
184187

185188
const SentryScreenshotWidgetStatus({
186189
required this.size,
187190
required this.pixelRatio,
188-
required this.orientantion,
191+
required this.orientation,
189192
});
193+
194+
bool matches(SentryScreenshotWidgetStatus other) {
195+
if (identical(this, other)) return true;
196+
if (orientation != other.orientation) return false;
197+
198+
if (pixelRatio == null || other.pixelRatio == null) {
199+
if (pixelRatio != other.pixelRatio) return false;
200+
} else if ((pixelRatio! - other.pixelRatio!).abs() > _pixelRatioTolerance) {
201+
return false;
202+
}
203+
204+
if (size == null || other.size == null) {
205+
if (size != other.size) return false;
206+
} else {
207+
if ((size!.width - other.size!.width).abs() > _sizeTolerance) {
208+
return false;
209+
}
210+
if ((size!.height - other.size!.height).abs() > _sizeTolerance) {
211+
return false;
212+
}
213+
}
214+
215+
return true;
216+
}
190217
}

‎flutter/test/replay/replay_integration_test.dart

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33
@TestOn('vm')
44
library;
55

6+
import 'dart:ui';
7+
68
import 'package:flutter_test/flutter_test.dart';
79
import 'package:mockito/mockito.dart';
810
import 'package:sentry_flutter/sentry_flutter.dart';
@@ -30,6 +32,10 @@ void main() {
3032

3133
tearDown(() {
3234
SentryScreenshotWidget.reset();
35+
final binding = TestWidgetsFlutterBinding.ensureInitialized();
36+
for (final view in binding.platformDispatcher.views) {
37+
view.resetPhysicalSize();
38+
}
3339
});
3440

3541
for (var supportsReplay in [true, false]) {
@@ -104,4 +110,43 @@ void main() {
104110
expect(config.width, 640);
105111
expect(config.height, 480);
106112
});
113+
114+
testWidgets(
115+
'Does not call setReplayConfig again when widget size remains unchanged',
116+
(tester) async {
117+
options.replay.sessionSampleRate = 1.0;
118+
when(native.setReplayConfig(any)).thenReturn(null);
119+
sut.call(hub, options);
120+
121+
TestWidgetsFlutterBinding.ensureInitialized();
122+
123+
tester.view.physicalSize = Size(10, 20);
124+
await pumpTestElement(tester);
125+
await tester.pumpAndSettle(Duration(seconds: 1));
126+
127+
tester.view.physicalSize = Size(10, 20);
128+
await pumpTestElement(tester);
129+
await tester.pumpAndSettle(Duration(seconds: 1));
130+
131+
verify(native.setReplayConfig(any)).called(1);
132+
});
133+
134+
testWidgets('Does call setReplayConfig again when widget size changed',
135+
(tester) async {
136+
options.replay.sessionSampleRate = 1.0;
137+
when(native.setReplayConfig(any)).thenReturn(null);
138+
sut.call(hub, options);
139+
140+
TestWidgetsFlutterBinding.ensureInitialized();
141+
142+
tester.view.physicalSize = Size(10, 20);
143+
await pumpTestElement(tester);
144+
await tester.pumpAndSettle(Duration(seconds: 1));
145+
146+
tester.view.physicalSize = Size(20, 20);
147+
await pumpTestElement(tester);
148+
await tester.pumpAndSettle(Duration(seconds: 1));
149+
150+
verify(native.setReplayConfig(any)).called(2);
151+
});
107152
}

‎flutter/test/screenshot/sentry_screenshot_widget_test.dart

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,107 @@ void main() {
124124
expect(find.text('Send Bug Report'), findsOne);
125125
});
126126
});
127+
128+
group('SentryScreenshotWidgetStatus', () {
129+
group('matches', () {
130+
test('returns true for instances with very close pixelRatio values', () {
131+
final status1 = SentryScreenshotWidgetStatus(
132+
size: const Size(100, 200),
133+
pixelRatio: 2.0,
134+
orientation: Orientation.portrait,
135+
);
136+
final status2 = SentryScreenshotWidgetStatus(
137+
size: const Size(100, 200),
138+
pixelRatio: 2.0000001, // Very close to 2.0
139+
orientation: Orientation.portrait,
140+
);
141+
142+
expect(status1.matches(status2), isTrue);
143+
});
144+
145+
test('returns true for instances with very close size dimensions', () {
146+
final status1 = SentryScreenshotWidgetStatus(
147+
size: const Size(100.0, 200.0),
148+
pixelRatio: 2.0,
149+
orientation: Orientation.portrait,
150+
);
151+
final status2 = SentryScreenshotWidgetStatus(
152+
size: const Size(100.005, 200.005),
153+
// Very close dimensions (within 0.01 tolerance)
154+
pixelRatio: 2.0,
155+
orientation: Orientation.portrait,
156+
);
157+
158+
expect(status1.matches(status2), isTrue);
159+
});
160+
161+
test(
162+
'returns false for instances with significantly different pixelRatio',
163+
() {
164+
final status1 = SentryScreenshotWidgetStatus(
165+
size: const Size(100, 200),
166+
pixelRatio: 2.0,
167+
orientation: Orientation.portrait,
168+
);
169+
final status2 = SentryScreenshotWidgetStatus(
170+
size: const Size(100, 200),
171+
pixelRatio: 2.1, // Significantly different
172+
orientation: Orientation.portrait,
173+
);
174+
175+
expect(status1.matches(status2), isFalse);
176+
});
177+
178+
test(
179+
'returns false for instances with significantly different size dimensions',
180+
() {
181+
final status1 = SentryScreenshotWidgetStatus(
182+
size: const Size(100.0, 200.0),
183+
pixelRatio: 2.0,
184+
orientation: Orientation.portrait,
185+
);
186+
final status2 = SentryScreenshotWidgetStatus(
187+
size: const Size(100.1, 200.0), // Significantly different width
188+
pixelRatio: 2.0,
189+
orientation: Orientation.portrait,
190+
);
191+
192+
expect(status1.matches(status2), isFalse);
193+
});
194+
195+
test(
196+
'returns true for instances with significantly different size dimensions',
197+
() {
198+
final status1 = SentryScreenshotWidgetStatus(
199+
size: const Size(100.0, 200.0),
200+
pixelRatio: 2.0,
201+
orientation: Orientation.portrait,
202+
);
203+
final status2 = SentryScreenshotWidgetStatus(
204+
size: const Size(100.01, 200.0), // Significantly different width
205+
pixelRatio: 2.0,
206+
orientation: Orientation.portrait,
207+
);
208+
209+
expect(status1.matches(status2), isTrue);
210+
});
211+
212+
test('returns true if values are within tolerance', () {
213+
final status1 = SentryScreenshotWidgetStatus(
214+
size: const Size(100.0, 200.0),
215+
pixelRatio: 2.0,
216+
orientation: Orientation.portrait,
217+
);
218+
final status2 = SentryScreenshotWidgetStatus(
219+
size: const Size(100.005, 200.005),
220+
pixelRatio: 2.0000001,
221+
orientation: Orientation.portrait,
222+
);
223+
224+
expect(status1.matches(status2), isTrue);
225+
});
226+
});
227+
});
127228
}
128229

129230
@GenerateMocks([Callbacks])

0 commit comments

Comments
 (0)