http icon indicating copy to clipboard operation
http copied to clipboard

Migrate to the `fetch` API

Open natebosch opened this issue 2 years ago • 15 comments

dart2js will stop supporting internet explorer soon - so all supported browser should support fetch which allows for streaming. I'm not sure if streaming works on firefox so that will need to be tested.

https://api.dart.dev/stable/2.13.4/dart-html/Window/fetch.html

https://github.com/dart-lang/http/blob/f93c76fabdb03963303762e3fe16cbdf60799cff/lib/src/browser_client.dart#L27-L29

natebosch avatar Jul 15 '21 19:07 natebosch

How about this since one year past?

pedia avatar Apr 11 '22 10:04 pedia

This would be a good feature request for us. Happy to take a look and see how well it maps.

jtmcdole avatar Apr 14 '22 02:04 jtmcdole

Example of using fetch in Dart: https://github.com/dart-lang/dartdoc/blob/f0d2cbf5f517d936d01a61e7350147e3ad12edb9/web/search.dart#L35

natebosch avatar Apr 24 '22 00:04 natebosch

We need cross all platform fetch. e.g. long polling http request, come from here It's called from package:http/http.

pedia avatar Apr 26 '22 04:04 pedia

Looks like we need to fallback on websockets. Flutter web's http (BrowserClient) has no streaming capability. EventSources have their own issues (no http headers, for starters). That's a huge minus. Fetch, QUIC, web-transport, http3 are still years away. websockets it is.

srix55 avatar Sep 28 '22 06:09 srix55

Looks like we need to fallback on websockets. Flutter web's http (BrowserClient) has no streaming capability. EventSources have their own issues (no http headers, for starters). That's a huge minus. Fetch, QUIC, web-transport, http3 are still years away. websockets it is.

what about web-rtc?

jtmcdole avatar Sep 30 '22 23:09 jtmcdole

Bump this thread. There is no XHR in Chrome manifest v3 service workers so fetch must be used there instead of XHR. https://developer.chrome.com/docs/extensions/mv3/intro/mv3-migration/#background-service-workers

kaoet avatar Oct 10 '22 02:10 kaoet

Is there any chance of this happening any time soon, or what is the status? Are there any other reasons to keep using XHR now that IE is dead?

momrak avatar Dec 15 '22 13:12 momrak

I've come across the issue that XHR missing information about redirects. I've made fetch binding and custom Client on userland, so you can use my fetch_client as a workaround.

Note: requires Dart 2.19, or someone to help backport bindings to previous versions.

Zekfad avatar Jan 22 '23 21:01 Zekfad

Any updates on this?

Deishelon avatar Feb 02 '23 08:02 Deishelon

I came across this issue myself, because I needed a streamed response on web. Since an implementation did not exist, I implemented it myself:

Implementation
import 'dart:async' show Future, StreamController, unawaited;
import 'dart:html' show window;
import 'dart:js_util' show callConstructor, getProperty, globalThis, jsify;
import 'dart:typed_data' show Uint8List;

import 'package:http/http.dart' as http;
import 'package:js/js.dart' as js;
import 'package:js_bindings/bindings/dom.dart';
import 'package:js_bindings/bindings/fetch.dart';
import 'package:js_bindings/bindings/streams.dart';

/// https://developer.chrome.com/articles/fetch-streaming-requests/
class StreamedBrowserClient extends http.BaseClient {
  /// The currently active requests.
  ///
  /// These are aborted if the client is closed.
  final _abortControllers = <AbortController>{};

  /// Whether to send credentials such as cookies or authorization headers for
  /// cross-site requests.
  ///
  /// Defaults to `false`.
  bool withCredentials = false;

  @override
  Future<http.StreamedResponse> send(http.BaseRequest request) async {
    var bytes = request.finalize();

    final abortController = AbortController();

    _abortControllers.add(abortController);

    try {
      // For some reason, just calling `ReadableStream()` didn't work in debug mode, but worked fine, when compiled??
      final ReadableStream nativeStream =
          callConstructor(getProperty(globalThis, "ReadableStream"), [
        jsify({
          "start": js.allowInterop((ReadableStreamDefaultController controller) {
            return _futureToPromise(Future(() async {
              await for (var chunk in bytes) {
                controller.enqueue(Uint8List.fromList(chunk));
              }
              controller.close();
            }));
          })
        }),
      ]);

      final Response response = await window.fetch(
        request.url.toString(),
        {
          if (request.method != "GET" && request.method != "HEAD") "body": nativeStream,
          "method": request.method,
          "headers": request.headers,
          "credentials":
              (withCredentials ? RequestCredentials.include : RequestCredentials.sameOrigin).value,
          "signal": abortController.signal,
          "duplex": "half",
        },
      );

      final ReadableStreamDefaultReader reader = response.body!.getReader();

      final controller = StreamController<List<int>>();

      unawaited(Future(() async {
        while (true) {
          ReadableStreamReadResult result = await reader.read();
          if (result.done) {
            controller.close();
            _abortControllers.remove(abortController);
            break;
          }
          controller.add(result.value);
        }
      }));

      return http.StreamedResponse(controller.stream, response.status);
    } catch (e) {
      _abortControllers.remove(abortController);
      throw http.ClientException(e.toString(), request.url);
    }
  }

  /// Closes the client.
  ///
  /// This terminates all active requests.
  @override
  void close() {
    super.close();
    for (final controller in _abortControllers) {
      controller.abort();
    }
  }
}

Promise<T> _futureToPromise<T>(Future<T> future) {
  return Promise(js.allowInterop((Function resolve, Function reject) {
    future.then((result) => resolve(result), onError: reject);
  }));
}

@js.JS()
abstract class Promise<T> {
  external factory Promise(
      void Function(void Function(T result) resolve, Function reject) executor);
  external Promise then(void Function(T result) onFulfilled, [Function onRejected]);
}

I used the awesome js_bindings package for many js class bindings, but One might consider creating dedicated bindings for it, or even adding them to dart:html

Dampfwalze avatar Apr 17 '23 12:04 Dampfwalze

Is there any reason why we shouldn't just point people to fetch_client?

brianquinlan avatar Jul 18 '23 21:07 brianquinlan

I see no reason not to suggest an alternative implementation. I don't know if we should only do that - I expect that users would benefit from fetch being the default implementation in this package.

natebosch avatar Jul 18 '23 22:07 natebosch

@natebosch is right. the http package exists to abstract the platform layer. Right now we have code that uses flutter that only breaks on web because the web implementation of the http package is coded against IE 5.5 APIs. Of course we could just rewrite our code, but the issue isn't that we aren't capable of writing platform specific code using the fetch client, the issue is that the whole point of the http package is to prevent us from having to do that.

jezell avatar Oct 06 '23 18:10 jezell

package:fetch_client implements the same Client interface as package:http so, aside from changing your pubspec.yaml and a few lines of configuration, your application should not have to change.

brianquinlan avatar Oct 26 '23 22:10 brianquinlan

How in the world this still open in 2024 @kevmoo @mraleph? Come on guys.

jezell avatar Apr 15 '24 07:04 jezell

Also note that fetch_client doesn't actually solve the problem "aside from changing your pubspec.yaml and a few lines of configuration, your application should not have to change" is just plain false. If you actually use StreamingRequest, you'll notice that fetch_client doesn't work properly at all. For example it sets keepAlive: true when you try to stream requests and fails unless you make more changes to your code specifically to use the package. Also, the package is published by "unverified uploader" on pub.dev, so Google telling everyone to use some random package by an unverified publisher instead of the built it one is an xz style vulnerability waiting to happen.

jezell avatar Apr 15 '24 09:04 jezell

@jezell, just to clarify, keepAlive: true mirrors persistentConnection which is true by default, XHR not supporting it doesn't mean fetch client is wrongly made. As for "unverified uploader" I'm thinking of moving it to a publisher, but ultimately it doesn't make a difference, because anyone can register domain and package uploads are tied to Google accounts anyways. As for xz supply chain attack - no one is safe from it, in theory some evil genius can work in Google and just do the same deed. If you use OSS you should check the updates (pub pins versions unless you upgrade the deps) or you're asserting the trust without checking. Apart from that I also think that fetch should have a default/standard implementation without resorting for a 3rd party package.

Zekfad avatar Apr 15 '24 16:04 Zekfad

@Zekfad point is the fetch_client is not really a drop in replacement. Don't mean to say it's a bad package, just that it's not just a matter of changing some package ref. Code that works fine with built in http clients on iOS and Web doesn't work at all in fetch_client if you want streaming. For instance, if you try to make a StreamedRequest the same way you would on iOS or Web (even though the built in web implementation is a lie about streaming), it will blow up with an exception immediately due to setting the keepAlive to true. If you set it to "stream requests" in the constructor, it will stop blowing up, but it won't send your POST bodies at all anymore. So you have to go tweak your code to get it to work right. Should be just built in so all the same tests can be run against all the clients and no one has to worry about subtle differences in the implementations, or their apps randomly blowing up because the built in package doesn't do things properly for no good reason.

jezell avatar Apr 15 '24 17:04 jezell

@jezell request streaming is very experimental in web browsers (Chromium based only as of now). But that's a good catch, I'll repeat warning in class description and not only in property. In default node StreamedRequest should work with persistentConnection = false without a problems. Native client will stream it indeed, but fetch will finalize request to single blob and then send it like XHR does. So setting persistentConnection to false in requests should make it a almost drop-in replacement, platforms are too different in capabilities, so you'll still have to accommodate for special cases (e.g. streaming).

Zekfad avatar Apr 15 '24 20:04 Zekfad

Using streaming in supabase-flutter for web is a nightmare at the moment. Now there is a PR fixing streaming in supabase-flutter working on all platforms but Web, as dart-lang/http is not implementing fetch. This is really a big problem for many using supabase-flutter for web. I can't even just replace the http-client with another 3rd party library as this would need to be implemented by the supabase team.

It would really be important to have this feature directly in http itself.

nietsmmar avatar Apr 26 '24 19:04 nietsmmar

I can't even just replace the http-client with another 3rd party library as this would need to be implemented by the supabase team.

We do intend for it to be low-change to code using package:http to swap implementations out, even without inversion of control patterns.

https://github.com/dart-lang/http/tree/master/pkgs/http#3-connect-the-http-client-to-the-code-that-uses-it

We have a runWithClient API which uses zones to override the default client implementation.

natebosch avatar Apr 26 '24 20:04 natebosch

We have a runWithClient API which uses zones to override the default client implementation.

Is there more documentation on how to use this? The link to runWithClient gives me 404 error. I am not sure how to make sure that if my app is run in web it uses fetch_client instead of normal http in case I can't pass the Client explicitly through arguments.

nietsmmar avatar Apr 26 '24 20:04 nietsmmar

Is there more documentation on how to use this?

https://pub.dev/documentation/http/latest/http/runWithClient.html

The link to runWithClient gives me 404 error.

Sorry for the broken link, looks like a syntax error. I'll fix it in https://github.com/dart-lang/http/pull/1184

natebosch avatar Apr 26 '24 21:04 natebosch

@nietsmmar streaming is just plain broken in dart on web. you cannot stream. You cannot make large file uploads. I don't understand how the team thinks this is acceptable.

jezell avatar Apr 27 '24 05:04 jezell

@jezell Do you have a suggestion for a technical approach to make this work?

According to the MDN compatibility chart, ReadableStream is not supported in Safari or Firefox. This is why package:fetch_client uses streamRequests to control whether to use streaming uploads are used or not. Would you sniff the browser version to determine the supported API service and run the compatibility tests on every supported browser (Google Chrome, Microsoft Edge, Firefox, Apple Safari)?

Likewise, the issue that you raised with keepalive is likely also a product of the APIs poor level or support.

Finally, the Dart Code of Conduct requires that you be courteous and respectful of people's work. Statements like "come on guys" and "I don't understand how the team thinks this is acceptable" are not acceptable ;-)

brianquinlan avatar Jun 10 '24 21:06 brianquinlan

There's a feature detection sample here to detect the presence of streaming requests:

https://developer.chrome.com/docs/capabilities/web-apis/fetch-streaming-requests

I think in general, feature detection is better than hard coding browser versions.

It's not true to say FF doesn't support ReadableStream, it at least supports it for downloads.

Screenshot 2024-06-11 at 3 23 21 PM

I think it would be preferable for that attempts to stream to throw an exception on browsers that don't support streaming saying that the browser doesn't support streaming, rather than fake support and blow up.

The larger issue at play here is that large files can't be uploaded with Flutter web at the moment due to everything being buffered into memory for XmlHttpRequest. Streaming would intuitively be the way you would solve this problem as a developer, and it can solve this problem in Chrome, yet because dart is taking a lowest common denominator approach here you can't stream in the browsers that support it. There's already the non streaming API for browsers that don't support streaming. If streaming was supported, blame could be passed on to FF and Safari in the subset of cases they don't support, and developers could write code like:

try {
  await uploadStream(file);
} catch {
  await uploadNonStream(file);
}

This code could be made robust by the consuming app providing an appropriate message back to the end user about being in a browser that doesn't support large file uploads, instructing them to use another Browser and removing the blame from Flutter/Dart just not supporting large file uploads.

Now raw streaming a byte at a time really isn't what's desired in a lot of cases. What people really want is the ability to push an entire File which fetch does support. Likely the reason FF doesn't support bidirectional streaming here is that most people are just need to upload a File object not a raw stream or download a file. However, generating a stream at runtime from some other source is pretty uncommon.

Let's say I need to upload a 4GB file with the current dart api, what am I to do? Certainly this should be supported, and certainly it can be done with fetch, but at the moment it's impossible. StreamedRequest/StreamedResponse definitely should allow this problem to be supported when they are supported by the browser the app is running in, but what about when they aren't. I still should be able to upload the file. Fetch and upload file directly, but the current implementation is tied to XmlHttpRequest.

These are the types fetch supports according to MDN: Screenshot 2024-06-11 at 3 49 40 PM

These are the types XmlHttpRequest supports according to MDN: Screenshot 2024-06-11 at 3 57 23 PM

Note that XmlHttpRequest lacks support for File, but Fetch does not. Note also that FormData is here for Fetch and due to the other problems the MultipartRequest impl is also going to blow up with 4GB files instead of using FormData on web when used with http. Slightly different problem because multipart mime requests are a little old school, but those are also kneecapped by buffering everything into memory in the current impl rather than using the browser native pathways.

There are a few potential solutions to this problem without changing the API and without relying on StreamedRequest/StreamedResponse. The dart client interface itself supports sending objects just like the browser interface:

Future<Response> post(Uri url,
          {Map<String, String>? headers, Object? body, Encoding? encoding})

Yet this isn't leveraged at all. First off, MultipartRequest / MultiPart file on web should use the native objects and be supported for passthrough so the native objects get used. Second, if I use static interop to get a file object, I should be able to send that the body param here and have it work. browser_client should not assume everything it gets is a request with a byte stream that needs to be finalized and read into memory.

Everyone on web will benefit from the dart web implementation moving to more modern APIs that don't result in very low limits on the amount of data you can send over the wire. At the moment it is very frustrating to deal with media in Flutter on web because of these gaps.

jezell avatar Jun 11 '24 23:06 jezell

@jezell while I understand your point, I think adding file to http package would be a bad decision, because http is designed to be cross platform, and there's no JS File in VM. It's possible to make a hacky union of JS or IO Files via conditional imports, or even use Object as type argument, I think that would be a bad API.

Your use case can be solved by actually using fecth in web, because it's absolutely not http job to provide JS File. What I mean is that you still have to somewhere get JS File object, that is not possible without making platform specific code. Since to get File you anyway need platform code, you can use fetch directly.

Zekfad avatar Jun 11 '24 23:06 Zekfad

I didn't say add File to the http package. File is already in dart:web which is maintained by the dart team already (https://pub.dev/documentation/web/latest/web/web-library.html). I was just pointing out that the API itself can already accept an Object and should work if I pass it the native browser object. There is plenty of precedence for this in dart:ui_web already. It's just a simple fact that in the browser accomplishing some things requires using the native objects. It doesn't require creating a new API, it just requires browser_client.dart to respect the type of the object it is passed instead of blowing up.

Dart should support uploading files that are larger than a 200MB. The fact that it doesn't remains a travesty. I'm just trying to present solutions to the problem.

jezell avatar Jun 11 '24 23:06 jezell

@Zekfad also note that the http package already has MultiPartFile, so the dart team already officially supports a file abstraction within the http package:

https://pub.dev/documentation/http/latest/http/MultipartFile-class.html

My suggestion there was that this should work, since FormData is actually supported in the browser. In this case it's again the browser_client implementation that is the problem, not the high level API being surfaced by dart http. While technically it "works" in a test, in practice multipart uploads are going to hit that upper bound of XmlHttpRequest much sooner, making it not very useful outside of a unit test.

jezell avatar Jun 12 '24 00:06 jezell