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.

Sign up for GitHub

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

Jump to bottom

Rework CancelToken handling and HttpClient creation for IOHttpClientAdapter #1811

Closed
kuhnroyal opened this issue May 11, 2023 · 0 comments · Fixed by #1818
Closed

Rework CancelToken handling and HttpClient creation for IOHttpClientAdapter #1811

kuhnroyal opened this issue May 11, 2023 · 0 comments · Fixed by #1818
Assignees
@kuhnroyal
Labels
fixed s: feature This issue indicates a feature request

Comments

@kuhnroyal
Copy link
Member

Request Statement

  1. We should change the return type of typedef OnHttpClientCreate = HttpClient? Function(HttpClient client); not non-nullable HttpClient. Keeping it nullable makes no sense is probably a leftover of the NNBD migration.
  2. Currently we create a new HttpClient for every request where a CancelToken is provided. In Dart 2.10 an API to abort requests was added, we should use HttpClientRequest.abort when the CancelToken is cancelled

Solution Brainstorm

No response
@kuhnroyal kuhnroyal added the s: feature This issue indicates a feature request label May 11, 2023
@kuhnroyal kuhnroyal self-assigned this May 11, 2023
This was referenced May 11, 2023
Merged
Merged
AlexV525 pushed a commit that referenced this issue May 13, 2023
@kuhnroyal
Fix return type of OnHttpClientCreate (#1812)
690896a 
The return type of `OnHttpClientCreate` has been changed from
`HttpClient?` to `HttpClient`.
While technically a breaking change, it is considered a bug because the
callback was actually
a no-op when the provided client was modified but not returned.

Fixes 1. of #1811

### New Pull Request Checklist
- [x] I have read the
[Documentation](https://pub.dev/documentation/dio/latest/)
- [x] I have searched for a similar pull request in the
[project](https://github.com/cfug/dio/pulls) and found none
- [x] I have updated this branch with the latest `main` branch to avoid
conflicts (via merge from master or rebase)
- [ ] I have added the required tests to prove the fix/feature I'm
adding
- [x] I have updated the documentation (if necessary)
- [x] I have run the tests without failures
- [x] I have updated the `CHANGELOG.md` in the corresponding package
@AlexV525 AlexV525 mentioned this issue May 15, 2023
Merged
7 tasks
@AlexV525 AlexV525 added the fixed label Jun 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kuhnroyal kuhnroyal

Labels
fixed s: feature This issue indicates a feature request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Abort HttpRequests when CancelToken is cancelled kuhnroyal/dio
2 participants
@kuhnroyal @AlexV525