Remove sentry frames if SDK falls back to current stack trace

CHANGELOG.md

@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Enhancements
+
+- Remove `sentry` frames if SDK falls back to current stack trace ([#2351](https://github.com/getsentry/sentry-dart/pull/2351))
+
 ## 8.10.0-beta.2
 
 ### Fixes

dart/lib/src/sentry_client.dart

@@ -2,6 +2,7 @@ import 'dart:async';
 import 'dart:math';
 
 import 'package:meta/meta.dart';
+import 'package:sentry/src/type_check_hint.dart';
 
 import 'client_reports/client_report_recorder.dart';
 import 'client_reports/discard_reason.dart';
@@ -123,10 +124,10 @@ 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 {
@@ -208,7 +209,7 @@ 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,
@@ -245,6 +246,7 @@ class SentryClient {
         var sentryException = _exceptionFactory.getSentryException(
           extractedException.exception,
           stackTrace: extractedException.stackTrace,
+          removeSentryFrames: hint.get(TypeCheckHint.currentStackTrace),
         );
 
         SentryThread? sentryThread;
@@ -280,8 +282,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,
@@ -353,11 +361,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,21 @@ 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);
+          _stacktraceFactory
+              .parse(stackTrace, removeSentryFrames: removeSentryFrames)
+              .copyWith(snapshot: snapshot);
       if (sentryStackTrace.frames.isEmpty) {
         sentryStackTrace = null;
       }

dart/lib/src/sentry_options.dart

@@ -564,13 +564,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,24 @@ 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 +105,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

@@ -522,6 +522,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', () {
@@ -2169,10 +2201,55 @@ 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

@@ -230,6 +230,23 @@ void main() {
     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.frames.length, 17);
+    expect(sentryStackTrace.frames[16].package, 'sentry_flutter_example');
+    expect(sentryStackTrace.frames[15].package, 'flutter');
+  });
 }
 
 class CustomError extends Error {}
@@ -288,11 +305,51 @@ 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);
   }
 }