Skip to content

Commit fd4d716

Browse files
authored
fix: ScreenshotIntegration not being added for web (#3055)
* Fix enabling of screenshots for web * Update * Update tests * Update * Fix analyze * Update CHANGELOG
1 parent dbd526b commit fd4d716

File tree

10 files changed

+143
-39
lines changed

10 files changed

+143
-39
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+
- `ScreenshotIntegration` not being added for web ([#3055](https://github.com/getsentry/sentry-dart/pull/3055))
8+
39
## 9.4.0
410

511
### Fixes

flutter/lib/src/event_processor/screenshot_event_processor.dart

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,10 @@ import 'dart:typed_data';
33

44
import 'package:meta/meta.dart';
55
import '../../sentry_flutter.dart';
6-
import '../renderer/renderer.dart';
76
import '../screenshot/recorder.dart';
87
import '../screenshot/recorder_config.dart';
98

9+
import '../screenshot/screenshot_support.dart';
1010
import '../utils/debouncer.dart';
1111

1212
class ScreenshotEventProcessor implements EventProcessor {
@@ -47,6 +47,15 @@ class ScreenshotEventProcessor implements EventProcessor {
4747
return event; // No need to attach screenshot of feedback form.
4848
}
4949

50+
final renderer = _options.rendererWrapper.renderer;
51+
if (!_options.isScreenshotSupported) {
52+
_options.log(
53+
SentryLevel.debug,
54+
'Screenshot: not supported in this environment with renderer $renderer',
55+
);
56+
return event;
57+
}
58+
5059
// skip capturing in case of debouncing (=too many frequent capture requests)
5160
// the BeforeCaptureCallback may overrule the debouncing decision
5261
final shouldDebounce = _debouncer.shouldDebounce();
@@ -70,7 +79,7 @@ class ScreenshotEventProcessor implements EventProcessor {
7079
} else if (shouldDebounce) {
7180
_options.log(
7281
SentryLevel.debug,
73-
'Skipping screenshot capture due to debouncing (too many captures within ${_debouncer.waitTime.inMilliseconds}ms)',
82+
'Screenshot: skipping capture due to debouncing (too many captures within ${_debouncer.waitTime.inMilliseconds}ms)',
7483
);
7584
takeScreenshot = false;
7685
}
@@ -81,7 +90,7 @@ class ScreenshotEventProcessor implements EventProcessor {
8190
} catch (exception, stackTrace) {
8291
_options.log(
8392
SentryLevel.error,
84-
'The beforeCaptureScreenshot/beforeScreenshot callback threw an exception',
93+
'Screenshot: the beforeCaptureScreenshot/beforeScreenshot callback threw an exception',
8594
exception: exception,
8695
stackTrace: stackTrace,
8796
);
@@ -90,16 +99,6 @@ class ScreenshotEventProcessor implements EventProcessor {
9099
}
91100
}
92101

93-
final renderer = _options.rendererWrapper.getRenderer();
94-
95-
if (_options.platform.isWeb && renderer != FlutterRenderer.canvasKit) {
96-
_options.log(
97-
SentryLevel.debug,
98-
'Cannot take screenshot with ${renderer?.name} renderer.',
99-
);
100-
return event;
101-
}
102-
103102
final screenshotData = await createScreenshot();
104103
if (screenshotData != null) {
105104
hint.screenshot = SentryAttachment.fromScreenshotData(screenshotData);

flutter/lib/src/renderer/renderer.dart

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ import 'unknown_renderer.dart'
66

77
@internal
88
class RendererWrapper {
9-
FlutterRenderer? getRenderer() {
10-
return implementation.getRenderer();
11-
}
9+
late final FlutterRenderer? renderer = implementation.getRenderer();
1210
}
1311

1412
enum FlutterRenderer {
@@ -18,10 +16,13 @@ enum FlutterRenderer {
1816
/// https://docs.flutter.dev/perf/impeller
1917
impeller,
2018

21-
/// https://docs.flutter.dev/platform-integration/web/renderers
19+
/// https://docs.flutter.dev/platform-integration/web/renderers#canvaskit
2220
canvasKit,
2321

24-
/// https://docs.flutter.dev/platform-integration/web/renderers
22+
/// https://docs.flutter.dev/platform-integration/web/renderers#skwasm
23+
skwasm,
24+
25+
/// HTML is still there but considered legacy
2526
html,
2627
unknown,
2728
}
Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,9 @@
1-
import 'dart:js_interop';
1+
import 'package:flutter/foundation.dart';
22

33
import 'renderer.dart';
44

55
FlutterRenderer? getRenderer() {
6-
return isCanvasKitRenderer ? FlutterRenderer.canvasKit : FlutterRenderer.html;
6+
if (isCanvasKit) return FlutterRenderer.canvasKit;
7+
if (isSkwasm) return FlutterRenderer.skwasm;
8+
return FlutterRenderer.html;
79
}
8-
9-
bool get isCanvasKitRenderer {
10-
return _windowFlutterCanvasKit != null;
11-
}
12-
13-
// These values are set by the engine. They are used to determine if the
14-
// application is using canvaskit or skwasm.
15-
//
16-
// See https://github.com/flutter/flutter/blob/414d9238720a3cde85475f49ce0ba313f95046f7/packages/flutter/lib/src/foundation/_capabilities_web.dart#L10
17-
@JS('window.flutterCanvasKit')
18-
external JSAny? get _windowFlutterCanvasKit;
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import '../../sentry_flutter.dart';
2+
import '../renderer/renderer.dart';
3+
4+
extension ScreenshotSupport on SentryFlutterOptions {
5+
/// Returns `true` if capturing screenshots are allowed in the current environment.
6+
///
7+
/// - On mobile / desktop we allow them unconditionally.
8+
/// - On Web we allow them only when the renderer is CanvasKit or Skwasm.
9+
bool get isScreenshotSupported {
10+
// Mobile / desktop → always OK.
11+
if (!platform.isWeb) return true;
12+
13+
const supportedWebRenderers = {
14+
FlutterRenderer.canvasKit,
15+
FlutterRenderer.skwasm,
16+
};
17+
return supportedWebRenderers.contains(rendererWrapper.renderer);
18+
}
19+
}

flutter/lib/src/sentry_flutter.dart

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,8 @@ import 'native/factory.dart';
2727
import 'native/native_scope_observer.dart';
2828
import 'native/sentry_native_binding.dart';
2929
import 'profiling.dart';
30-
import 'renderer/renderer.dart';
3130
import 'replay/integration.dart';
31+
import 'screenshot/screenshot_support.dart';
3232
import 'utils/platform_dispatcher_wrapper.dart';
3333
import 'version.dart';
3434
import 'view_hierarchy/view_hierarchy_integration.dart';
@@ -205,8 +205,7 @@ mixin SentryFlutter {
205205
options.enableDartSymbolication = false;
206206
}
207207

208-
final renderer = options.rendererWrapper.getRenderer();
209-
if (!platform.isWeb || renderer == FlutterRenderer.canvasKit) {
208+
if (options.isScreenshotSupported) {
210209
integrations.add(ScreenshotIntegration());
211210
}
212211

flutter/test/event_processor/screenshot_event_processor_test.dart

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,12 @@ void main() {
8383
added: false, isWeb: true);
8484
});
8585

86+
testWidgets('adds screenshot attachment with skwasm renderer',
87+
(tester) async {
88+
await _addScreenshotAttachment(tester, FlutterRenderer.skwasm,
89+
added: true, isWeb: true);
90+
});
91+
8692
testWidgets('does add screenshot in correct resolution for low',
8793
(tester) async {
8894
final height = SentryScreenshotQuality.low.targetResolution()!;

flutter/test/mocks.dart

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,9 +124,7 @@ class MockRendererWrapper implements RendererWrapper {
124124
final FlutterRenderer? _renderer;
125125

126126
@override
127-
FlutterRenderer? getRenderer() {
128-
return _renderer;
129-
}
127+
FlutterRenderer? get renderer => _renderer;
130128
}
131129

132130
class TestBindingWrapper implements BindingWrapper {
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import 'package:flutter_test/flutter_test.dart';
2+
import 'package:sentry/src/platform/mock_platform.dart';
3+
import 'package:sentry_flutter/sentry_flutter.dart';
4+
import 'package:sentry_flutter/src/renderer/renderer.dart';
5+
import 'package:sentry_flutter/src/screenshot/screenshot_support.dart';
6+
7+
import '../mocks.dart';
8+
9+
void main() {
10+
group('isScreenshotSupported', () {
11+
SentryFlutterOptions _buildOptions({
12+
required bool isWeb,
13+
FlutterRenderer? renderer,
14+
}) {
15+
final options = defaultTestOptions();
16+
options.platform = MockPlatform(isWeb: isWeb);
17+
options.rendererWrapper = MockRendererWrapper(renderer);
18+
return options;
19+
}
20+
21+
test('returns true for non-web platforms', () {
22+
final options = _buildOptions(isWeb: false);
23+
expect(options.isScreenshotSupported, isTrue);
24+
});
25+
26+
test('returns true for web canvasKit renderer', () {
27+
final options = _buildOptions(
28+
isWeb: true,
29+
renderer: FlutterRenderer.canvasKit,
30+
);
31+
expect(options.isScreenshotSupported, isTrue);
32+
});
33+
34+
test('returns true for web skwasm renderer', () {
35+
final options = _buildOptions(
36+
isWeb: true,
37+
renderer: FlutterRenderer.skwasm,
38+
);
39+
expect(options.isScreenshotSupported, isTrue);
40+
});
41+
42+
test('returns false for web html renderer', () {
43+
final options = _buildOptions(
44+
isWeb: true,
45+
renderer: FlutterRenderer.html,
46+
);
47+
expect(options.isScreenshotSupported, isFalse);
48+
});
49+
50+
test('returns false for web unknown renderer', () {
51+
final options = _buildOptions(
52+
isWeb: true,
53+
renderer: FlutterRenderer.unknown,
54+
);
55+
expect(options.isScreenshotSupported, isFalse);
56+
});
57+
});
58+
}

flutter/test/sentry_flutter_test.dart

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -564,12 +564,12 @@ void main() {
564564
await Sentry.close();
565565
}, testOn: 'vm');
566566

567-
test('installed with canvasKit renderer', () async {
567+
test('installed on web with canvasKit renderer', () async {
568568
List<Integration> integrations = [];
569569

570570
final sentryFlutterOptions =
571571
defaultTestOptions(checker: MockRuntimeChecker())
572-
..platform = MockPlatform.iOS()
572+
..platform = MockPlatform.iOS(isWeb: true)
573573
..rendererWrapper = MockRendererWrapper(FlutterRenderer.canvasKit)
574574
..release = ''
575575
..dist = '';
@@ -591,6 +591,33 @@ void main() {
591591
await Sentry.close();
592592
}, testOn: 'browser');
593593

594+
test('installed on web with skwasm renderer', () async {
595+
List<Integration> integrations = [];
596+
597+
final sentryFlutterOptions =
598+
defaultTestOptions(checker: MockRuntimeChecker())
599+
..platform = MockPlatform.iOS(isWeb: true)
600+
..rendererWrapper = MockRendererWrapper(FlutterRenderer.skwasm)
601+
..release = ''
602+
..dist = '';
603+
604+
await SentryFlutter.init(
605+
(options) async {
606+
integrations = options.integrations;
607+
},
608+
appRunner: appRunner,
609+
options: sentryFlutterOptions,
610+
);
611+
612+
expect(
613+
integrations
614+
.map((e) => e.runtimeType)
615+
.contains(ScreenshotIntegration),
616+
true);
617+
618+
await Sentry.close();
619+
}, testOn: 'browser');
620+
594621
test('not installed with html renderer', () async {
595622
List<Integration> integrations = [];
596623

0 commit comments

Comments
 (0)