diff --git a/CHANGELOG.md b/CHANGELOG.md index 28b309dbdb..51543538ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Fixes +- Flutter renderer information was removed on dart:io platforms since it didn't add the correct value ([#1723](https://github.com/getsentry/sentry-dart/pull/1723)) - Unsupported types with Expando ([#1690](https://github.com/getsentry/sentry-dart/pull/1690)) ### Features diff --git a/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart b/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart index b0eaa0fe71..f8003248ec 100644 --- a/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart +++ b/flutter/lib/src/event_processor/flutter_enricher_event_processor.dart @@ -132,6 +132,8 @@ class FlutterEnricherEventProcessor implements EventProcessor { // ignore: deprecated_member_use final hasRenderView = _widgetsBinding?.renderViewElement != null; + final renderer = _options.rendererWrapper.getRenderer()?.name; + return { 'has_render_view': hasRenderView.toString(), if (tempDebugBrightnessOverride != null) @@ -149,7 +151,7 @@ class FlutterEnricherEventProcessor implements EventProcessor { // Also always fails in tests. // See https://github.com/flutter/flutter/issues/83919 // 'window_is_visible': _window.viewConfiguration.visible, - 'renderer': _options.rendererWrapper.getRenderer().name, + if (renderer != null) 'renderer': renderer, }; } diff --git a/flutter/lib/src/event_processor/screenshot_event_processor.dart b/flutter/lib/src/event_processor/screenshot_event_processor.dart index bb1157c1de..263722fdad 100644 --- a/flutter/lib/src/event_processor/screenshot_event_processor.dart +++ b/flutter/lib/src/event_processor/screenshot_event_processor.dart @@ -32,10 +32,13 @@ class ScreenshotEventProcessor implements EventProcessor { } final renderer = _options.rendererWrapper.getRenderer(); - if (renderer != FlutterRenderer.skia && + + if (_options.platformChecker.isWeb && renderer != FlutterRenderer.canvasKit) { - _options.logger(SentryLevel.debug, - 'Cannot take screenshot with ${_options.rendererWrapper.getRenderer().name} renderer.'); + _options.logger( + SentryLevel.debug, + 'Cannot take screenshot with ${renderer?.name} renderer.', + ); return event; } diff --git a/flutter/lib/src/renderer/html_renderer.dart b/flutter/lib/src/renderer/html_renderer.dart index b768a14c09..de41f5e77a 100644 --- a/flutter/lib/src/renderer/html_renderer.dart +++ b/flutter/lib/src/renderer/html_renderer.dart @@ -2,7 +2,7 @@ import 'dart:js' as js; import 'renderer.dart'; -FlutterRenderer getRenderer() { +FlutterRenderer? getRenderer() { return isCanvasKitRenderer ? FlutterRenderer.canvasKit : FlutterRenderer.html; } diff --git a/flutter/lib/src/renderer/io_renderer.dart b/flutter/lib/src/renderer/io_renderer.dart index 961feb7a1f..77322a4e08 100644 --- a/flutter/lib/src/renderer/io_renderer.dart +++ b/flutter/lib/src/renderer/io_renderer.dart @@ -1,5 +1,3 @@ import 'renderer.dart'; -FlutterRenderer getRenderer() { - return FlutterRenderer.skia; -} +FlutterRenderer? getRenderer() => null; diff --git a/flutter/lib/src/renderer/renderer.dart b/flutter/lib/src/renderer/renderer.dart index b1b9c2dd69..3e41eced70 100644 --- a/flutter/lib/src/renderer/renderer.dart +++ b/flutter/lib/src/renderer/renderer.dart @@ -6,14 +6,22 @@ import 'unknown_renderer.dart' @internal class RendererWrapper { - FlutterRenderer getRenderer() { + FlutterRenderer? getRenderer() { return implementation.getRenderer(); } } enum FlutterRenderer { + /// https://skia.org/ skia, + + /// https://docs.flutter.dev/perf/impeller + impeller, + + /// https://docs.flutter.dev/platform-integration/web/renderers canvasKit, + + /// https://docs.flutter.dev/platform-integration/web/renderers html, unknown, } diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index d1fd8ef1c2..cef52ee564 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -167,8 +167,7 @@ mixin SentryFlutter { integrations.add(LoadImageListIntegration(channel)); } final renderer = options.rendererWrapper.getRenderer(); - if (renderer == FlutterRenderer.skia || - renderer == FlutterRenderer.canvasKit) { + if (!platformChecker.isWeb || renderer == FlutterRenderer.canvasKit) { integrations.add(ScreenshotIntegration()); } diff --git a/flutter/test/event_processor/flutter_enricher_event_processor_test.dart b/flutter/test/event_processor/flutter_enricher_event_processor_test.dart index 4e748b2934..6b95793ad7 100644 --- a/flutter/test/event_processor/flutter_enricher_event_processor_test.dart +++ b/flutter/test/event_processor/flutter_enricher_event_processor_test.dart @@ -22,7 +22,38 @@ void main() { fixture = Fixture(); }); - testWidgets('flutter context', (WidgetTester tester) async { + testWidgets('flutter context on dart:io', (WidgetTester tester) async { + if (kIsWeb) { + // widget tests don't support onPlatform config + // https://pub.dev/packages/test#platform-specific-configuration + return; + } + // These two values need to be changed inside the test, + // otherwise the Flutter test framework complains that these + // values are changed outside of a test. + debugBrightnessOverride = Brightness.dark; + debugDefaultTargetPlatformOverride = TargetPlatform.android; + final enricher = fixture.getSut( + binding: () => tester.binding, + ); + + final event = await enricher.apply(SentryEvent()); + + debugBrightnessOverride = null; + debugDefaultTargetPlatformOverride = null; + + final flutterContext = event?.contexts['flutter_context']; + expect(flutterContext, isNotNull); + expect(flutterContext, isA>()); + }, skip: !kIsWeb); + + testWidgets('flutter context on web', (WidgetTester tester) async { + if (!kIsWeb) { + // widget tests don't support onPlatform config + // https://pub.dev/packages/test#platform-specific-configuration + return; + } + // These two values need to be changed inside the test, // otherwise the Flutter test framework complains that these // values are changed outside of a test. diff --git a/flutter/test/event_processor/screenshot_event_processor_test.dart b/flutter/test/event_processor/screenshot_event_processor_test.dart index da063e776c..e0ed2eb182 100644 --- a/flutter/test/event_processor/screenshot_event_processor_test.dart +++ b/flutter/test/event_processor/screenshot_event_processor_test.dart @@ -19,11 +19,15 @@ void main() { }); Future _addScreenshotAttachment( - WidgetTester tester, FlutterRenderer renderer, bool added, - {int? expectedMaxWidthOrHeight}) async { + WidgetTester tester, + FlutterRenderer? renderer, { + required bool isWeb, + required bool added, + int? expectedMaxWidthOrHeight, + }) async { // Run with real async https://stackoverflow.com/a/54021863 await tester.runAsync(() async { - final sut = fixture.getSut(renderer); + final sut = fixture.getSut(renderer, isWeb); await tester.pumpWidget(SentryScreenshotWidget( child: Text('Catching Pokémon is a snap!', @@ -48,55 +52,54 @@ void main() { }); } - testWidgets('adds screenshot attachment with skia renderer', (tester) async { - await _addScreenshotAttachment(tester, FlutterRenderer.skia, true); + testWidgets('adds screenshot attachment dart:io', (tester) async { + await _addScreenshotAttachment(tester, null, added: true, isWeb: false); }); testWidgets('adds screenshot attachment with canvasKit renderer', (tester) async { - await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit, true); + await _addScreenshotAttachment(tester, FlutterRenderer.canvasKit, + added: true, isWeb: true); }); testWidgets('does not add screenshot attachment with html renderer', (tester) async { - await _addScreenshotAttachment(tester, FlutterRenderer.html, false); - }); - - testWidgets('does not add screenshot attachment with unknown renderer', - (tester) async { - await _addScreenshotAttachment(tester, FlutterRenderer.unknown, false); + await _addScreenshotAttachment(tester, FlutterRenderer.html, + added: false, isWeb: true); }); testWidgets('does add screenshot in correct resolution for low', (tester) async { final height = SentryScreenshotQuality.low.targetResolution()!; fixture.options.screenshotQuality = SentryScreenshotQuality.low; - await _addScreenshotAttachment(tester, FlutterRenderer.skia, true, - expectedMaxWidthOrHeight: height); + await _addScreenshotAttachment(tester, null, + added: true, isWeb: false, expectedMaxWidthOrHeight: height); }); testWidgets('does add screenshot in correct resolution for medium', (tester) async { final height = SentryScreenshotQuality.medium.targetResolution()!; fixture.options.screenshotQuality = SentryScreenshotQuality.medium; - await _addScreenshotAttachment(tester, FlutterRenderer.skia, true, - expectedMaxWidthOrHeight: height); + await _addScreenshotAttachment(tester, null, + added: true, isWeb: false, expectedMaxWidthOrHeight: height); }); testWidgets('does add screenshot in correct resolution for high', (tester) async { final widthOrHeight = SentryScreenshotQuality.high.targetResolution()!; fixture.options.screenshotQuality = SentryScreenshotQuality.high; - await _addScreenshotAttachment(tester, FlutterRenderer.skia, true, - expectedMaxWidthOrHeight: widthOrHeight); + await _addScreenshotAttachment(tester, null, + added: true, isWeb: false, expectedMaxWidthOrHeight: widthOrHeight); }); } class Fixture { SentryFlutterOptions options = SentryFlutterOptions(dsn: fakeDsn); - ScreenshotEventProcessor getSut(FlutterRenderer flutterRenderer) { + ScreenshotEventProcessor getSut( + FlutterRenderer? flutterRenderer, bool isWeb) { options.rendererWrapper = MockRendererWrapper(flutterRenderer); + options.platformChecker = MockPlatformChecker(isWebValue: isWeb); return ScreenshotEventProcessor(options); } } diff --git a/flutter/test/mocks.dart b/flutter/test/mocks.dart index fe032c9271..b2e01788c1 100644 --- a/flutter/test/mocks.dart +++ b/flutter/test/mocks.dart @@ -396,10 +396,10 @@ class MockNativeChannel implements SentryNativeBinding { class MockRendererWrapper implements RendererWrapper { MockRendererWrapper(this._renderer); - final FlutterRenderer _renderer; + final FlutterRenderer? _renderer; @override - FlutterRenderer getRenderer() { + FlutterRenderer? getRenderer() { return _renderer; } } diff --git a/flutter/test/sentry_flutter_test.dart b/flutter/test/sentry_flutter_test.dart index ef8c848348..87e4f46e84 100644 --- a/flutter/test/sentry_flutter_test.dart +++ b/flutter/test/sentry_flutter_test.dart @@ -495,7 +495,7 @@ void main() { await Sentry.close(); }); - test('installed with skia renderer', () async { + test('installed on io platforms', () async { List integrations = []; await SentryFlutter.init( @@ -505,7 +505,8 @@ void main() { integrations = options.integrations; }, appRunner: appRunner, - platformChecker: getPlatformChecker(platform: MockPlatform.iOs()), + platformChecker: + getPlatformChecker(platform: MockPlatform.iOs(), isWeb: false), rendererWrapper: MockRendererWrapper(FlutterRenderer.skia), ); @@ -528,7 +529,8 @@ void main() { integrations = options.integrations; }, appRunner: appRunner, - platformChecker: getPlatformChecker(platform: MockPlatform.iOs()), + platformChecker: + getPlatformChecker(platform: MockPlatform.iOs(), isWeb: true), rendererWrapper: MockRendererWrapper(FlutterRenderer.canvasKit), ); @@ -551,7 +553,8 @@ void main() { integrations = options.integrations; }, appRunner: appRunner, - platformChecker: getPlatformChecker(platform: MockPlatform.iOs()), + platformChecker: + getPlatformChecker(platform: MockPlatform.iOs(), isWeb: true), rendererWrapper: MockRendererWrapper(FlutterRenderer.html), ); @@ -563,29 +566,6 @@ void main() { await Sentry.close(); }, testOn: 'vm'); - - test('not installed with unknown renderer', () async { - List integrations = []; - - await SentryFlutter.init( - (options) async { - options.dsn = fakeDsn; - options.automatedTestMode = true; - integrations = options.integrations; - }, - appRunner: appRunner, - platformChecker: getPlatformChecker(platform: MockPlatform.iOs()), - rendererWrapper: MockRendererWrapper(FlutterRenderer.unknown), - ); - - expect( - integrations - .map((e) => e.runtimeType) - .contains(ScreenshotIntegration), - false); - - await Sentry.close(); - }, testOn: 'vm'); }); group('initial values', () {