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

Configure HttpClient #788

Open
1 of 3 tasks
Leptopoda opened this issue Sep 18, 2023 · 11 comments
Open
1 of 3 tasks

Configure HttpClient #788

Leptopoda opened this issue Sep 18, 2023 · 11 comments
Labels

Comments

@Leptopoda
Copy link
Member

Leptopoda commented Sep 18, 2023

  • add follow redirects (discuss the security implications!)
  • add keepalive (persistent connection)
  • investigae supporting http2
@Leptopoda Leptopoda added feature New feature or request package: dynamite labels Sep 18, 2023
@Leptopoda
Copy link
Member Author

Looks like the persistentConnection is only a header indicating whether a persistent connection is kept.
The property to change would be idleTimeout wich already defaults to a reasonable 15 seconds.

I think this is enough but @provokateurin do cou think we should raise it nevertheless?

https://api.flutter.dev/flutter/dart-io/HttpClient/idleTimeout.html
https://api.flutter.dev/flutter/dart-io/HttpHeaders/persistentConnection.html

@provokateurin
Copy link
Member

I think that is enough, we also don't want people to wait forever while the network is not working at all.

The other points should still be investigated.

@Leptopoda
Copy link
Member Author

Leptopoda commented Dec 7, 2023

While a dart only http2 package exists the dart team does currently not have the bandwidth to support http3 (dart-lang/sdk#38595).

They instead link to the in development cupertino_http and cronet_http packages. They both interact with the native networking stack advertising easier working with a VPN, system wide proxy and whatnot.
As they both provide the dart:io HttpClient interface it should be an easy drop in replacement.

Downsides:

  1. cronet_http by default calls the cronet lib packaged by the play services.
  2. When using a packaged cronet version we'll inflate the apk size (imo this is worth it). The bundled version is currently not set up to the dart teams CI so the package is outdated. Somebody is working on it and it looks like the PR is quite active.
  3. The cronet package does currently not advertise a stable api (I don't think this is an issue especially as we'd only use the HttpClient interface)
  4. windows and linux miss out on this for now :(

@provokateurin
Copy link
Member

Shall we switch to cronet_http_embedded now?

@Leptopoda
Copy link
Member Author

In theory yes but we should not rush it.

  1. the embedded version is probably going to be deprecated in the long run while the normal version is going to take a compile time option to distinguish between both. Not a stopper for us as migration would be straight forward.

  2. With cronet using ffi to communicate to the native lib we loose the benefits of streams. While they expose streaming interfaces all calls are collected into a single buffer copied over to the native code and then sent (similar for receiving). This would not be a big issue for the api requests but with large webdav files we might hit memory limitations.
    Don't quote me on this point as I haven't had the time to dig into this deeper and do actual tests and I might have misunderstood some code I read a few weeks ago.

@provokateurin
Copy link
Member

Oh, no streams support is kind of a deal breaker. Users should be able to download a 4GB file on a device with only 2GB RAM.

@Leptopoda
Copy link
Member Author

to my knowledge webdav isn't really benefiting from http2 or http3 as there probably aren't many concurrent dowloads ever happening.
We could just use dart:io client for webdav.

@Leptopoda
Copy link
Member Author

Moving to the neon_framework as dynamite should accept any client

@provokateurin
Copy link
Member

Anything that still needs to be done?

@Leptopoda
Copy link
Member Author

I still want to use a client that supports http2 or even http3.
On Android my favorite would be the cronet client, but that either relies on the play services or bundling it ourselves would increase the apk size by roughly 14MB.
A different approach would be the ok_http client. It only supports http2 but doesn't bloat the apk size (https://github.com/dart-lang/http/tree/master/pkgs/ok_http).

@provokateurin
Copy link
Member

Yeah GPlay or 14 MB are not great options, let's do ok_http then.
How about closing this issue and just creating a new one for ok_http to decrease the overall scope?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants