Remove sentry frames if SDK falls back to current stack trace

CHANGELOG.md

@@ -2,6 +2,15 @@
 
 ## Unreleased
 
+### Enhancements
+
+- Remove `sentry` frames if SDK falls back to current stack trace ([#2351](https://github.com/getsentry/sentry-dart/pull/2351))  
+  - Flutter doesn't always provide stack traces for unhandled errors - this is normal Flutter behavior
+  - When no stack trace is provided (in Flutter errors, `captureException`, or `captureMessage`):
+    - SDK creates a synthetic trace using `StackTrace.current`
+    - Internal SDK frames are removed to reduce noise
+  - Original stack traces (when provided) are left unchanged
+
 ### Features
 
 - Improve frame tracking accuracy ([#2372](https://github.com/getsentry/sentry-dart/pull/2372))

dart/lib/src/sentry_client.dart

@@ -2,6 +2,7 @@ import 'dart:async';
 import 'dart:math';
 
 import 'package:meta/meta.dart';
+import 'type_check_hint.dart';
 
 import 'client_reports/client_report_recorder.dart';
 import 'client_reports/discard_reason.dart';
@@ -126,10 +127,11 @@ class SentryClient {
       return _emptySentryId;
     }
 
-    SentryEvent? preparedEvent = _prepareEvent(event, stackTrace: stackTrace);
-
     hint ??= Hint();
 
+    SentryEvent? preparedEvent =
+        _prepareEvent(event, hint, stackTrace: stackTrace);
+
     if (scope != null) {
       preparedEvent = await scope.applyToEvent(preparedEvent, hint);
     } else {
@@ -211,7 +213,8 @@ class SentryClient {
     return isMatchingRegexPattern(message, _options.ignoreErrors);
   }
 
-  SentryEvent _prepareEvent(SentryEvent event, {dynamic stackTrace}) {
+  SentryEvent _prepareEvent(SentryEvent event, Hint hint,
+      {dynamic stackTrace}) {
     event = event.copyWith(
       serverName: event.serverName ?? _options.serverName,
       dist: event.dist ?? _options.dist,
@@ -248,6 +251,7 @@ class SentryClient {
         var sentryException = _exceptionFactory.getSentryException(
           extractedException.exception,
           stackTrace: extractedException.stackTrace,
+          removeSentryFrames: hint.get(TypeCheckHint.currentStackTrace),
         );
 
         SentryThread? sentryThread;
@@ -283,8 +287,14 @@ class SentryClient {
     // therefore add it to the threads.
     // https://develop.sentry.dev/sdk/event-payloads/stacktrace/
     if (stackTrace != null || _options.attachStacktrace) {
-      stackTrace ??= getCurrentStackTrace();
-      final sentryStackTrace = _stackTraceFactory.parse(stackTrace);
+      if (stackTrace == null || stackTrace == StackTrace.empty) {
+        stackTrace = getCurrentStackTrace();
+        hint.addAll({TypeCheckHint.currentStackTrace: true});
+      }
+      final sentryStackTrace = _stackTraceFactory.parse(
+        stackTrace,
+        removeSentryFrames: hint.get(TypeCheckHint.currentStackTrace),
+      );
       if (sentryStackTrace.frames.isNotEmpty) {
         event = event.copyWith(threads: [
           ...?event.threads,
@@ -356,11 +366,11 @@ class SentryClient {
     Scope? scope,
     SentryTraceContextHeader? traceContext,
   }) async {
-    SentryTransaction? preparedTransaction =
-        _prepareEvent(transaction) as SentryTransaction;
-
     final hint = Hint();
 
+    SentryTransaction? preparedTransaction =
+        _prepareEvent(transaction, hint) as SentryTransaction;
+
     if (scope != null) {
       preparedTransaction = await scope.applyToEvent(preparedTransaction, hint)
           as SentryTransaction?;

dart/lib/src/sentry_exception_factory.dart

@@ -18,6 +18,7 @@ class SentryExceptionFactory {
   SentryException getSentryException(
     dynamic exception, {
     dynamic stackTrace,
+    bool? removeSentryFrames,
   }) {
     var throwable = exception;
     Mechanism? mechanism;
@@ -38,17 +39,20 @@ class SentryExceptionFactory {
     // throwable.stackTrace is null if its an exception that was never thrown
     // hence we check again if stackTrace is null and if not, read the current stack trace
     // but only if attachStacktrace is enabled
+
     if (_options.attachStacktrace) {
       if (stackTrace == null || stackTrace == StackTrace.empty) {
         snapshot = true;
         stackTrace = getCurrentStackTrace();
+        removeSentryFrames = true;
       }
     }
 
     SentryStackTrace? sentryStackTrace;
     if (stackTrace != null) {
-      sentryStackTrace =
-          _stacktraceFactory.parse(stackTrace).copyWith(snapshot: snapshot);
+      sentryStackTrace = _stacktraceFactory
+          .parse(stackTrace, removeSentryFrames: removeSentryFrames)
+          .copyWith(snapshot: snapshot);
       if (sentryStackTrace.frames.isEmpty) {
         sentryStackTrace = null;
       }

dart/lib/src/sentry_options.dart

@@ -590,13 +590,13 @@ class SentryOptions {
   }
 
   /// Adds an inAppExclude
-  void addInAppExclude(String inApp) {
-    _inAppExcludes.add(inApp);
+  void addInAppExclude(String inAppInclude) {
+    _inAppExcludes.add(inAppInclude);
   }
 
   /// Adds an inAppIncludes
-  void addInAppInclude(String inApp) {
-    _inAppIncludes.add(inApp);
+  void addInAppInclude(String inAppExclude) {
+    _inAppIncludes.add(inAppExclude);
   }
 
   /// Returns if tracing should be enabled. If tracing is disabled, starting transactions returns

dart/lib/src/sentry_stack_trace_factory.dart

@@ -24,19 +24,26 @@ class SentryStackTraceFactory {
     return parse(stackTrace).frames;
   }
 
-  SentryStackTrace parse(dynamic stackTrace) {
+  SentryStackTrace parse(dynamic stackTrace, {bool? removeSentryFrames}) {
     final parsed = _parseStackTrace(stackTrace);
     final frames = <SentryStackFrame>[];
     var onlyAsyncGap = true;
 
     for (var t = 0; t < parsed.traces.length; t += 1) {
       final trace = parsed.traces[t];
 
-      // NOTE: We want to keep the Sentry frames for crash detection
+      // NOTE: We want to keep the Sentry frames for SDK crash detection
       // this does not affect grouping since they're not marked as inApp
+      // only exception if there was no stack trace, we remove them
       for (final frame in trace.frames) {
-        final stackTraceFrame = encodeStackTraceFrame(frame);
+        var stackTraceFrame = encodeStackTraceFrame(frame);
+
         if (stackTraceFrame != null) {
+          if (removeSentryFrames == true &&
+              (stackTraceFrame.package == 'sentry' ||
+                  stackTraceFrame.package == 'sentry_flutter')) {
+            continue;
+          }
           frames.add(stackTraceFrame);
           onlyAsyncGap = false;
         }
@@ -100,7 +107,7 @@ class SentryStackTraceFactory {
     final member = frame.member;
 
     if (frame is UnparsedFrame && member != null) {
-      // if --split-debug-info is enabled, thats what we see:
+      // if --split-debug-info is enabled, that's what we see:
       //     #00 abs 000000723d6346d7 _kDartIsolateSnapshotInstructions+0x1e26d7
 
       // we are only interested on the #01, 02... items which contains the 'abs' addresses.

dart/lib/src/type_check_hint.dart

@@ -16,4 +16,7 @@ class TypeCheckHint {
 
   /// Widget that was tapped in `sentry_flutter/SentryUserInteractionWidget`
   static const widget = 'widget';
+
+  /// Used to indicate that the SDK added a synthetic current stack trace.
+  static const currentStackTrace = 'currentStackTrace';
 }

dart/test/sentry_client_test.dart

@@ -521,6 +521,38 @@ void main() {
         true,
       );
     });
+
+    test('should remove sentry frames if null stackStrace', () async {
+      final throwable = Object();
+
+      final client = fixture.getSut(attachStacktrace: true);
+      await client.captureException(throwable, stackTrace: null);
+
+      final capturedEnvelope = (fixture.transport).envelopes.first;
+      final capturedEvent = await eventFromEnvelope(capturedEnvelope);
+
+      final sentryFramesCount = capturedEvent.exceptions?[0].stackTrace!.frames
+          .where((frame) => frame.package == 'sentry')
+          .length;
+
+      expect(sentryFramesCount, 0);
+    });
+
+    test('should remove sentry frames if empty stackStrace', () async {
+      final throwable = Object();
+
+      final client = fixture.getSut(attachStacktrace: true);
+      await client.captureException(throwable, stackTrace: StackTrace.empty);
+
+      final capturedEnvelope = (fixture.transport).envelopes.first;
+      final capturedEvent = await eventFromEnvelope(capturedEnvelope);
+
+      final sentryFramesCount = capturedEvent.exceptions?[0].stackTrace!.frames
+          .where((frame) => frame.package == 'sentry')
+          .length;
+
+      expect(sentryFramesCount, 0);
+    });
   });
 
   group('SentryClient captures transaction', () {
@@ -2069,10 +2101,62 @@ void main() {
 
       final capturedEnvelope = (fixture.transport).envelopes.first;
       final attachmentItem = capturedEnvelope.items.firstWhereOrNull(
-          (element) => element.header.type == SentryItemType.attachment);
-
+        (element) => element.header.type == SentryItemType.attachment,
+      );
       expect(attachmentItem, isNull);
     });
+
+    test(
+        'null stack trace marked in hint & sentry frames removed from thread stackTrace',
+        () async {
+      final beforeSendCallback = (SentryEvent event, Hint hint) {
+        expect(hint.get(TypeCheckHint.currentStackTrace), isTrue);
+        return event;
+      };
+      final client = fixture.getSut(
+          beforeSend: beforeSendCallback, attachStacktrace: true);
+      await client.captureEvent(fakeEvent);
+
+      final capturedEnvelope = (fixture.transport).envelopes.first;
+      final capturedEvent = await eventFromEnvelope(capturedEnvelope);
+
+      final sentryFramesCount = capturedEvent.threads?[0].stacktrace!.frames
+          .where((frame) => frame.package == 'sentry')
+          .length;
+
+      expect(sentryFramesCount, 0);
+    });
+
+    test(
+        'empty stack trace marked in hint & sentry frames removed from thread stackTrace',
+        () async {
+      final beforeSendCallback = (SentryEvent event, Hint hint) {
+        expect(hint.get(TypeCheckHint.currentStackTrace), isTrue);
+        return event;
+      };
+      final client = fixture.getSut(
+          beforeSend: beforeSendCallback, attachStacktrace: true);
+      await client.captureEvent(fakeEvent, stackTrace: StackTrace.empty);
+
+      final capturedEnvelope = (fixture.transport).envelopes.first;
+      final capturedEvent = await eventFromEnvelope(capturedEnvelope);
+
+      final sentryFramesCount = capturedEvent.threads?[0].stacktrace!.frames
+          .where((frame) => frame.package == 'sentry')
+          .length;
+
+      expect(sentryFramesCount, 0);
+    });
+
+    test('non-null stack trace not marked in hint', () async {
+      final beforeSendCallback = (SentryEvent event, Hint hint) {
+        expect(hint.get(TypeCheckHint.currentStackTrace), isNull);
+        return event;
+      };
+      final client = fixture.getSut(
+          beforeSend: beforeSendCallback, attachStacktrace: true);
+      await client.captureEvent(fakeEvent, stackTrace: StackTrace.current);
+    });
   });
 
   group('Capture metrics', () {

dart/test/sentry_exception_factory_test.dart

@@ -208,7 +208,8 @@ void main() {
     final sentryException =
         fixture.getSut(attachStacktrace: false).getSentryException(Object());
 
-    expect(sentryException.stackTrace!.snapshot, true);
+    // stackTrace is null anyway when not present and attachStacktrace false
+    expect(sentryException.stackTrace?.snapshot, isNull);
   });
 
   test('sets stacktrace build id and image address', () {
@@ -226,9 +227,26 @@ void main() {
         .getSut(attachStacktrace: false)
         .getSentryException(Object(), stackTrace: null);
 
+    // stackTrace is null anyway with null stack trace and attachStacktrace false
+    final sentryStackTrace = sentryException.stackTrace;
+    expect(sentryStackTrace?.baseAddr, isNull);
+    expect(sentryStackTrace?.buildId, isNull);
+  });
+
+  test('remove sentry frames', () {
+    final sentryException =
+        fixture.getSut(attachStacktrace: false).getSentryException(
+              SentryStackTraceError(),
+              stackTrace: SentryStackTrace(),
+              removeSentryFrames: true,
+            );
+
     final sentryStackTrace = sentryException.stackTrace!;
     expect(sentryStackTrace.baseAddr, isNull);
-    expect(sentryStackTrace.buildId, isNull);
+
+    expect(sentryStackTrace.frames.length, 17);
+    expect(sentryStackTrace.frames[16].package, 'sentry_flutter_example');
+    expect(sentryStackTrace.frames[15].package, 'flutter');
   });
 }
 
@@ -288,11 +306,50 @@ isolate_instructions: 7526344980, vm_instructions: 752633f000
   }
 }
 
+class SentryStackTraceError extends Error {
+  var prefix = "Unknown error without own stacktrace";
+
+  @override
+  String toString() {
+    return '''
+$prefix
+
+${SentryStackTrace()}''';
+  }
+}
+
+class SentryStackTrace extends StackTrace {
+  @override
+  String toString() {
+    return '''
+      #0      getCurrentStackTrace (package:sentry/src/utils/stacktrace_utils.dart:10:49)
+#1      OnErrorIntegration.call.<anonymous closure> (package:sentry_flutter/src/integrations/on_error_integration.dart:82:22)
+#2      MainScaffold.build.<anonymous closure> (package:sentry_flutter_example/main.dart:349:23)
+#3      _InkResponseState.handleTap (package:flutter/src/material/ink_well.dart:1170:21)
+#4      GestureRecognizer.invokeCallback (package:flutter/src/gestures/recognizer.dart:351:24)
+#5      TapGestureRecognizer.handleTapUp (package:flutter/src/gestures/tap.dart:656:11)
+#6      BaseTapGestureRecognizer._checkUp (package:flutter/src/gestures/tap.dart:313:5)
+#7      BaseTapGestureRecognizer.acceptGesture (package:flutter/src/gestures/tap.dart:283:7)
+#8      GestureArenaManager.sweep (package:flutter/src/gestures/arena.dart:169:27)
+#9      GestureBinding.handleEvent (package:flutter/src/gestures/binding.dart:505:20)
+#10     GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:481:22)
+#11     RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:450:11)
+#12     GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:426:7)
+#13     GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:389:5)
+#14     GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:336:7)
+#15     GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:305:9)
+#16     _invoke1 (dart:ui/hooks.dart:328:13)
+#17     PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:442:7)
+#18     _dispatchPointerDataPacket (dart:ui/hooks.dart:262:31)
+      ''';
+  }
+}
+
 class Fixture {
   final options = defaultTestOptions();
 
   SentryExceptionFactory getSut({bool attachStacktrace = true}) {
-    options.attachStacktrace = true;
+    options.attachStacktrace = attachStacktrace;
     return SentryExceptionFactory(options);
   }
 }