Update `sentry_dio` to dio 5 #1282

ueman · 2023-02-13T15:14:58Z

📜 Description

Dio changed in ownership to the guys behind diox, which previously was forked from dio.
Since the ownership changed, they've published diox v5 also as dio v5.
This PR makes the necessary changes to be compatible.

💡 Motivation and Context

Compatibility with dio v5

💚 How did you test it?

Tests

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
I updated the docs if needed
All tests passing
No breaking changes

🔮 Next steps

codecov · 2023-02-13T15:19:33Z

Codecov Report

Patch coverage: 100.00% and no project coverage change

Comparison is base (2af98bf) 90.53% compared to head (d77f04f) 90.53%.

Additional details and impacted files

@@           Coverage Diff           @@
##           v7.0.0    #1282   +/-   ##
=======================================
  Coverage   90.53%   90.53%           
=======================================
  Files          50       50           
  Lines        1649     1649           
=======================================
  Hits         1493     1493           
  Misses        156      156

Impacted Files	Coverage Δ
dio/lib/src/breadcrumb_client_adapter.dart	`80.00% <ø> (ø)`
dio/lib/src/sentry_dio_client_adapter.dart	`83.33% <ø> (ø)`
dio/lib/src/tracing_client_adapter.dart	`85.71% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

krystofwoldrich · 2023-02-13T16:31:35Z

Thanks, it looks good, but CI is not happy yet. :)

kuhnroyal · 2023-02-13T17:01:27Z

There are a few things missing.

ueman · 2023-02-13T17:25:17Z

Thanks, it looks good, but CI is not happy yet. :)

I'll look at it later.

There are a few things missing.

What did I miss?

kuhnroyal · 2023-02-14T11:26:16Z

Thanks, it looks good, but CI is not happy yet. :)

I'll look at it later.

There are a few things missing.

What did I miss?

Nah all good, sorry. I was still working on the 6.x branch.

kuhnroyal · 2023-02-14T11:26:50Z

Would be great if we can get this in a new 7.0.0 beta asap.

ueman · 2023-02-14T11:54:42Z

Would be great if we can get this in a new 7.0.0 beta asap.

I'm on it 🚀

ueman · 2023-02-14T11:55:05Z

dio/test/breadcrumb_client_adapter_test.dart

-        expect(e.message, 'Exception: test');
+        expect(e.error.toString(), 'Exception: test');


This is due to changes in dio

ueman · 2023-02-14T11:55:29Z

dio/test/dio_event_processor_test.dart

@@ -75,7 +75,6 @@ void main() {
      expect(processedEvent.request?.queryString, 'foo=bar');
      expect(processedEvent.request?.headers, <String, String>{
        'foo': 'bar',
-        'content-type': 'application/json; charset=utf-8'


dio doesn't apply a content-type by itself anymore

flutter/example/pubspec.yaml

brustolin

LGTM

krystofwoldrich

LGTM 🚀

ueman · 2023-02-16T16:22:20Z

Is there something left to do before it can be merged?

krystofwoldrich · 2023-02-17T09:31:17Z

@ueman I think it's ready, just let's update the changelog to show this as a breaking change.

CHANGELOG.md

kuhnroyal · 2023-02-20T10:34:40Z

Would love to get a beta release with this :)

ueman · 2023-02-28T13:27:57Z

The pipeline still has one failing job, but it's for the pure Dart SDK. Since that's not touch in this PR, I'm wondering whether that's just a flake. Can someone rerun the jobs?

Edit: Apparently, that job is failing for all PRs which target the v7 branch, so it's unrelated to this PR.

marandaneto · 2023-02-28T15:43:54Z

@ueman

The build_web_compilers version – 3.2.7 – is not within the allowed constraint – ^4.0.0.

We have to upgrade build_web_compilers to ^4.0.0 and build_runner to ^2.4.0 cus the beta channel uses Dart 3 and the new versions require Dart 3.
Pretty much the inverse of #1299
Can be another PR since its not related to this issue.

marandaneto · 2023-02-28T15:44:47Z

I will look into this tomorrow, sorry about the delay, just came back from vacation and catching up with all the pending things.

ueman · 2023-02-28T17:09:29Z

No worries, thanks for looking into it.

marandaneto

Thanks for doing this @ueman

marandaneto · 2023-03-01T16:24:43Z

Mind appending the migration page?

ueman · 2023-03-02T05:09:15Z

Mind appending the migration page?

Sure. Is there anything in particular you'd like to see?

Edit: It's here getsentry/sentry-docs#6400

kuhnroyal · 2023-03-09T14:46:21Z

I have tested this change excessively over the last few days and the whole update from Sentry 6.x/Dio 4.x to Sentry 7.0/Dio 5.0 is a big step back regarding traceability of the DioError. This is in big part due to changes in dio which removed the whole source stacktrace. But due to the DioErrorExtractor there are now also cases where the inner stacktrace of DioError.error.stacktrace is completely missing in the event.

I think the DioError.stacktrace needs to be added by default, as it was the case in 6.x and only the inner cause error/stacktrace should be extracted via the DioErrorExtractor.

Something like:

class DioErrorExtractor extends ExceptionCauseExtractor<DioError> {
  @override
  ExceptionCause? cause(DioError error) {
    if (error.error == null) {
      return null;
    }
    return ExceptionCause(
      error.error,
      error.error is Error ? (error.error as Error).stackTrace : null,
    );
  }
}

ueman added 2 commits February 13, 2023 16:11

Update to dio 5

ff85c07

changelog

371b882

fix tests

81c2f69

ueman commented Feb 14, 2023

View reviewed changes

set constrain for example app

9287fe6

kuhnroyal reviewed Feb 14, 2023

View reviewed changes

flutter/example/pubspec.yaml Show resolved Hide resolved

another round of constraints

08e3369

ueman marked this pull request as ready for review February 15, 2023 11:48

ueman requested review from marandaneto, brustolin and krystofwoldrich as code owners February 15, 2023 11:48

brustolin approved these changes Feb 16, 2023

View reviewed changes

krystofwoldrich approved these changes Feb 16, 2023

View reviewed changes

kuhnroyal approved these changes Feb 17, 2023

View reviewed changes

ueman commented Feb 17, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

c201708

ueman mentioned this pull request Feb 22, 2023

Add support for Dio 5.0.0 #1292

Closed

marandaneto added 4 commits March 1, 2023 17:21

Merge branch 'v7.0.0' into update-to-dio-5

ccb5c78

fix changelog

2645db2

Merge branch 'v7.0.0' into update-to-dio-5

f0d12f6

fix

d77f04f

marandaneto approved these changes Mar 1, 2023

View reviewed changes

marandaneto merged commit 0034370 into getsentry:v7.0.0 Mar 2, 2023

ueman deleted the update-to-dio-5 branch March 2, 2023 09:36

This was referenced Mar 9, 2023

Update sentry dio to last dio version #1321

Closed

DioError extractor extracts wrong stacktrace #1322

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Update `sentry_dio` to dio 5 #1282

Update `sentry_dio` to dio 5 #1282

ueman commented Feb 13, 2023

codecov bot commented Feb 13, 2023 •

edited

Loading

krystofwoldrich commented Feb 13, 2023 •

edited

Loading

kuhnroyal commented Feb 13, 2023

ueman commented Feb 13, 2023 •

edited

Loading

kuhnroyal commented Feb 14, 2023

kuhnroyal commented Feb 14, 2023

ueman commented Feb 14, 2023

ueman Feb 14, 2023

ueman Feb 14, 2023

brustolin left a comment

krystofwoldrich left a comment

ueman commented Feb 16, 2023

krystofwoldrich commented Feb 17, 2023

kuhnroyal commented Feb 20, 2023

ueman commented Feb 28, 2023 •

edited

Loading

marandaneto commented Feb 28, 2023 •

edited

Loading

marandaneto commented Feb 28, 2023

ueman commented Feb 28, 2023

marandaneto left a comment

marandaneto commented Mar 1, 2023

ueman commented Mar 2, 2023 •

edited

Loading

kuhnroyal commented Mar 9, 2023

		expect(e.message, 'Exception: test');
		expect(e.error.toString(), 'Exception: test');

Update sentry_dio to dio 5 #1282

Update sentry_dio to dio 5 #1282

Conversation

ueman commented Feb 13, 2023

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

codecov bot commented Feb 13, 2023 • edited Loading

Codecov Report

krystofwoldrich commented Feb 13, 2023 • edited Loading

kuhnroyal commented Feb 13, 2023

ueman commented Feb 13, 2023 • edited Loading

kuhnroyal commented Feb 14, 2023

kuhnroyal commented Feb 14, 2023

ueman commented Feb 14, 2023

ueman Feb 14, 2023

Choose a reason for hiding this comment

ueman Feb 14, 2023

Choose a reason for hiding this comment

brustolin left a comment

Choose a reason for hiding this comment

krystofwoldrich left a comment

Choose a reason for hiding this comment

ueman commented Feb 16, 2023

krystofwoldrich commented Feb 17, 2023

kuhnroyal commented Feb 20, 2023

ueman commented Feb 28, 2023 • edited Loading

marandaneto commented Feb 28, 2023 • edited Loading

marandaneto commented Feb 28, 2023

ueman commented Feb 28, 2023

marandaneto left a comment

Choose a reason for hiding this comment

marandaneto commented Mar 1, 2023

ueman commented Mar 2, 2023 • edited Loading

kuhnroyal commented Mar 9, 2023

Update `sentry_dio` to dio 5 #1282

Update `sentry_dio` to dio 5 #1282

codecov bot commented Feb 13, 2023 •

edited

Loading

krystofwoldrich commented Feb 13, 2023 •

edited

Loading

ueman commented Feb 13, 2023 •

edited

Loading

ueman commented Feb 28, 2023 •

edited

Loading

marandaneto commented Feb 28, 2023 •

edited

Loading

ueman commented Mar 2, 2023 •

edited

Loading