Fix truncated stacktraces in unhandled errors (#2152)

* Fix stacktrace * Fix stacktrace * Update * Update changelog * Add test cases * formatting * formatting * Fix await
getsentry · Jul 17, 2024 · d0476e1 · d0476e1
1 parent dd76eef
commit d0476e1
Show file tree

Hide file tree

Showing 5 changed files with 77 additions and 10 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -30,6 +30,8 @@
 
 ### Fixes
 
+- Capture meaningful stack traces when unhandled errors have empty or missing stack traces ([#2152](https://github.com/getsentry/sentry-dart/pull/2152))
+  - This will affect grouping for unhandled errors that have empty or missing stack traces.
 - Fix sentry_drift compatibility with Drift 2.19.0 ([#2162](https://github.com/getsentry/sentry-dart/pull/2162))
 - App starts hanging for 30s ([#2140](https://github.com/getsentry/sentry-dart/pull/2140))
   - Time out for app start info retrieval has been reduced to 10s

diff --git a/flutter/lib/src/integrations/flutter_error_integration.dart b/flutter/lib/src/integrations/flutter_error_integration.dart
@@ -2,6 +2,9 @@ import 'package:flutter/foundation.dart';
 import 'package:sentry/sentry.dart';
 import '../sentry_flutter_options.dart';
 
+// ignore: implementation_imports
+import 'package:sentry/src/utils/stacktrace_utils.dart';
+
 /// Integration that capture errors on the [FlutterError.onError] handler.
 ///
 /// Remarks:
@@ -77,7 +80,8 @@ class FlutterErrorIntegration implements Integration<SentryFlutterOptions> {
         );
 
         await hub.captureEvent(event,
-            stackTrace: errorDetails.stack,
+            // ignore: invalid_use_of_internal_member
+            stackTrace: errorDetails.stack ?? getCurrentStackTrace(),
             hint:
                 Hint.withMap({TypeCheckHint.syntheticException: errorDetails}));
         // we don't call Zone.current.handleUncaughtError because we'd like

diff --git a/flutter/lib/src/integrations/on_error_integration.dart b/flutter/lib/src/integrations/on_error_integration.dart
@@ -4,6 +4,9 @@ import 'package:flutter/widgets.dart';
 import 'package:sentry/sentry.dart';
 import '../sentry_flutter_options.dart';
 
+// ignore: implementation_imports
+import 'package:sentry/src/utils/stacktrace_utils.dart';
+
 typedef ErrorCallback = bool Function(Object exception, StackTrace stackTrace);
 
 /// Integration which captures `PlatformDispatcher.onError`
@@ -74,6 +77,11 @@ class OnErrorIntegration implements Integration<SentryFlutterOptions> {
         (scope) => scope.span?.status ??= const SpanStatus.internalError(),
       );
 
+      if (stackTrace == StackTrace.empty) {
+        // ignore: invalid_use_of_internal_member
+        stackTrace = getCurrentStackTrace();
+      }
+
       // unawaited future
       hub.captureEvent(event, stackTrace: stackTrace);
 

diff --git a/flutter/test/integrations/flutter_error_integration_test.dart b/flutter/test/integrations/flutter_error_integration_test.dart
@@ -18,7 +18,8 @@ void main() {
     void _mockValues() {
       when(fixture.hub.configureScope(captureAny)).thenAnswer((_) {});
 
-      when(fixture.hub.captureEvent(captureAny, hint: anyNamed('hint')))
+      when(fixture.hub.captureEvent(captureAny,
+              hint: anyNamed('hint'), stackTrace: anyNamed('stackTrace')))
           .thenAnswer((_) => Future.value(SentryId.empty()));
 
       when(fixture.hub.options).thenReturn(fixture.options);
@@ -63,7 +64,11 @@ void main() {
       _reportError(exception: exception);
 
       final event = verify(
-        await fixture.hub.captureEvent(captureAny, hint: anyNamed('hint')),
+        await fixture.hub.captureEvent(
+          captureAny,
+          hint: anyNamed('hint'),
+          stackTrace: anyNamed('stackTrace'),
+        ),
       ).captured.first as SentryEvent;
 
       expect(event.level, SentryLevel.fatal);
@@ -95,7 +100,8 @@ void main() {
       _reportError(exception: StateError('error'), optionalDetails: details);
 
       final event = verify(
-        await fixture.hub.captureEvent(captureAny, hint: anyNamed('hint')),
+        await fixture.hub.captureEvent(captureAny,
+            hint: anyNamed('hint'), stackTrace: anyNamed('stackTrace')),
       ).captured.first as SentryEvent;
 
       expect(event.level, SentryLevel.fatal);
@@ -119,7 +125,8 @@ void main() {
       _reportError(exception: StateError('error'), optionalDetails: details);
 
       final event = verify(
-        await fixture.hub.captureEvent(captureAny, hint: anyNamed('hint')),
+        await fixture.hub.captureEvent(captureAny,
+            hint: anyNamed('hint'), stackTrace: anyNamed('stackTrace')),
       ).captured.first as SentryEvent;
 
       expect(event.level, SentryLevel.fatal);
@@ -141,7 +148,9 @@ void main() {
       _reportError(handler: defaultError);
 
       verify(
-          await fixture.hub.captureEvent(captureAny, hint: anyNamed('hint')));
+        await fixture.hub.captureEvent(captureAny,
+            hint: anyNamed('hint'), stackTrace: anyNamed('stackTrace')),
+      );
 
       expect(called, true);
     });
@@ -166,8 +175,10 @@ void main() {
 
       FlutterError.reportError(details);
 
-      verify(await fixture.hub.captureEvent(captureAny, hint: anyNamed('hint')))
-          .called(1);
+      verify(
+        await fixture.hub.captureEvent(captureAny,
+            hint: anyNamed('hint'), stackTrace: anyNamed('stackTrace')),
+      ).called(1);
 
       expect(numberOfDefaultCalls, 1);
     });
@@ -200,6 +211,26 @@ void main() {
       expect(FlutterError.onError, afterIntegrationOnError);
     });
 
+    test('captureEvent never uses an empty or null stack trace', () async {
+      final exception = StateError('error');
+      final details = FlutterErrorDetails(
+        exception: exception,
+        stack: null, // Explicitly set stack to null
+      );
+
+      _reportError(optionalDetails: details);
+
+      final captured = verify(
+        await fixture.hub.captureEvent(captureAny,
+            hint: anyNamed('hint'), stackTrace: captureAnyNamed('stackTrace')),
+      ).captured;
+
+      final stackTrace = captured[1] as StackTrace?;
+
+      expect(stackTrace, isNotNull);
+      expect(stackTrace.toString(), isNotEmpty);
+    });
+
     test('do not capture if silent error', () async {
       _reportError(silent: true);
 
@@ -211,7 +242,9 @@ void main() {
       _reportError(silent: true);
 
       verify(
-          await fixture.hub.captureEvent(captureAny, hint: anyNamed('hint')));
+        await fixture.hub.captureEvent(captureAny,
+            hint: anyNamed('hint'), stackTrace: anyNamed('stackTrace')),
+      );
     });
 
     test('adds integration', () {
@@ -255,7 +288,8 @@ void main() {
       _reportError(exception: exception);
 
       final event = verify(
-        await fixture.hub.captureEvent(captureAny, hint: anyNamed('hint')),
+        await fixture.hub.captureEvent(captureAny,
+            hint: anyNamed('hint'), stackTrace: anyNamed('stackTrace')),
       ).captured.first as SentryEvent;
 
       expect(event.level, SentryLevel.error);

diff --git a/flutter/test/integrations/on_error_integration_test.dart b/flutter/test/integrations/on_error_integration_test.dart
@@ -95,6 +95,25 @@ void main() {
       expect(throwableMechanism.mechanism.handled, false);
     });
 
+    test('captureEvent never uses an empty or null stack trace', () async {
+      final exception = StateError('error');
+      _reportError(
+        exception: exception,
+        stackTrace: StackTrace.current,
+        onErrorReturnValue: false,
+      );
+
+      final captured = verify(
+        await fixture.hub.captureEvent(captureAny,
+            hint: anyNamed('hint'), stackTrace: captureAnyNamed('stackTrace')),
+      ).captured;
+
+      final stackTrace = captured[1] as StackTrace?;
+
+      expect(stackTrace, isNotNull);
+      expect(stackTrace.toString(), isNotEmpty);
+    });
+
     test('calls default error', () async {
       var called = false;
       final defaultError = (_, __) {