Merge branch 'main' into feat/drift-apm

getsentry · Nov 14, 2023 · 6fb54e5 · 6fb54e5
2 parents 13e67e7 + 48adddf
commit 6fb54e5
Show file tree

Hide file tree

Showing 15 changed files with 184 additions and 180 deletions.
diff --git a/.github/workflows/flutter.yml b/.github/workflows/flutter.yml
@@ -201,6 +201,6 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       # To recreate baseline run: detekt -i flutter/android,flutter/example/android -b flutter/config/detekt-bl.xml -cb
-      - uses: natiginfo/action-detekt-all@e01de6ff0eef7c24131e8a133bf598cfac6ceeab # pin@1.21.0
+      - uses: natiginfo/action-detekt-all@68eb02dd9f2c2686d5026f5957756064424261a9 # pin@1.23.3
         with:
           args: -i flutter/android,flutter/example/android --baseline flutter/config/detekt-bl.xml --jvm-target 1.8 --build-upon-default-config --all-rules
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,11 +4,13 @@
 
 ### 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
 
 - Add APM integration for Drift ([#1709](https://github.com/getsentry/sentry-dart/pull/1709))
+- StackTraces in `PlatformException.message` will get nicely formatted too when present ([#1716](https://github.com/getsentry/sentry-dart/pull/1716))
 - Breadcrumbs for database operations ([#1656](https://github.com/getsentry/sentry-dart/pull/1656))
 - Add `attachScreenshotOnlyWhenResumed` to options ([#1700](https://github.com/getsentry/sentry-dart/pull/1700))
 

diff --git a/flutter/example/android/app/src/main/kotlin/io/sentry/samples/flutter/MainActivity.kt b/flutter/example/android/app/src/main/kotlin/io/sentry/samples/flutter/MainActivity.kt
@@ -11,37 +11,33 @@ class MainActivity : FlutterActivity() {
 
   override fun configureFlutterEngine(flutterEngine: FlutterEngine) {
     super.configureFlutterEngine(flutterEngine)
-    MethodChannel(flutterEngine.dartExecutor.binaryMessenger, _channel).setMethodCallHandler {
-        call, result ->
+    MethodChannel(
+      flutterEngine.dartExecutor.binaryMessenger,
+      _channel,
+    ).setMethodCallHandler { call, result ->
       // Note: this method is invoked on the main thread.
       when (call.method) {
-        "throw" -> {
+        "throw" ->
           thread(isDaemon = true) {
             throw Exception("Catch this java exception thrown from Kotlin thread!")
           }
-        }
-        "anr" -> {
-          Thread.sleep(6_000)
-        }
-        "capture" -> {
+
+        "anr" -> Thread.sleep(6_000)
+
+        "capture" ->
           try {
             throw RuntimeException("Catch this java exception!")
           } catch (e: Exception) {
             Sentry.captureException(e)
           }
-        }
-        "crash" -> {
-          crash()
-        }
-        "cpp_capture_message" -> {
-          message()
-        }
-        "platform_exception" -> {
-          throw RuntimeException("Catch this platform exception!")
-        }
-        else -> {
-          result.notImplemented()
-        }
+
+        "crash" -> crash()
+
+        "cpp_capture_message" -> message()
+
+        "platform_exception" -> throw RuntimeException("Catch this platform exception!")
+
+        else -> result.notImplemented()
       }
       result.success("")
     }

diff --git a/flutter/lib/src/event_processor/android_platform_exception_event_processor.dart b/flutter/lib/src/event_processor/android_platform_exception_event_processor.dart
@@ -15,9 +15,6 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
 
   final SentryFlutterOptions _options;
 
-  // Because of obfuscation, we need to dynamically get the name
-  static final _platformExceptionType = (PlatformException).toString();
-
   @override
   Future<SentryEvent?> apply(SentryEvent event, {Hint? hint}) async {
     if (event is SentryTransaction) {
@@ -29,19 +26,23 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
       return event;
     }
 
-    final nativeStackTrace = plaformException.stacktrace;
-    if (nativeStackTrace == null) {
-      return event;
-    }
-
     try {
       // PackageInfo has an internal cache, so no need to do it ourselves.
       final packageInfo = await PackageInfo.fromPlatform();
+
+      final nativeStackTrace =
+          _tryParse(plaformException.stacktrace, packageInfo.packageName);
+      final messageStackTrace =
+          _tryParse(plaformException.message, packageInfo.packageName);
+
+      if (nativeStackTrace == null && messageStackTrace == null) {
+        return event;
+      }
+
       return _processPlatformException(
         event,
-        plaformException,
         nativeStackTrace,
-        packageInfo.packageName,
+        messageStackTrace,
       );
     } catch (e, stackTrace) {
       _options.logger(
@@ -55,25 +56,35 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
     }
   }
 
-  SentryEvent _processPlatformException(
-    SentryEvent event,
-    PlatformException exception,
-    String nativeStackTrace,
+  List<MapEntry<SentryException, SentryThread>>? _tryParse(
+    String? potentialStackTrace,
     String packageName,
   ) {
-    final jvmException =
-        _JvmExceptionFactory(packageName).fromJvmStackTrace(nativeStackTrace);
+    if (potentialStackTrace == null) {
+      return null;
+    }
 
-    final exceptions = _removePlatformExceptionStackTraceFromValue(
-      event.exceptions,
-      exception,
-    );
+    return _JvmExceptionFactory(packageName)
+        .fromJvmStackTrace(potentialStackTrace);
+  }
 
+  SentryEvent _processPlatformException(
+    SentryEvent event,
+    List<MapEntry<SentryException, SentryThread>>? nativeStackTrace,
+    List<MapEntry<SentryException, SentryThread>>? messageStackTrace,
+  ) {
     final threads = _markDartThreadsAsNonCrashed(event.threads);
 
-    final jvmExceptions = jvmException.map((e) => e.key);
+    final jvmExceptions = [
+      ...?nativeStackTrace?.map((e) => e.key),
+      ...?messageStackTrace?.map((e) => e.key)
+    ];
+
+    var jvmThreads = [
+      ...?nativeStackTrace?.map((e) => e.value),
+      ...?messageStackTrace?.map((e) => e.value),
+    ];
 
-    var jvmThreads = jvmException.map((e) => e.value).toList(growable: false);
     if (jvmThreads.isNotEmpty) {
       // filter potential duplicated threads
       final first = jvmThreads.first;
@@ -84,13 +95,16 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
       jvmThreads.add(first);
     }
 
-    return event.copyWith(exceptions: [
-      ...?exceptions,
-      ...jvmExceptions,
-    ], threads: [
-      ...?threads,
-      if (_options.attachThreads) ...jvmThreads,
-    ]);
+    return event.copyWith(
+      exceptions: [
+        ...?event.exceptions,
+        ...jvmExceptions,
+      ],
+      threads: [
+        ...?threads,
+        if (_options.attachThreads) ...jvmThreads,
+      ],
+    );
   }
 
   /// If the crash originated on Android, the Dart side didn't crash.
@@ -99,60 +113,16 @@ class AndroidPlatformExceptionEventProcessor implements EventProcessor {
     List<SentryThread>? threads,
   ) {
     return threads
-        ?.map((e) => e.copyWith(
-              crashed: false,
-              // Isolate is safe to use directly,
-              // because Android is only run in the dart:io context.
-              current: e.name == Isolate.current.debugName,
-            ))
+        ?.map(
+          (e) => e.copyWith(
+            crashed: false,
+            // Isolate is safe to use directly,
+            // because Android is only run in the dart:io context.
+            current: e.name == Isolate.current.debugName,
+          ),
+        )
         .toList(growable: false);
   }
-
-  /// Remove the StackTrace from [PlatformException] so the message on Sentry
-  /// looks much better.
-  List<SentryException>? _removePlatformExceptionStackTraceFromValue(
-    List<SentryException>? exceptions,
-    PlatformException platformException,
-  ) {
-    if (exceptions == null || exceptions.isEmpty) {
-      return null;
-    }
-    final exceptionCopy = List<SentryException>.from(exceptions);
-
-    final sentryExceptions = exceptionCopy
-        .where((element) => element.type == _platformExceptionType);
-    if (sentryExceptions.isEmpty) {
-      return null;
-    }
-    var sentryException = sentryExceptions.first;
-
-    final exceptionIndex = exceptionCopy.indexOf(sentryException);
-    exceptionCopy.remove(sentryException);
-
-    // Remove stacktrace, so that the PlatformException value doesn't
-    // include the chained exception.
-    // PlatformException.stackTrace is an empty string so that
-    // PlatformException.toString() results in
-    // `PlatformException(error, Exception Message, null, )`
-    // instead of
-    // `PlatformException(error, Exception Message, null, null)`.
-    // While `null` for `PlatformException.stackTrace` is technically correct
-    // it's semantically wrong.
-    platformException = PlatformException(
-      code: platformException.code,
-      details: platformException.details,
-      message: platformException.message,
-      stacktrace: '',
-    );
-
-    sentryException = sentryException.copyWith(
-      value: platformException.toString(),
-    );
-
-    exceptionCopy.insert(exceptionIndex, sentryException);
-
-    return exceptionCopy;
-  }
 }
 
 class _JvmExceptionFactory {
@@ -161,7 +131,8 @@ class _JvmExceptionFactory {
   final String nativePackageName;
 
   List<MapEntry<SentryException, SentryThread>> fromJvmStackTrace(
-      String exceptionAsString) {
+    String exceptionAsString,
+  ) {
     final jvmException = JvmException.parse(exceptionAsString);
     final jvmExceptions = <JvmException>[
       jvmException,

diff --git 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 <String, String>{
       '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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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());
     }