refactor(fresh_dio): replace Interceptor with QueuedInterceptor

[SebastianKlaiber]

Interceptors are called concurrently, this could lead to a problem if multiple API calls are executed at the same time. Therefore, Queued Interceptor was introduced to solve this problem.

I also updated dio to version 4.0.6 because Queued Interceptor was introduced in version 4.0.2

From the Dio documentation:

QueuedInterceptor
Interceptor can be executed concurrently, that is, all of the requests enter the interceptor at once, rather than executing sequentially. However, in some cases we expect that requests enter the interceptor sequentially like #590 。 Therefore, we need to provide a mechanism for sequential access（one by one） to interceptors and QueuedInterceptor can solve this problem.

[vasilich6107]

Hi @felangel do you have any estimates on reviewing this PR?

[felangel]

Hi @felangel do you have any estimates on reviewing this PR?

sorry for the delay planning to test and publish today

[felangel]

@vasilich6107 can you confirm whether the changes resolve the issue for you?

[SebastianKlaiber]

You could test it by replacing

fresh_dio: ^0.3.2

with

fresh_dio: 
    git:
      url: https://github.com/SebastianKlaiber/fresh.git
      path: packages/fresh_dio

in your pubspec.yaml

[SebastianKlaiber]

@felangel @vasilich6107 Is there anything else I can support? So that we can get on with the PR?

[felangel]

@felangel @vasilich6107 Is there anything else I can support? So that we can get on with the PR?

I was waiting for confirmation that the changes in the PR address the issue. @SebastianKlaiber can you confirm?

[SebastianKlaiber]

I'll check the related issue #29 and follow the steps to reproduce.

[SebastianKlaiber]

The refresh calls have definitely become less, but still too many. Maybe there is a way to clear the queue if the call was successful.

[SebastianKlaiber]

Unfortunately, I could not find a way to clear all Queued Interceptors of a given type. Hopefully I have time to investigate a little more in this topic this week. Any suggestion from your side?

[vasilich6107]

@SebastianKlaiber thanks for help with testing
This is strange behaviour cause Queued Interceptors are supposed to wait in queue rather than being executed in parallel manner...

[SebastianKlaiber]

I think we could solve the Issue if we add an JWT Decoder and check if the access token is a valid JWT and then check if the JWT is still valid. If the JWT is expired, then trigger the refresh callback. We could also add the validation function as a parameter, then the consumer of the library can specify when the access token is valid.

Or we create an issue on the Dio repository.

@felangel what is your opinion on this?

[Gaurav192]

@SebastianKlaiber @felangel Can we check if current token is same compared to token in request headers? In case current token is different then return it instead of calling refresh token.

[SebastianKlaiber]

@Gaurav192 thanks for helping out. I was busy the last time, releasing a new version of my app. I will check your changes today.

[SebastianKlaiber]

Unfortunately, we still have too many refresh calls here as well.

[Gaurav192]

Unfortunately, we still have too many refresh calls here as well.

Just to make sure, are you using default values in repo and url: https://github.com/Gaurav192/fresh.git in pubspec?

[SebastianKlaiber]

I checked out your repo and changed the version to the local path in the pubspec.yaml. I added a breakpoint and isEqual returns always true.

[lalit3102]

Hi,
I am also facing this issue, when this PR is going to merge?

[felangel]

Hi,
I am also facing this issue, when this PR is going to merge?

I’ll try to take a look this weekend apologies for the delay

[SebastianKlaiber]

@felangel if you have a suggestion, I'm happy to help further with the implementation, just currently not sure what direction the implementation should go in.

Provide some validation logic or do some header checks.

[kuhnroyal]

This can be picked up again after the dio 5.0.0 merge.
There might have been some changes and if not, then we need to possibly adjust on the dio side.

[felangel]

@SebastianKlaiber just resolved merge conflicts and pulled in dio 5.0.0 can you please re-test when you have a chance? Thanks!

[SebastianKlaiber]

Thanks. Hopefully I've some time this afternoon.

[SebastianKlaiber]

Still the same issue with the cookie example, https://github.com/artflutter/fresh_repro.

I did some more investigation:
If the refresh handling is not delayed, then I seem to work as expected.

final response = await authRestClient.refreshToken(refreshToken: refreshToken);

return OAuth2Token(
  accessToken: response.token,
  refreshToken: response.refreshToken,
);

But If I add some delay, then I get too much refresh calls.

return await Future.delayed(const Duration(milliseconds: 500), () async {
  final response = await authRestClient.refreshToken(refreshToken: refreshToken);

  return OAuth2Token(
    accessToken: response.token,
    refreshToken: response.refreshToken,
  );
});

The new version is using a shelf API with bearer auth from this repository. https://github.com/graphicbeacon/dart_spa_boilerplate

[felangel]

LGTM ✅