[dio] Improve fingerprinting

[kuhnroyal]

I tested the new sentry_dio package a little bit.

My issue is that due to how DIO throws DioErrors, the stacktrace is always the same (that's a problem in itself and probably needs to be improved) - this results in all captured errors having the same fingerprint.

Additionally a DioError can wrap a cause error which may be the real reason why the request failed, this currently does not get reported.

@ueman What are you thoughts on this?

[ueman]

I believe we can create a custom event processor which edits DioErrors to use chained exceptions, so we report both the DioError and the cause as seperate exceptions with their respective stacktrace in a single event. @marandaneto does that sound like a good idea?

Dio itself has also this line of code which may lead to the problem the inner cause also has a wrong stacktrace. I was about to raise a PR for that.

[kuhnroyal]

Dio itself has also this line of code which may lead to the problem the inner cause also has a wrong stacktrace. I was about to raise a PR for that.

Yea, I know. I am looking into it as well, feel free to open the ticket.

[marandaneto]

@kuhnroyal thanks for raising, can you link me to 2 issues that you think should not be grouped together? so I can analyze the stack trace.

[marandaneto]

I believe we can create a custom event processor which edits DioErrors to use chained exceptions, so we report both the DioError and the cause as seperate exceptions with their respective stacktrace in a single event. @marandaneto does that sound like a good idea?

I think it depends on what the cause points, if it's the caller stack trace, that would make sense, using "chained" exceptions and reporting both would not be wrong either, we do that on Java.
I'd need to run and check the stack trace of the cause and error, will check late this week.

Dio itself has also this line of code which may lead to the problem the inner cause also has a wrong stacktrace. I was about to raise a PR for that.

[kuhnroyal]

I only have samples in a Sentry instance on premise.

DioError [DioErrorType.response]: Http status error [404]
Source stack:
#0      DioMixin.fetch (package:dio/src/dio_mixin.dart:473:35)
#1      DioMixin.request (package:dio/src/dio_mixin.dart:468:12)
#2      DioMixin.get (package:dio/src/dio_mixin.dart:55:12)
#3      _DebugSettingsState._testApiException.<anonymous closure> (package:foo_app/ui/settings/debug_settings.dart:230:19)
#4      _DebugSettingsState._testApiException.<anonymous closure> (package:foo_app/ui/settings/debug_settings.dart:228:14)
#5      _InkResponseState._handleTap (package:flutter/src/material/ink_well.dart:989:21)
#6      GestureRecognizer.invokeCallback (package:flutter/src/gestures/recognizer.dart:198:24)
#7      TapGestureRecognizer.handleTapUp (package:flutter/src/gestures/tap.dart:608:11)
#8      BaseTapGestureRecognizer._checkUp (package:flutter/src/gestures/tap.dart:296:5)
#9      BaseTapGestureRecognizer.acceptGesture (package:flutter/src/gestures/tap.dart:267:7)
#10     GestureArenaManager.sweep (package:flutter/src/gestures/arena.dart:157:27)
#11     GestureBinding.handleEvent (package:flutter/src/gestures/binding.dart:443:20)
#12     GestureBinding.dispatchEvent (package:flutter/src/gestures/binding.dart:419:22)
#13     RendererBinding.dispatchEvent (package:flutter/src/rendering/binding.dart:322:11)
#14     GestureBinding._handlePointerEventImmediately (package:flutter/src/gestures/binding.dart:374:7)
#15     GestureBinding.handlePointerEvent (package:flutter/src/gestures/binding.dart:338:5)
#16     GestureBinding._flushPointerEventQueue (package:flutter/src/gestures/binding.dart:296:7)
#17     GestureBinding._handlePointerDataPacket (package:flutter/src/gestures/binding.dart:279:7)
#18     _rootRunUnary (dart:async/zone.dart:1442:13)
#19     _CustomZone.runUnary (dart:async/zone.dart:1335:19)
#20     _CustomZone.runUnaryGuarded (dart:async/zone.dart:1244:7)
#21     _invoke1 (dart:ui/hooks.dart:170:10)
#22     PlatformDispatcher._dispatchPointerDataPacket (dart:ui/platform_dispatcher.dart:331:7)
#23     _dispatchPointerDataPacket (dart:ui/hooks.dart:94:31)

So basically the stacktrace is not that bad, the source line (3/4) are in there. These lines are different for multiple events but still get grouped. The first 3 lines are are always identical.

[marandaneto]

@kuhnroyal There's the concept of inApp or not for frames, only frames that are inApp should be considered for grouping, check on the UI the grouping strategy, that should be the case already, we need to understand why they grouped together if they have different frames.

This is the Issues detail page.

[kuhnroyal]

Yea ok, By-in-app is off for me.

[kuhnroyal]

I am using this in the config:

    ..considerInAppFramesByDefault = false
    ..addInAppInclude('foo_app')
   // more includes

[marandaneto]

I am using this in the config:

    ..considerInAppFramesByDefault = false
    ..addInAppInclude('foo_app')
   // more includes

also try to ..addInAppExclude(...) for what you don't need to and the SDK considers inApp automatically, if it's the case.

[ueman]

If you set considerInAppFramesByDefault = false and add an in-app-include, only the in-app-includes are considered to be in app. Everything else is considered in-app-exclude.

[marandaneto]

The problem might be that the stack trace does not contain the caller of _testApiException, so every exception is the same, maybe using the cause contains the caller and would not group from different callers.
You can also force grouping events by using equal (or different) fingerprints, https://docs.sentry.io/platforms/dart/usage/sdk-fingerprinting/

[kuhnroyal]

If you set considerInAppFramesByDefault = false and add an in-app-include, only the in-app-includes are considered to be in app. Everything else is considered in-app-exclude.

Yea and this works for other stacktraces/events.

The problem might be that the stack trace does not contain the caller of _testApiException, so every exception is the same, maybe using the cause contains the caller and would not group from different callers.

The cause likely doesn't contain the caller here. The causes would mostly be system level exceptions/HTTP/Socket etc.
The 'caller' for _testApiException is just a button press, so it is the TapGestureRecognizer which is not inApp.

I tried all all possible combinations of include/exclude/considerInAppFramesByDefault. Can't get it to have a unique fingerprint.

[kuhnroyal]

I think there are 2 different stacktraces in play here:

The stacktrace from #715 (comment) and the one used for fingerprinting are not the same.

[marandaneto]

@kuhnroyal sounds like the error captured by dio then do not contain the caller and that makes it difficult to not group together when there's no different information.

Are you using the captureFailedRequests feature or capturing them by hand? if so, try to disable it and handle and capture the exception yourself using the Sentry.captureException(...) API, maybe if you handle yourself, the stack trace contains more than dio is able to get it.

[kuhnroyal]

Ok, scratch that. There was no dio_sentry configured for those events. I used a different instance for testing...

However, now that it is configured, the stacktrace for the same test exception doesn't contain the _testException call anymore:

SentryHttpClientError: Exception: Event was captured because the request status code was 404
  File "failed_request_client_adapter.dart", line 152, col 16, in FailedRequestClientAdapter._captureEvent
  File "failed_request_client_adapter.dart", line 101, col 15, in FailedRequestClientAdapter.fetch
  File "<asynchronous suspension>"
  File "tracing_client_adapter.dart", line 42, col 18, in TracingClientAdapter.fetch
  File "<asynchronous suspension>"
  File "breadcrumb_client_adapter.dart", line 41, col 11, in BreadcrumbClientAdapter.fetch
  File "<asynchronous suspension>"
  File "dio_mixin.dart", line 648, col 22, in DioMixin._dispatchRequest

[kuhnroyal]

So there is something wrong in the FailedRequestClient.

    try {
      final response =
          await _client.fetch(options, requestStream, cancelFuture);
      statusCode = response.statusCode;
      return response;
    } catch (e, st) {
      exception = e;
      stackTrace = st;
      rethrow;
    } finally {
      stopwatch.stop();

Unfortunately the catch block doesn't get called with any DioError because the future always contains a ResponseBody in a state before it is converted to a DioError. We don't have the cause exception and real stacktrace available when determining failedRequestStatusCodes.containsStatusCode(statusCode).

All the interceptors have not been called at this point.

[kuhnroyal]

The whole idea with wrapping the adapter doesn't work :( The error handling is done in the DioMixin after the adapters. The adapter never knows about any DioError, everything is a ResponseBody there.

[ueman]

But does that matter? The adapter shouldn't report any exceptions. The exception is only there, so we can add it as a reason for the transaction or 'bad status code requests'. The global error handler or Sentry.captureException can then report the error.

The FailedRequestAdapter still captures if anything is wrong while doing the request, but any exception from interceptors aren't captured by the adapter.

[kuhnroyal]

But that results in 2 events for every error doesn't it? And the one from the adapter isn't helpful because it never has the correct stacktrace?

[ueman]

Good point. There's code for deduplication in the SDK, but since DIO wraps its excpetions it most probably won't work. So the FailedRequestAdapter should not collect exceptions & stacktraces but only bad status codes.

[marandaneto]

@kuhnroyal and @ueman how did it go with the changes for Dio on 6.4 betas?

[ueman]

@kuhnroyal and @ueman how did it go with the changes for Dio on 6.4 betas?

What exactly are you referring to?

[marandaneto]

@kuhnroyal and @ueman how did it go with the changes for Dio on 6.4 betas?

What exactly are you referring to?

After the changes we've done to improve the reporting of events on sentry_dio, they were beta for a while, did it improve with the changes? (this issue and its PRs).

[kuhnroyal]

I am happy with the current configuration, but just about to roll it in to production so will see in a few days.