Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Created TrackingInterceptor as an alternative to TrackedDioClient #1

Merged
merged 28 commits into from
Mar 30, 2023

Conversation

lohnn
Copy link
Contributor

@lohnn lohnn commented Jun 14, 2022

The TrackedDioClient had some shortcomings, so I created an interceptor for Dio to have flexibility and support Dio clients without having to keep implementation up to date with DioMixin interface, as Interceptors will most likely have a more stable API surface.

Current shortcomings of this implementation is that StackTrace in error flow will not work until PR cfug/dio#1503 is merged.

@teodor-appd
Copy link
Collaborator

Waiting for cfug/dio#1503 to be merged.

@teodor-appd
Copy link
Collaborator

teodor-appd commented Sep 15, 2022

What's the status on this one, @lohnn? I think I lost the Discord account where we were communicating.

@lohnn
Copy link
Contributor Author

lohnn commented Sep 15, 2022

We are still waiting for the Dio team to become active again and merge my PR on their side. Once that is done, this PR should be all good to go.
We mailed one of the maintainers in hopes of getting it going.

@lohnn
Copy link
Contributor Author

lohnn commented Sep 15, 2022

In regards to your Discord account, it says you are "Deleted user", not sure if it's possible to get it back tho. 🤔

@spydon
Copy link
Contributor

spydon commented Mar 6, 2023

How's this PR looking now when Dio 5.0.0 is out?
It's a bit unclear what happened to all the PRs when they first moved to Diox and then back to Dio.

@teodor-appd
Copy link
Collaborator

How's this PR looking now when Dio 5.0.0 is out? It's a bit unclear what happened to all the PRs when they first moved to Diox and then back to Dio.

We've added supporting Dio 5.0.0 to the roadmap. However, it will be scheduled for next month. Regarding the PRs, I don't know if you're talking about this project's or Dio's repo. You might be talking about the latter, where I'm unfamiliar with the context, but maybe @lohnn can help.

@lohnn
Copy link
Contributor Author

lohnn commented Mar 6, 2023

I'll take a look at if I need to reopen the cfug/dio#1503 pr or not for this one to work. Just haven't had time to do so yet.

@lohnn
Copy link
Contributor Author

lohnn commented Mar 6, 2023

Okay, so I've done a small test locally now and the status of the connected PR cfug/dio#1503 is that I'll have to reopen that one, fix any potential merge conflicts or breaking changes and still wait for approval.
Good thing is that it's a new team of people and it will most likely be easier to get it merged.

@lohnn
Copy link
Contributor Author

lohnn commented Mar 7, 2023

As the old PR at Dio was closed, there is a new one cfug/dio#1721, hopefully this one is merged soon, then this PR should be able to be merged once they release a new Dio version.

@AlexV525
Copy link

AlexV525 commented Mar 7, 2023

How's this PR looking now when Dio 5.0.0 is out? It's a bit unclear what happened to all the PRs when they first moved to Diox and then back to Dio.

I just realized that we've never talked about this. From the start, we assume Dio will not get maintained anymore, so we've picked valid (at the first few looks) into our hard fork. Then we got the ownership, so those merged PRs were merged back to the original repo, again. If you can see a linked PR targeting Diox that is merged, it's merged in the Dio too.

@lohnn
Copy link
Contributor Author

lohnn commented Mar 27, 2023

Thanks to cfug/dio#1722 this is no longer blocked.
The failing tests are in classes that I have, as far as I can see, not touched. Not sure what to do with those.

…gent into tracking_interceptor

# Conflicts:
#	pubspec.yaml
Make skipped error path test work.
@lohnn
Copy link
Contributor Author

lohnn commented Mar 30, 2023

I'll have a look at your changes and see, and let's then try to get this merged shortly 👍

lohnn and others added 6 commits March 30, 2023 14:24
…lutter_agent into tracking_interceptor

# Conflicts:
#	lib/src/tracked_clients/tracked_dio_interceptor.dart
#	test/tracked_dio_interceptor_test.dart
It seems like it reduces flakiness.
@teodor-appd teodor-appd self-requested a review March 30, 2023 14:31

final Map<RequestOptions, RequestTracker> _activeTrackers = {};

@visibleForTesting
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was searching for this one. 🍻

@teodor-appd teodor-appd merged commit a76ca20 into Appdynamics:master Mar 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants