Generated by 🚫 dangerJS against 5ddd2af

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.05%. Comparing base (6d5e62f) to head (5ddd2af).
Report is 2 commits behind head on main.

@@            Coverage Diff             @@
##             main    #2351      +/-   ##
==========================================
+ Coverage   85.02%   85.05%   +0.03%     
==========================================
  Files         257      257              
  Lines        9175     9190      +15     
==========================================
+ Hits         7801     7817      +16     
+ Misses       1374     1373       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Android Performance metrics 🚀

iOS Performance metrics 🚀

what about sdk crashes/errors? If a real sdk crash/error happened we wouldn't know because the frames would be removed, no?

the main reason why we re-enabled it a couple months ago was to catch sdk errors

@buenaflor We should somehow verify that we are not involuntarily breaking SDK crash detection, do you have any idea how?

don't think we can directly test it but we can do a sanity check: https://github.com/getsentry/sentry/blob/master/src/sentry/utils/sdk_crashes/sdk_crash_detection_config.py#L288C1-L302C1 if these paths are contained in one of the stacktraces we ingest it as a sdk crash/error

one edge case is if we throw an unhandled error from the sdk and for some reason flutter doesn't provide a stacktrace, we try to capture a stacktrace with StackTrace.current and then remove all sentry frames, then we wouldn't be able to capture the crash detection in that case.

@buenaflor Checked wit some flutter sdk errors. Works as expected, sentry frames are removed.

FlutterError.reportError(
  FlutterErrorDetails(exception: FlutterError.fromParts(
    <DiagnosticsNode>[
      ErrorSummary(
        'The Element for ${toStringShort()} cannot be inserted into slot "$slot" of its ancestor. ',
      ),
      ErrorDescription(
        'The ownership chain for the Element in question was:\n  ${debugGetCreatorChain(10)}',
      ),
      ErrorDescription(
        'This Element allows the creation of multiple independent render trees, which cannot '
        'be attached to an ancestor in an existing render tree. However, an ancestor RenderObject '
        'is expecting that a child will be attached.'
      ),
      ErrorHint(
        'Try moving the subtree that contains the ${toStringShort()} widget into the '
        'view property of a ViewAnchor widget or to the root of the widget tree, where '
        'it is not expected to attach its RenderObject to its ancestor.',
      ),
    ],
  )),
);

https://github.com/flutter/flutter/blob/47035a0102339c0464b1866a09cd6a25e86922d7/packages/flutter/lib/src/widgets/view.dart#L784

https://sentry-sdks.sentry.io/issues/3856107391/events/latest/?project=5428562&query=&referrer=latest-event&sort=date&statsPeriod=1h&stream_index=0

FlutterError.reportError(
  const FlutterErrorDetails(
    exception:
      'Both Router.navigate and Router.neglect have been called in this '
      'build cycle, and the Router cannot decide whether to report the '
      'route information. Please make sure only one of them is called '
      'within the same build cycle.',
  ),
);

https://github.com/flutter/flutter/blob/47035a0102339c0464b1866a09cd6a25e86922d7/packages/flutter/lib/src/widgets/router.dart#L674

https://sentry-sdks.sentry.io/issues/6008675625/?project=5428562&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D&referrer=issue-stream&statsPeriod=1h&stream_index=1

FlutterError.reportError(
      FlutterErrorDetails(
        exception: FlutterError('A $runtimeType overflowed by $overflowText.'),
        library: 'rendering library',
        context: ErrorDescription('during layout'),
        informationCollector: () => <DiagnosticsNode>[
          // debugCreator should only be set in DebugMode, but we want the
          // treeshaker to know that.
          if (kDebugMode && debugCreator != null)
            DiagnosticsDebugCreator(debugCreator!),
          ...overflowHints!,
          describeForError('The specific $runtimeType in question is'),
          // TODO(jacobr): this line is ascii art that it would be nice to
          // handle a little more generically in GUI debugging clients in the
          // future.
          DiagnosticsNode.message('◢◤' * (FlutterError.wrapWidth ~/ 2), allowWrap: false),
        ],
      ),
    );

https://github.com/flutter/flutter/blob/47035a0102339c0464b1866a09cd6a25e86922d7/packages/flutter/lib/src/rendering/debug_overflow_indicator.dart#L246

https://sentry-sdks.sentry.io/issues/3856107391/events/latest/?project=5428562&query=is%3Aunresolved%20issue.priority%3A%5Bhigh%2C%20medium%5D&referrer=latest-event&sort=date&statsPeriod=1h&stream_index=0

one edge case is if we throw an unhandled error from the sdk and for some reason flutter doesn't provide a stacktrace, we try to capture a stacktrace with StackTrace.current and then remove all sentry frames, then we wouldn't be able to capture the crash detection in that case.

We could only avoid this if we only remove the stacktraces if the errors originiated in our error integrations, and not for any error where we fall back to StackTrace.current.

@denrase if we throw an unhandled error within the SDK it is forwarded to our FlutterError.onError integration and I'm not sure if there are cases where flutter does not provide a stacktrace, I'll also check out if there are such cases in our SDK

And for the rest where we fall back to Stacktrace.current, those happen if a user tries to do captureException or captureEvent without a stacktrace or captureMessage e.g

// no stacktrace attached so we fallback to Stacktrace.current in stacktrace factory
Sentry.captureException(exception)
Sentry.captureMessage('msg')

and that's also where the sentry frames are added that are misleading because it captures all sdk function calls until Stacktrace.current

@buenaflor Threw an exception in sentry. It does get caught in on_error_integration.dart and has a stack trace. Since the callback of this integration always has a non-null stack trace, it should at least always be there, but we don't knwo if this could also be StackTrace.empty.

The case where users call Sentry.captureException and Sentry.captureMessage, we remove sentry frames as well, so this case is covered.

@buenaflor We also catch errors thrown in the SDK in various parts of the SDK and call the logger with them, so they will never reach our backend anyway AFAIK.

try {
  sentryId = await item.client.captureFeedback(
    feedback,
    hint: hint,
    scope: scope,
  );
} catch (exception, stacktrace) {
  _options.logger(
    SentryLevel.error,
    'Error while capturing feedback',
    exception: exception,
    stackTrace: stacktrace,
  );
}

yeah, tbh I think this is fine.

I'll do some final tests and then we can wrap it up if it looks good

my only concern is: there will be users complaining that they now have an empty stacktrace for unhandled errors where FlutterError.onError doesn't provide one OR the stacktrace provided is not very in-depth because we synthetically create it. I somehow want to make it clear that this is intended behaviour because Flutter doesn't always provide a stacktrace (most users don't know this) and we are doing best effort to provide useful info. does it make sense to add some kind of context to the event to clarify this? or something similar.

imo that would be useful and helps the user understand why this is happening and we don't have to explain it to them all the time

cc @kahest maybe you have insight or an idea how we could surface this to the user clearly

cc @kahest maybe you have insight or an idea how we could surface this to the user clearly

We can show something directly on the issue though not sure what's involved to do that. But maybe calling this out in release notes and docs is enough?

Let's make it clear in the changelog here and update the sentry-docs in flutter/troubleshooting

I'll take care of that since I'm currently updating some docs anyway