-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
feat: support request cancellation for native HTTP clients #2434
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
feat: support request cancellation for native HTTP clients #2434
Conversation
plugins/native_dio_adapter/lib/src/conversion_layer_adapter.dart
Outdated
Show resolved
Hide resolved
Co-authored-by: Alex Li <github@alexv525.com> Signed-off-by: Zhen Zhi Lee <zzyzy@users.noreply.github.com>
Co-authored-by: Alex Li <github@alexv525.com> Signed-off-by: Zhen Zhi Lee <zzyzy@users.noreply.github.com>
AlexV525
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for adding tests
plugins/native_dio_adapter/test/conversion_layer_adapter_test.dart
Outdated
Show resolved
Hide resolved
plugins/native_dio_adapter/test/conversion_layer_adapter_test.dart
Outdated
Show resolved
Hide resolved
…dart Co-authored-by: Alex Li <github@alexv525.com> Signed-off-by: Zhen Zhi Lee <zzyzy@users.noreply.github.com>
Co-authored-by: Alex Li <github@alexv525.com> Signed-off-by: Zhen Zhi Lee <zzyzy@users.noreply.github.com>
AlexV525
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with only one nit picking
| @@ -1,3 +1,4 @@ | |||
| import 'package:async/async.dart'; | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we only show classes we are using here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Code Coverage Report: Only Changed Files listed
Minimum allowed coverage is |
New versions of
http,cronet_httpandcupertino_httpsupports request cancellation (just like theHttpClientfromdart:io).Minor change to use
AbortableRequestand pass thecancelFutureto theabortTriggerargument.New Pull Request Checklist
mainbranch to avoid conflicts (via merge from master or rebase)CHANGELOG.mdin the corresponding packageAdditional context and info (if any)
Testing with ASP.NET Core API
Test Flutter app (Android and iOS only):
https://github.com/zzyzy/dio_lzz_fork_flutter/blob/main/lib/test_dio_request_cancellation.dart
Test API endpoint:
Android
Before:
After:
IOS
Before:
After: