Feature: Improve Dio exception reports #718

ueman · 2022-01-26T16:26:59Z

📜 Description

Adds an event processor, which add chained exceptions for the inner exception of DioError,
see https://sentry.io/share/issue/638a3758a43d4f61bc2a0c62acdefd69/

This one should be merged after #728

💡 Motivation and Context

Fixes #715

💚 How did you test it?

📝 Checklist

🔮 Next steps

flutter/example/lib/main.dart

kuhnroyal · 2022-01-26T16:31:13Z

I never get any DioError in the FailedRequestClient , the whole block here: https://github.com/flutterchina/dio/blob/develop/dio/lib/src/dio_mixin.dart#L620. is executed later.

dio/lib/src/dio_event_processor.dart

kuhnroyal · 2022-01-26T17:07:57Z

This works, I have the correct in-app grouping.

kuhnroyal

I basically copied this and and added some more code from FailedRequestClient in order to add a SentryRequest to the event. And I disabled the FailedRequestClient. This combination now gives me a pretty good result.
The only problem I see, is that the error is not tied to the transaction in any way.

dio/lib/src/dio_event_processor.dart

ueman · 2022-01-26T18:41:01Z

I basically copied this and and added some more code from FailedRequestClient in order to add a SentryRequest to the event. And I disabled the FailedRequestClient. This combination now gives me a pretty good result.

That's actually a pretty good idea.

The only problem I see, is that the error is not tied to the transaction in any way.

As this integration doesn't start any transactions, you basically have to write code like the one below anyway

  final transaction = Sentry.getSpan() ??
      Sentry.startTransaction(
        'dio-web-request',
        'request',
        bindToScope: true,
      );
  try {
    final response = await dio.get<String>('https://flutter.dev/');
    transaction.spanStatus = SpanStatus.ok();
  } catch (exception, stackTrace) {
    transaction.throwable = exception;
    transaction.spanStatus = SpanStatus.internalError();
    Sentry.captureException(exception, stacktrace);
  } finally {
    await transaction.finish();
  }

and then it is tied together.

dio/lib/src/dio_event_processor.dart

kuhnroyal · 2022-01-26T18:46:41Z

As this integration doesn't start any transactions, you basically have to write code like the one below anyway

True, it is all tied to my Flutter route transactions.

codecov-commenter · 2022-01-26T19:04:53Z

Codecov Report

Merging #718 (43f2daa) into main (edc6acd) will increase coverage by 1.39%.
The diff coverage is 92.15%.

@@            Coverage Diff             @@
##             main     #718      +/-   ##
==========================================
+ Coverage   90.53%   91.93%   +1.39%     
==========================================
  Files         104       18      -86     
  Lines        3328      533    -2795     
==========================================
- Hits         3013      490    -2523     
+ Misses        315       43     -272

Impacted Files	Coverage Δ
dio/lib/src/sentry_dio_extension.dart	`81.81% <85.71%> (+1.81%)`	⬆️
dio/lib/src/dio_event_processor.dart	`93.18% <93.18%> (ø)`
dio/lib/src/sentry_transformer.dart	`94.44% <0.00%> (-5.56%)`	⬇️
...art/lib/src/http_client/failed_request_client.dart
dart/lib/src/protocol/max_request_body_size.dart
dart/lib/src/protocol/sentry_stack_trace.dart
dart/lib/src/default_integrations.dart
dart/lib/src/protocol/debug_image.dart
dart/lib/src/protocol/sentry_runtime.dart
dart/lib/src/noop_sentry_span.dart
... and 80 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update edc6acd...43f2daa. Read the comment docs.

dio/lib/src/dio_event_processor.dart

dart/lib/src/http_client/sentry_http_client.dart

dio/lib/src/dio_event_processor.dart

Co-authored-by: Peter Leibiger <[email protected]>

kuhnroyal · 2022-01-27T09:58:06Z

When a description is set on the mechanism, the type is not shown in Sentry UI. Not sure why.

ueman · 2022-01-27T10:02:08Z

When a description is set on the mechanism, the type is not shown in Sentry UI. Not sure why.

Yeah, that's also true for FlutterError which sometime have the description set.
I don't know whether that's working as expected or not. Maybe @marandaneto know.
Probably this is worth an issue at the Sentry repo itself.

dio/lib/src/dio_event_processor.dart

marandaneto · 2022-01-27T11:22:33Z

When a description is set on the mechanism, the type is not shown in Sentry UI. Not sure why.

Yeah, that's also true for FlutterError which sometime have the description set. I don't know whether that's working as expected or not. Maybe @marandaneto know. Probably this is worth an issue at the Sentry repo itself.

I agree, an issue there would be better, I don't know the answer either.

dio/lib/src/dio_event_processor.dart

# Conflicts: # dio/lib/src/sentry_dio_extension.dart

marandaneto · 2022-02-12T08:33:55Z

dio/lib/src/dio_event_processor.dart

+      if (innerDioErrorException.runtimeType == String) {
+        innerException =
+            innerException.copyWith(type: 'DioError inner stacktrace');
+      }


What's the reason for not using it's type? String in this case.

marandaneto · 2022-02-12T08:36:23Z

@ueman can you share a link of an issue after those changes or a screenshot? thanks.
Looks like this PR indeed makes it much better.
Logic wise LGTM, @brustolin please review :)

ueman · 2022-02-12T09:15:40Z

await dio.get<String>('https://flutter.dev/asdfasg');, throws because 404 status code: https://sentry.io/organizations/sentry-sdks/issues/3007144862/events/31495690091f4d7684c339e6fd9a6ebc/?project=5428562

await dio.post<String>('https://flutter.dev/', data: Exception());, throws because Exception can't be serialized: https://sentry.io/organizations/sentry-sdks/issues/3009008663/events/fcb1d27e11be4af4b71b25b6c04128c6/?project=5428562

brustolin · 2022-02-14T11:26:49Z

@marandaneto, as you said, code looks good.
But I don't really know how EventProcessor works in the Dart SDK, need to get familiar with this.

ueman · 2022-02-14T11:33:19Z

@marandaneto, as you said, code looks good. But I don't really know how EventProcessor works in the Dart SDK, need to get familiar with this.

Feel free to ask (here or on Discord), I'll probably can answer at least some questions.

marandaneto · 2022-02-14T13:20:25Z

@ueman sentry-dio / package-analysis is unhappy, otherwise LGTM.

marandaneto

@ueman thanks for making the dio integration better :)

marandaneto · 2022-02-14T13:21:07Z

@kuhnroyal also thanks for the valuable tests and insights.

ueman · 2022-02-14T14:04:25Z

I would guess it's because pana doesn't check the dependency overrides? The getter which pana complains about isn't available in the latest stable but on main. I don't think I can do anything about that.

Pana isn't happy because of:

[x] 0/30 points: code has no errors, warnings, lints, or formatting issues

Found 3 issues. Showing the first 2:

ERROR: The getter 'exceptionFactory' isn't defined for the type 'SentryOptions'.

`lib/src/dio_event_processor.dart:20:16```` ╷ 20 │ _options.exceptionFactory; │ ^^^^^^^^^^^^^^^^ ╵ ``` To reproduce make sure you are using the [lints_core](https://pub.dev/packages/lints) and run `dart analyze lib/src/dio_event_processor.dart`

ERROR: The method 'shouldAddBody' isn't defined for the type 'MaxRequestBodySize'.

`lib/src/dio_event_processor.dart:127:31` ``` ╷ 127 │ if (_maxRequestBodySize.shouldAddBody(data.codeUnits.length)) { │ ^^^^^^^^^^^^^ ╵ ``` To reproduce make sure you are using the [lints_core](https://pub.dev/packages/lints) and run `dart analyze lib/src/dio_event_processor.dart`

dio/lib/src/dio_event_processor.dart

dio/lib/src/sentry_dio_extension.dart

marandaneto · 2022-02-15T18:08:02Z

I would guess it's because pana doesn't check the dependency overrides? The getter which pana complains about isn't available in the latest stable but on main. I don't think I can do anything about that.

Pana isn't happy because of:

[x] 0/30 points: code has no errors, warnings, lints, or formatting issues

Found 3 issues. Showing the first 2:

ERROR: The getter 'exceptionFactory' isn't defined for the type 'SentryOptions'.
ERROR: The method 'shouldAddBody' isn't defined for the type 'MaxRequestBodySize'.

Indeed, I believe pana should respect dependency_overrides or at least offer an option to respect it since multi-module projects would run into this issue very often.
Maybe worth filing an issue on https://github.com/dart-lang/pana/ as a feature request.
Let's merge and not block on this then :)

marandaneto · 2022-02-15T18:08:53Z

I'm fine if you want to address #718 (comment) or keep as it is, your call.
I'm good to merge as it is.

Improve Dio

0a38749

ueman commented Jan 26, 2022

View reviewed changes

flutter/example/lib/main.dart Outdated Show resolved Hide resolved

ueman commented Jan 26, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Outdated Show resolved Hide resolved

kuhnroyal reviewed Jan 26, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Outdated Show resolved Hide resolved

dio/lib/src/dio_event_processor.dart Outdated Show resolved Hide resolved

ueman commented Jan 26, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Outdated Show resolved Hide resolved

ueman added 2 commits January 26, 2022 20:02

Improve DioError EventProcessor

b5892ac

Improve example

61a5b31

some more improvements

0234ec4

kuhnroyal reviewed Jan 26, 2022

View reviewed changes

Update dio/lib/src/dio_event_processor.dart

f810592

Co-authored-by: Peter Leibiger <[email protected]>

marandaneto reviewed Jan 27, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Outdated Show resolved Hide resolved

marandaneto reviewed Jan 27, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Outdated Show resolved Hide resolved

ueman mentioned this pull request Jan 27, 2022

Mechanism name doesn't show in crash report view if it has a description getsentry/sentry#31428

Open

ueman added 4 commits January 27, 2022 17:58

wip

c3fce27

revert changes

8906540

add some tests

9b2ec26

revert

b902a54

kuhnroyal reviewed Jan 27, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Outdated Show resolved Hide resolved

kuhnroyal reviewed Jan 27, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Show resolved Hide resolved

wip

c899deb

This was referenced Jan 30, 2022

ref: Change how factories are accessed #727

Merged

[Dio] Replace failed http client adapter with interceptor #728

Merged

ueman added 5 commits February 11, 2022 15:21

Merge remote-tracking branch 'origin/main' into improve-dio

ff4dc79

# Conflicts: # dio/lib/src/sentry_dio_extension.dart

some more tests

2b79904

more tests

d291d03

fix test

1fcf734

further improve errors

76adf4c

marandaneto reviewed Feb 12, 2022

View reviewed changes

ueman added 4 commits February 12, 2022 09:55

remove type

0286f4d

lint clean up

f88aaa3

further improve reports

a38c2d8

improve in app frames in example

7d74c5b

marandaneto approved these changes Feb 14, 2022

View reviewed changes

Merge branch 'main' into improve-dio

d75dd3d

marandaneto reviewed Feb 15, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Show resolved Hide resolved

marandaneto reviewed Feb 15, 2022

View reviewed changes

dio/lib/src/dio_event_processor.dart Show resolved Hide resolved

marandaneto reviewed Feb 15, 2022

View reviewed changes

dio/lib/src/sentry_dio_extension.dart Show resolved Hide resolved

marandaneto added 3 commits February 21, 2022 12:24

Merge branch 'main' into improve-dio

79b9df9

Merge branch 'main' into improve-dio

c7289d9

Merge branch 'main' into improve-dio

43f2daa

marandaneto enabled auto-merge (squash) February 21, 2022 15:15

marandaneto merged commit f4c06af into getsentry:main Feb 21, 2022

marandaneto mentioned this pull request Feb 21, 2022

Reduce dio package pana analyzer to 90 points #764

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: Improve Dio exception reports #718

Feature: Improve Dio exception reports #718

ueman commented Jan 26, 2022 •

edited

Loading

kuhnroyal commented Jan 26, 2022

kuhnroyal commented Jan 26, 2022

kuhnroyal left a comment

ueman commented Jan 26, 2022 •

edited

Loading

kuhnroyal commented Jan 26, 2022

codecov-commenter commented Jan 26, 2022 •

edited

Loading

kuhnroyal commented Jan 27, 2022

ueman commented Jan 27, 2022

marandaneto commented Jan 27, 2022

marandaneto Feb 12, 2022

marandaneto commented Feb 12, 2022

ueman commented Feb 12, 2022

brustolin commented Feb 14, 2022

ueman commented Feb 14, 2022

marandaneto commented Feb 14, 2022

marandaneto left a comment

marandaneto commented Feb 14, 2022

ueman commented Feb 14, 2022 •

edited

Loading

marandaneto commented Feb 15, 2022

[x] 0/30 points: code has no errors, warnings, lints, or formatting issues

marandaneto commented Feb 15, 2022

Feature: Improve Dio exception reports #718

Feature: Improve Dio exception reports #718

Conversation

ueman commented Jan 26, 2022 • edited Loading

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

kuhnroyal commented Jan 26, 2022

kuhnroyal commented Jan 26, 2022

kuhnroyal left a comment

Choose a reason for hiding this comment

ueman commented Jan 26, 2022 • edited Loading

kuhnroyal commented Jan 26, 2022

codecov-commenter commented Jan 26, 2022 • edited Loading

Codecov Report

kuhnroyal commented Jan 27, 2022

ueman commented Jan 27, 2022

marandaneto commented Jan 27, 2022

marandaneto Feb 12, 2022

Choose a reason for hiding this comment

marandaneto commented Feb 12, 2022

ueman commented Feb 12, 2022

brustolin commented Feb 14, 2022

ueman commented Feb 14, 2022

marandaneto commented Feb 14, 2022

marandaneto left a comment

Choose a reason for hiding this comment

marandaneto commented Feb 14, 2022

ueman commented Feb 14, 2022 • edited Loading

[x] 0/30 points: code has no errors, warnings, lints, or formatting issues

marandaneto commented Feb 15, 2022

[x] 0/30 points: code has no errors, warnings, lints, or formatting issues

marandaneto commented Feb 15, 2022

ueman commented Jan 26, 2022 •

edited

Loading

ueman commented Jan 26, 2022 •

edited

Loading

codecov-commenter commented Jan 26, 2022 •

edited

Loading

ueman commented Feb 14, 2022 •

edited

Loading