Fixed: update in example/lib/queued_interceptor_csrftoken.dart #1262

seunghwanly · 2021-08-31T16:10:34Z

New Pull Request Checklist

I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass

This merge request fixes / refers to the following issues: example/interceptor_lock.dart

Pull Request Description

In my case, the example code had keep occurring the infinite requests to the server
So, I changed the callback functions to await and it worked :)

Here is the code that I run

`options`

dio.options.connectTimeout = 5000;
dio.options.maxRedirects = 3;
dio.options.responseType = ResponseType.bytes;

`onRequest`, if the token is null

           tokenDio
              .post('/obtain-token/', data: jsonEncode(requestBody))
              .then((res) {
            /// response will be different by server
            final parsedResult = jsonDecode(utf8.decode(res.data as Uint8List));
            accessToken = parsedResult['result']['access'];
            refreshToken = parsedResult['result']['refresh'];
            options.headers['Authorization'] = 'Bearer $accessToken';

            /// save to [SharedPrefernces], it can be null
            prefs.setString('accessToken', accessToken!);
            prefs.setString('refreshToken', refreshToken!);

            /// go on to next operation
            handler.next(options);
          }).catchError((error, stackTrace) {
            log(error.toString());
            handler.reject(error, true);
          }).whenComplete(
                  () => dio.unlock()); // unlock after all the task is done

`onError`, if the status code is 401

     if ('Bearer $accessToken' != options.headers['Authorization']) {
            log('Bearer ${accessToken!}', name: 'string');
            log(options.headers['Authorization'], name: 'option');
            options.headers['Authorization'] = 'Bearer $accessToken';

            final response = await dio.fetch(options);
            return handler.resolve(response);
          }

Summary

What I recommend is that avoid using too many callback functions in a single task
I think nested callback functions can occur some conflicts and are hard to debug as well 👍

AlexV525 · 2022-10-19T04:34:15Z

Hi @seunghwanly, now dio has QueuedInterceptor that handles fetch by sequence. Can you verify if the change is still needed and make a rebase on the latest code? Thanks!

seunghwanly · 2022-10-19T04:46:56Z

@AlexV525
Thanks for the reply! It's been a while since I left this pull request. As you say so, I'll check if dio still needs some changes, or else may I just close this pull request with a comment?

AlexV525 · 2022-10-19T05:23:56Z

or else may I just close this pull request with a comment?

We're planning to merge valid PRs to another community-maintained fork, so it would be great if you can follow up the upstream changes.

AlexV525 · 2022-10-19T12:48:47Z

Oh if you make force pushes on the master branch, the PR will be closed automatically.

seunghwanly · 2022-10-19T12:55:50Z

Yeah, I thought the example I pushed is not suitable from now on... Let me add some examples for QueuedInterceptorsWrapper used in example/lib/queued_interceptor_crsftoken.dart. Thanks for reminding!

seunghwanly · 2022-10-19T15:03:12Z

New Pull Request Checklist

I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
This merge request fixes / refers to the following issues: example/lib/queued_interceptor_csrftoken.dart

Pull Request Description

Summary

added getter methods in the Response class for better usage of statusCode
removed callbacks
replaced callbacks(by using .then( ) and .catchError( )s) with async/await
reflected the changes in README.md

Issues

in the example(example/lib/queued_interceptor_csrftoken.dart) the _onResult method was not being called, since the example baseUrl ('http://www.dtworkroom.com/doris/1/2.0.0/') was not able to run
at line 34 in the example, the token is already expired because the status code was 401. And also in onError closure, after the token has been updated then the dio asks for fetch and resolve it.
So I thought the below code might be an extra task because the dio will update the token and resolve the request.

// If the token has been updated, repeat directly.
        if (csrfToken != options.headers['csrfToken']) {
          options.headers['csrfToken'] = csrfToken;
          //repeat
          dio.fetch(options).then(
            (r) => handler.resolve(r),
            onError: (e) {
              handler.reject(e);
            },
          );
          return;
        }

Opinion

for better practice, using async/await can be more helpful than using in a synchronous way

AlexV525

Thanks for the update!

AlexV525 · 2022-10-20T03:03:21Z

dio/lib/src/response.dart

+  /// Returns if the response has an error
+  ///
+  /// `hasError` is true when status code is not between 200 and 299
+  bool get hasError => statusCode != null && statusCode! ~/ 100 != 2;


IIRC we have ValidateStatus as a prediction, it might be inappropriate to use such a getter.

I thought it would be nice to have the getter direct to the Responseobject, but if it is an appropriate way I'll remove the getter. Thanks :)

AlexV525 · 2022-10-20T03:04:44Z

#1457 is trying to solve the issue by refactoring the interceptor in the example. Can you verify if both fixes are valid or which should we apply?

seunghwanly · 2022-10-20T04:25:53Z

#1457 is trying to solve the issue by refactoring the interceptor in the example. Can you verify if both fixes are valid or which should we apply?

I've seen the #1457 pull request and I think we have both valid fixes. Since we are assuming the same way as below

{
  "data": {
    "token": "valid token"
  }
}

and in my opinion, it needs some changes like

too many callbacks occurred in a nested way
too much implementation for updating the token, such as creating a private class, which makes it less reusable. Handling the token updates can be easily done by implementing onRequest, onError, and onResponse.
handling Futures in a nested callback will make it the users hard to debug and trace the issues.

and also I think the examples should be easy to read and understand as well so that new comers can adjust and implement their codes.

AlexV525 · 2022-10-20T04:39:57Z

I've seen the #1457 pull request and I think we have both valid fixes.

Great. Given your reply I would like to pick your PR first, then we can continue to discuss another one if it gets more feedback.

since `dio` has a `ValidateStatus` as a prediction, it's inappropriate methods.

seunghwanly · 2022-10-20T05:25:37Z

Glad to hear that :) I removed the getter methods as well.

AlexV525 · 2022-10-20T16:07:10Z

We need to fix these before merge.

Analyzing example...

  error - lib/queued_interceptor_crsftoken.dart:22:20 - The getter 'hasSucceed' isn't defined for the type 'Response<dynamic>'. Try importing the library that defines 'hasSucceed', correcting the name to the name of an existing getter, or defining a getter or field named 'hasSucceed'. - undefined_getter
  error - lib/queued_interceptor_crsftoken.dart:67:2[8](https://github.com/cfug/dio3/actions/runs/3291142303/jobs/5424914251#step:5:9) - The getter 'hasSucceed' isn't defined for the type 'Response<dynamic>'. Try importing the library that defines 'hasSucceed', correcting the name to the name of an existing getter, or defining a getter or field named 'hasSucceed'. - undefined_getter

seunghwanly · 2022-10-21T04:32:50Z

Sorry, I forgot to remove the getter method in the example. Thanks for the notice!

AlexV525 · 2022-12-18T16:00:27Z

Hi @seunghwanly. We've made our hardfork repo public and published a new version of dio, named diox.
The new package contains the PR of the fix.
Please refer to https://pub.dev/packages/diox/versions/5.0.0-dev.1 to use the fork.
You can also see why we're working for a hardfork at cfug/diox#29 and #1607.

seunghwanly · 2022-12-18T17:34:06Z

Thanks for letting me know :) Good to hear that more future jobs will be taken at diox library!
It seems that this PR is already in diox's example. Is more action needed for the next step?

AlexV525 · 2022-12-18T17:58:43Z

Is more action needed for the next step?

No. :) Thanks again for your contribution.

seunghwanly changed the title ~~removed infinite request~~ Fixed: update in example/interceptor_lock.dart, removed infinite request Aug 31, 2021

seunghwanly closed this Oct 19, 2022

seunghwanly reopened this Oct 19, 2022

seunghwanly closed this Oct 19, 2022

seunghwanly force-pushed the master branch from df17261 to ac78e61 Compare October 19, 2022 12:17

seunghwanly added 2 commits October 19, 2022 23:30

feat: getter method for status code

d37915d

fix: csrftoken example using async/await

f335c91

seunghwanly reopened this Oct 19, 2022

fix: reflect changes in README.md

f092d0d

seunghwanly changed the title ~~Fixed: update in example/interceptor_lock.dart, removed infinite request~~ Fixed: update in example/lib/queued_interceptor_csrftoken.dart Oct 20, 2022

AlexV525 mentioned this pull request Oct 20, 2022

fix queued_interceptor_crsftoken #1457

Closed

6 tasks

AlexV525 reviewed Oct 20, 2022

View reviewed changes

fix: remove getter methods

21f63c2

since `dio` has a `ValidateStatus` as a prediction, it's inappropriate methods.

AlexV525 mentioned this pull request Oct 20, 2022

Fix csrf token in example cfug/diox#1

Merged

6 tasks

fix: remove getter method from example

8f511c7

AlexV525 closed this Feb 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed: update in example/lib/queued_interceptor_csrftoken.dart #1262

Fixed: update in example/lib/queued_interceptor_csrftoken.dart #1262

seunghwanly commented Aug 31, 2021

AlexV525 commented Oct 19, 2022

seunghwanly commented Oct 19, 2022

AlexV525 commented Oct 19, 2022

AlexV525 commented Oct 19, 2022

seunghwanly commented Oct 19, 2022 •

edited

Loading

seunghwanly commented Oct 19, 2022 •

edited

Loading

AlexV525 left a comment

AlexV525 Oct 20, 2022

seunghwanly Oct 20, 2022

AlexV525 commented Oct 20, 2022

seunghwanly commented Oct 20, 2022 •

edited

Loading

AlexV525 commented Oct 20, 2022

seunghwanly commented Oct 20, 2022

AlexV525 commented Oct 20, 2022

seunghwanly commented Oct 21, 2022

AlexV525 commented Dec 18, 2022

seunghwanly commented Dec 18, 2022

AlexV525 commented Dec 18, 2022

Fixed: update in example/lib/queued_interceptor_csrftoken.dart #1262

Fixed: update in example/lib/queued_interceptor_csrftoken.dart #1262

Conversation

seunghwanly commented Aug 31, 2021

New Pull Request Checklist

Pull Request Description

options

onRequest, if the token is null

onError, if the status code is 401

Summary

AlexV525 commented Oct 19, 2022

seunghwanly commented Oct 19, 2022

AlexV525 commented Oct 19, 2022

AlexV525 commented Oct 19, 2022

seunghwanly commented Oct 19, 2022 • edited Loading

seunghwanly commented Oct 19, 2022 • edited Loading

New Pull Request Checklist

Pull Request Description

Summary

Issues

Opinion

AlexV525 left a comment

Choose a reason for hiding this comment

AlexV525 Oct 20, 2022

Choose a reason for hiding this comment

seunghwanly Oct 20, 2022

Choose a reason for hiding this comment

AlexV525 commented Oct 20, 2022

seunghwanly commented Oct 20, 2022 • edited Loading

AlexV525 commented Oct 20, 2022

seunghwanly commented Oct 20, 2022

AlexV525 commented Oct 20, 2022

seunghwanly commented Oct 21, 2022

AlexV525 commented Dec 18, 2022

seunghwanly commented Dec 18, 2022

AlexV525 commented Dec 18, 2022

`options`

`onRequest`, if the token is null

`onError`, if the status code is 401

seunghwanly commented Oct 19, 2022 •

edited

Loading

seunghwanly commented Oct 19, 2022 •

edited

Loading

seunghwanly commented Oct 20, 2022 •

edited

Loading