Switch browser_client.dart to use fetch (#1401)

Issue #595

Author: mraleph

pkgs/http/CHANGELOG.md

@@ -1,6 +1,7 @@
-## 1.2.3-wip
+## 1.3.0-wip
 
-* Fixed unintended HTML tags in doc comments. 
+* Fixed unintended HTML tags in doc comments.
+* Switched `BrowserClient` to use Fetch API instead of `XMLHttpRequest`.
 
 ## 1.2.2
 

pkgs/http/lib/src/browser_client.dart

@@ -5,16 +5,20 @@
 import 'dart:async';
 import 'dart:js_interop';
 
-import 'package:web/web.dart' show XHRGetters, XMLHttpRequest;
+import 'package:web/web.dart'
+    show
+        AbortController,
+        HeadersInit,
+        ReadableStreamDefaultReader,
+        RequestInit,
+        Response,
+        window;
 
 import 'base_client.dart';
 import 'base_request.dart';
-import 'byte_stream.dart';
 import 'exception.dart';
 import 'streamed_response.dart';
 
-final _digitRegex = RegExp(r'^\d+$');
-
 /// Create a [BrowserClient].
 ///
 /// Used from conditional imports, matches the definition in `client_stub.dart`.
@@ -27,18 +31,19 @@ BaseClient createClient() {
 }
 
 /// A `package:web`-based HTTP client that runs in the browser and is backed by
-/// [XMLHttpRequest].
+/// [`window.fetch`](https://fetch.spec.whatwg.org/).
+///
+/// This client inherits some limitations of `window.fetch`:
+///
+/// - [BaseRequest.persistentConnection] is ignored;
+/// - Setting [BaseRequest.followRedirects] to `false` will cause
+///   [ClientException] when a redirect is encountered;
+/// - The value of [BaseRequest.maxRedirects] is ignored.
 ///
-/// This client inherits some of the limitations of XMLHttpRequest. It ignores
-/// the [BaseRequest.contentLength], [BaseRequest.persistentConnection],
-/// [BaseRequest.followRedirects], and [BaseRequest.maxRedirects] fields. It is
-/// also unable to stream requests or responses; a request will only be sent and
-/// a response will only be returned once all the data is available.
+/// Responses are streamed but requests are not. A request will only be sent
+/// once all the data is available.
 class BrowserClient extends BaseClient {
-  /// The currently active XHRs.
-  ///
-  /// These are aborted if the client is closed.
-  final _xhrs = <XMLHttpRequest>{};
+  final _abortController = AbortController();
 
   /// Whether to send credentials such as cookies or authorization headers for
   /// cross-site requests.
@@ -55,55 +60,58 @@ class BrowserClient extends BaseClient {
       throw ClientException(
           'HTTP request failed. Client is already closed.', request.url);
     }
-    var bytes = await request.finalize().toBytes();
-    var xhr = XMLHttpRequest();
-    _xhrs.add(xhr);
-    xhr
-      ..open(request.method, '${request.url}', true)
-      ..responseType = 'arraybuffer'
-      ..withCredentials = withCredentials;
-    for (var header in request.headers.entries) {
-      xhr.setRequestHeader(header.key, header.value);
-    }
 
-    var completer = Completer<StreamedResponse>();
-
-    unawaited(xhr.onLoad.first.then((_) {
-      if (xhr.responseHeaders['content-length'] case final contentLengthHeader
-          when contentLengthHeader != null &&
-              !_digitRegex.hasMatch(contentLengthHeader)) {
-        completer.completeError(ClientException(
+    final bodyBytes = await request.finalize().toBytes();
+    try {
+      final response = await window
+          .fetch(
+            '${request.url}'.toJS,
+            RequestInit(
+              method: request.method,
+              body: bodyBytes.isNotEmpty ? bodyBytes.toJS : null,
+              credentials: withCredentials ? 'include' : 'same-origin',
+              headers: {
+                if (request.contentLength case final contentLength?)
+                  'content-length': contentLength,
+                for (var header in request.headers.entries)
+                  header.key: header.value,
+              }.jsify()! as HeadersInit,
+              signal: _abortController.signal,
+              redirect: request.followRedirects ? 'follow' : 'error',
+            ),
+          )
+          .toDart;
+
+      final contentLengthHeader = response.headers.get('content-length');
+
+      final contentLength = contentLengthHeader != null
+          ? int.tryParse(contentLengthHeader)
+          : null;
+
+      if (contentLength == null && contentLengthHeader != null) {
+        throw ClientException(
           'Invalid content-length header [$contentLengthHeader].',
           request.url,
-        ));
-        return;
+        );
       }
-      var body = (xhr.response as JSArrayBuffer).toDart.asUint8List();
-      var responseUrl = xhr.responseURL;
-      var url = responseUrl.isNotEmpty ? Uri.parse(responseUrl) : request.url;
-      completer.complete(StreamedResponseV2(
-          ByteStream.fromBytes(body), xhr.status,
-          contentLength: body.length,
-          request: request,
-          url: url,
-          headers: xhr.responseHeaders,
-          reasonPhrase: xhr.statusText));
-    }));
-
-    unawaited(xhr.onError.first.then((_) {
-      // Unfortunately, the underlying XMLHttpRequest API doesn't expose any
-      // specific information about the error itself.
-      completer.completeError(
-          ClientException('XMLHttpRequest error.', request.url),
-          StackTrace.current);
-    }));
-
-    xhr.send(bytes.toJS);
 
-    try {
-      return await completer.future;
-    } finally {
-      _xhrs.remove(xhr);
+      final headers = <String, String>{};
+      (response.headers as _IterableHeaders)
+          .forEach((String value, String header, [JSAny? _]) {
+        headers[header.toLowerCase()] = value;
+      }.toJS);
+
+      return StreamedResponseV2(
+        _readBody(request, response),
+        response.status,
+        headers: headers,
+        request: request,
+        contentLength: contentLength,
+        url: Uri.parse(response.url),
+        reasonPhrase: response.statusText,
+      );
+    } catch (e, st) {
+      _rethrowAsClientException(e, st, request);
     }
   }
 
@@ -113,36 +121,66 @@ class BrowserClient extends BaseClient {
   @override
   void close() {
     _isClosed = true;
-    for (var xhr in _xhrs) {
-      xhr.abort();
+    _abortController.abort();
+  }
+}
+
+Never _rethrowAsClientException(Object e, StackTrace st, BaseRequest request) {
+  if (e is! ClientException) {
+    var message = e.toString();
+    if (message.startsWith('TypeError: ')) {
+      message = message.substring('TypeError: '.length);
     }
-    _xhrs.clear();
+    e = ClientException(message, request.url);
   }
+  Error.throwWithStackTrace(e, st);
 }
 
-extension on XMLHttpRequest {
-  Map<String, String> get responseHeaders {
-    // from Closure's goog.net.Xhrio.getResponseHeaders.
-    var headers = <String, String>{};
-    var headersString = getAllResponseHeaders();
-    var headersList = headersString.split('\r\n');
-    for (var header in headersList) {
-      if (header.isEmpty) {
-        continue;
-      }
+Stream<List<int>> _readBody(BaseRequest request, Response response) async* {
+  final bodyStreamReader =
+      response.body?.getReader() as ReadableStreamDefaultReader?;
+
+  if (bodyStreamReader == null) {
+    return;
+  }
 
-      var splitIdx = header.indexOf(': ');
-      if (splitIdx == -1) {
-        continue;
+  var isDone = false, isError = false;
+  try {
+    while (true) {
+      final chunk = await bodyStreamReader.read().toDart;
+      if (chunk.done) {
+        isDone = true;
+        break;
       }
-      var key = header.substring(0, splitIdx).toLowerCase();
-      var value = header.substring(splitIdx + 2);
-      if (headers.containsKey(key)) {
-        headers[key] = '${headers[key]}, $value';
-      } else {
-        headers[key] = value;
+      yield (chunk.value! as JSUint8Array).toDart;
+    }
+  } catch (e, st) {
+    isError = true;
+    _rethrowAsClientException(e, st, request);
+  } finally {
+    if (!isDone) {
+      try {
+        // catchError here is a temporary workaround for
+        // http://dartbug.com/57046: an exception from cancel() will
+        // clobber an exception which is currently in flight.
+        await bodyStreamReader
+            .cancel()
+            .toDart
+            .catchError((_) => null, test: (_) => isError);
+      } catch (e, st) {
+        // If we have already encountered an error swallow the
+        // error from cancel and simply let the original error to be
+        // rethrown.
+        if (!isError) {
+          _rethrowAsClientException(e, st, request);
+        }
       }
     }
-    return headers;
   }
 }
+
+/// Workaround for `Headers` not providing a way to iterate the headers.
+@JS()
+extension type _IterableHeaders._(JSObject _) implements JSObject {
+  external void forEach(JSFunction fn);
+}

pkgs/http/pubspec.yaml

@@ -1,5 +1,5 @@
 name: http
-version: 1.2.3-wip
+version: 1.3.0-wip
 description: A composable, multi-platform, Future-based API for HTTP requests.
 repository: https://github.com/dart-lang/http/tree/master/pkgs/http
 

pkgs/http/test/html/client_conformance_test.dart

@@ -13,7 +13,7 @@ void main() {
   testAll(BrowserClient.new,
       redirectAlwaysAllowed: true,
       canStreamRequestBody: false,
-      canStreamResponseBody: false,
+      canStreamResponseBody: true,
       canWorkInIsolates: false,
       supportsMultipartRequest: false);
 }

pkgs/http/test/html/client_test.dart

@@ -34,8 +34,7 @@ void main() {
     var url = Uri.http('http.invalid', '');
     var request = http.StreamedRequest('POST', url);
 
-    expect(
-        client.send(request), throwsClientException('XMLHttpRequest error.'));
+    expect(client.send(request), throwsClientException());
 
     request.sink.add('{"hello": "world"}'.codeUnits);
     request.sink.close();