Rationalise error handling at the network layer (#5100)

packages/flutter/lib/src/http/http.dart

@@ -14,9 +14,13 @@ import 'response.dart';
 /// Sends an HTTP HEAD request with the given headers to the given URL, which
 /// can be a [Uri] or a [String].
 ///
+/// Network errors will be turned into [Response] object with a non-null
+/// [Response.error] field.
+///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<Response> head(dynamic url) {
   return _withClient/*<Response>*/((MojoClient client) => client.head(url));
 }
@@ -24,9 +28,13 @@ Future<Response> head(dynamic url) {
 /// Sends an HTTP GET request with the given headers to the given URL, which can
 /// be a [Uri] or a [String].
 ///
+/// Network errors will be turned into [Response] object with a non-null
+/// [Response.error] field.
+///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<Response> get(dynamic url, { Map<String, String> headers }) {
   return _withClient/*<Response>*/((MojoClient client) => client.get(url, headers: headers));
 }
@@ -48,9 +56,13 @@ Future<Response> get(dynamic url, { Map<String, String> headers }) {
 ///
 /// [encoding] defaults to [UTF8].
 ///
+/// Network errors will be turned into [Response] object with a non-null
+/// [Response.error] field.
+///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<Response> post(dynamic url, { Map<String, String> headers, dynamic body, Encoding encoding: UTF8 }) {
   return _withClient/*<Response>*/((MojoClient client) {
     return client.post(url, headers: headers, body: body, encoding: encoding);
@@ -74,9 +86,13 @@ Future<Response> post(dynamic url, { Map<String, String> headers, dynamic body,
 ///
 /// [encoding] defaults to [UTF8].
 ///
+/// Network errors will be turned into [Response] object with a non-null
+/// [Response.error] field.
+///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<Response> put(dynamic url, { Map<String, String> headers, dynamic body, Encoding encoding: UTF8 }) {
   return _withClient/*<Response>*/((MojoClient client) {
     return client.put(url, headers: headers, body: body, encoding: encoding);
@@ -100,9 +116,13 @@ Future<Response> put(dynamic url, { Map<String, String> headers, dynamic body, E
 ///
 /// [encoding] defaults to [UTF8].
 ///
+/// Network errors will be turned into [Response] object with a non-null
+/// [Response.error] field.
+///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<Response> patch(dynamic url, { Map<String, String> headers, dynamic body, Encoding encoding: UTF8 }) {
   return _withClient/*<Response>*/((MojoClient client) {
      return client.patch(url, headers: headers, body: body, encoding: encoding);
@@ -112,9 +132,13 @@ Future<Response> patch(dynamic url, { Map<String, String> headers, dynamic body,
 /// Sends an HTTP DELETE request with the given headers to the given URL, which
 /// can be a [Uri] or a [String].
 ///
+/// Network errors will be turned into [Response] object with a non-null
+/// [Response.error] field.
+///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<Response> delete(dynamic url, { Map<String, String> headers }) {
   return _withClient/*<Response>*/((MojoClient client) => client.delete(url, headers: headers));
 }
@@ -123,12 +147,13 @@ Future<Response> delete(dynamic url, { Map<String, String> headers }) {
 /// be a [Uri] or a [String], and returns a Future that completes to the body of
 /// the response as a [String].
 ///
-/// The Future will emit a [ClientException] if the response doesn't have a
-/// success status code.
+/// The Future will resolve with an error in the case of a network error or if
+/// the response doesn't have a success status code.
 ///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<String> read(dynamic url, { Map<String, String> headers }) {
   return _withClient/*<String>*/((MojoClient client) => client.read(url, headers: headers));
 }
@@ -137,12 +162,13 @@ Future<String> read(dynamic url, { Map<String, String> headers }) {
 /// be a [Uri] or a [String], and returns a Future that completes to the body of
 /// the response as a list of bytes.
 ///
-/// The Future will emit a [ClientException] if the response doesn't have a
-/// success status code.
+/// The Future will resolve with an error in the case of a network error or if
+/// the response doesn't have a success status code.
 ///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<Uint8List> readBytes(dynamic url, { Map<String, String> headers }) {
   return _withClient/*<Uint8List>*/((MojoClient client) => client.readBytes(url, headers: headers));
 }
@@ -151,12 +177,13 @@ Future<Uint8List> readBytes(dynamic url, { Map<String, String> headers }) {
 /// be a [Uri] or a [String], and returns a Future that completes to a data pipe
 /// containing the response bytes.
 ///
-/// The Future will emit a [ClientException] if the response doesn't have a
-/// success status code.
+/// The Future will resolve with an error in the case of a network error or if
+/// the response doesn't have a success status code.
 ///
 /// This automatically initializes a new [MojoClient] and closes that client once
 /// the request is complete. If you're planning on making multiple requests to
-/// the same server, you should use a single [MojoClient] for all of those requests.
+/// the same server, you should use a single [MojoClient] for all of those requests,
+/// so that the same underlying TCP connection can be re-used (via HTTP pipelining).
 Future<mojo.MojoDataPipeConsumer> readDataPipe(dynamic url, { Map<String, String> headers }) {
   return _withClient/*<mojo.MojoDataPipeConsumer>*/((MojoClient client) => client.readDataPipe(url, headers: headers));
 }

packages/flutter/lib/src/http/mojo_client.dart

@@ -22,14 +22,20 @@ class MojoClient {
 
   /// Sends an HTTP HEAD request with the given headers to the given URL, which
   /// can be a [Uri] or a [String].
+  ///
+  /// Network errors will be turned into [Response] object with a non-null
+  /// [Response.error] field.
   Future<Response> head(dynamic url, { Map<String, String> headers }) {
-    return _send("HEAD", url, headers);
+    return _createResponse(_send("HEAD", url, headers));
   }
 
   /// Sends an HTTP GET request with the given headers to the given URL, which can
   /// be a [Uri] or a [String].
+  ///
+  /// Network errors will be turned into [Response] object with a non-null
+  /// [Response.error] field.
   Future<Response> get(dynamic url, { Map<String, String> headers }) {
-    return _send("GET", url, headers);
+    return _createResponse(_send("GET", url, headers));
   }
 
   /// Sends an HTTP POST request with the given headers and body to the given URL,
@@ -48,8 +54,11 @@ class MojoClient {
   /// `"application/x-www-form-urlencoded"`; this cannot be overridden.
   ///
   /// [encoding] defaults to [UTF8].
+  ///
+  /// Network errors will be turned into [Response] object with a non-null
+  /// [Response.error] field.
   Future<Response> post(dynamic url, { Map<String, String> headers, dynamic body, Encoding encoding: UTF8 }) {
-    return _send("POST", url, headers, body, encoding);
+    return _createResponse(_send("POST", url, headers, body, encoding));
   }
 
   /// Sends an HTTP PUT request with the given headers and body to the given URL,
@@ -68,8 +77,11 @@ class MojoClient {
   /// `"application/x-www-form-urlencoded"`; this cannot be overridden.
   ///
   /// [encoding] defaults to [UTF8].
+  ///
+  /// Network errors will be turned into [Response] object with a non-null
+  /// [Response.error] field.
   Future<Response> put(dynamic url, { Map<String, String> headers, dynamic body, Encoding encoding: UTF8 }) {
-    return _send("PUT", url, headers, body, encoding);
+    return _createResponse(_send("PUT", url, headers, body, encoding));
   }
 
   /// Sends an HTTP PATCH request with the given headers and body to the given
@@ -88,25 +100,31 @@ class MojoClient {
   /// `"application/x-www-form-urlencoded"`; this cannot be overridden.
   ///
   /// [encoding] defaults to [UTF8].
+  ///
+  /// Network errors will be turned into [Response] object with a non-null
+  /// [Response.error] field.
   Future<Response> patch(dynamic url, {Map<String, String> headers, dynamic body, Encoding encoding: UTF8 }) {
-    return _send("PATCH", url, headers, body, encoding);
+    return _createResponse(_send("PATCH", url, headers, body, encoding));
   }
 
   /// Sends an HTTP DELETE request with the given headers to the given URL, which
   /// can be a [Uri] or a [String].
+  ///
+  /// Network errors will be turned into [Response] object with a non-null
+  /// [Response.error] field.
   Future<Response> delete(dynamic url, { Map<String, String> headers }) {
-    return _send("DELETE", url, headers);
+    return _createResponse(_send("DELETE", url, headers));
   }
 
   /// Sends an HTTP GET request with the given headers to the given URL, which can
   /// be a [Uri] or a [String], and returns a Future that completes to the body of
   /// the response as a [String].
   ///
-  /// The Future will emit a [ClientException] if the response doesn't have a
-  /// success status code.
+  /// The Future will resolve with an error in the case of a network error or if
+  /// the response doesn't have a success status code.
   Future<String> read(dynamic url, { Map<String, String> headers }) {
     return get(url, headers: headers).then((Response response) {
-      _checkResponseSuccess(url, response);
+      _requireSuccess(url, response.statusCode, response.error);
       return response.body;
     });
   }
@@ -115,11 +133,11 @@ class MojoClient {
   /// be a [Uri] or a [String], and returns a Future that completes to the body of
   /// the response as a list of bytes.
   ///
-  /// The Future will emit a [ClientException] if the response doesn't have a
-  /// success status code.
+  /// The Future will resolve with an error in the case of a network error or if
+  /// the response doesn't have a success status code.
   Future<Uint8List> readBytes(dynamic url, { Map<String, String> headers }) {
     return get(url, headers: headers).then((Response response) {
-      _checkResponseSuccess(url, response);
+      _requireSuccess(url, response.statusCode, response.error);
       return response.bodyBytes;
     });
   }
@@ -128,28 +146,13 @@ class MojoClient {
   /// be a [Uri] or a [String], and returns a Future that completes to the body of
   /// the response as a [mojo.MojoDataPipeConsumer].
   ///
-  /// The Future will emit a [ClientException] if the response doesn't have a
-  /// success status code.
+  /// The Future will resolve with an error in the case of a network error or if
+  /// the response doesn't have a success status code.
   Future<mojo.MojoDataPipeConsumer> readDataPipe(dynamic url, { Map<String, String> headers }) {
-    Completer<mojo.MojoDataPipeConsumer> completer = new Completer<mojo.MojoDataPipeConsumer>();
-    mojom.UrlLoaderProxy loader = new mojom.UrlLoaderProxy.unbound();
-    networkService.createUrlLoader(loader);
-    loader.start(_prepareRequest('GET', url, headers), (mojom.UrlResponse response) {
-      loader.close();
-      if (response.statusCode < 400) {
-        completer.complete(response.body);
-      } else {
-        Exception exception = new Exception("Request to $url failed with status ${response.statusCode}.");
-        FlutterError.reportError(new FlutterErrorDetails(
-          exception: exception,
-          library: 'networking HTTP library',
-          context: 'while sending bytes to the Mojo network library',
-          silent: true
-        ));
-        completer.completeError(exception);
-      }
+    return _send('GET', url, headers).then((mojom.UrlResponse response) {
+      _requireSuccess(url, response.statusCode, response.statusLine);
+      return response.body;
     });
-    return completer.future;
   }
 
   mojom.UrlRequest _prepareRequest(String method, dynamic url, Map<String, String> headers, [dynamic body, Encoding encoding = UTF8]) {
@@ -175,13 +178,36 @@ class MojoClient {
     return request;
   }
 
-  Future<Response> _send(String method, dynamic url, Map<String, String> headers, [dynamic body, Encoding encoding = UTF8]) {
-    Completer<Response> completer = new Completer<Response>();
+  Future<mojom.UrlResponse> _send(String method, dynamic url, Map<String, String> headers, [dynamic body, Encoding encoding = UTF8]) {
+    Completer<mojom.UrlResponse> completer = new Completer<mojom.UrlResponse>();
     mojom.UrlLoaderProxy loader = new mojom.UrlLoaderProxy.unbound();
     networkService.createUrlLoader(loader);
     mojom.UrlRequest request = _prepareRequest(method, url, headers, body, encoding);
     loader.start(request, (mojom.UrlResponse response) async {
       loader.close();
+      try {
+        if (response.error != null)
+          throw new Exception('Request to "$url" failed with error ${response.error.code}.\n${response.error.description}');
+        if (!response.body.handle.isValid)
+          throw new Exception('Response body does not have a valid handle, but no error was reported.\n${response.body}');
+        completer.complete(response);
+      } catch (e, stack) {
+        FlutterError.reportError(new FlutterErrorDetails(
+          exception: e,
+          stack: stack,
+          library: 'networking HTTP library',
+          context: 'while interacting with the Mojo network library',
+          silent: true
+        ));
+        completer.completeError(e);
+      }
+    });
+    return completer.future;
+  }
+
+  Future<Response> _createResponse(Future<mojom.UrlResponse> futureResponse) async {
+    try {
+      mojom.UrlResponse response = await futureResponse;
       try {
         ByteData data = await mojo.DataPipeDrainer.drainHandle(response.body);
         Uint8List bodyBytes = new Uint8List.view(data.buffer);
@@ -193,25 +219,36 @@ class MojoClient {
             headers[headerName] = existingValue != null ? '$existingValue, ${header.value}' : header.value;
           }
         }
-        completer.complete(new Response.bytes(bodyBytes, response.statusCode, headers: headers));
-      } catch (exception, stack) {
+        return new Response.bytes(bodyBytes, response.statusCode, headers: headers, error: response.statusLine);
+      } catch (e, stack) {
         FlutterError.reportError(new FlutterErrorDetails(
-          exception: exception,
+          exception: e,
           stack: stack,
           library: 'networking HTTP library',
-          context: 'while sending bytes to the Mojo network library',
+          context: 'while interacting with the Mojo network library',
           silent: true
         ));
-        completer.complete(new Response.bytes(null, 500));
+        rethrow;
       }
-    });
-    return completer.future;
+    } catch (e) {
+      return new Response.bytes(null, 500, error: e);
+    }
   }
 
-  void _checkResponseSuccess(dynamic url, Response response) {
-    if (response.statusCode < 400)
-      return;
-    throw new Exception("Request to $url failed with status ${response.statusCode}.");
+  void _requireSuccess(dynamic url, int statusCode, dynamic error) {
+    if (error is Exception)
+      throw error;
+    if (statusCode >= 400) {
+      String extra;
+      if (error is String && error != '') {
+        extra = '\nServer response: "$error"';
+      } else if (error != null) {
+        extra = '\n$error';
+      } else {
+        extra = '';
+      }
+      throw new Exception('Request to "$url" failed with status $statusCode.$extra');
+    }
   }
 
   static mojom.NetworkServiceProxy _initNetworkService() {

packages/flutter/lib/src/http/response.dart

@@ -10,9 +10,15 @@ class Response {
   /// Creates a [Response] object with the given fields.
   ///
   /// If [bodyBytes] is non-null, it is used to populate [body].
-  Response.bytes(this.bodyBytes, this.statusCode, { this.headers: const <String, String>{} });
+  Response.bytes(this.bodyBytes, this.statusCode, {
+    this.headers: const <String, String>{},
+    this.error
+  });
 
-  /// The result of decoding [bodyBytes] using ISO-8859-1.
+  /// The result of decoding [bodyBytes] using the character encoding declared
+  /// in the headers.
+  ///
+  /// Defaults to [LATIN1] (ISO 8859-1).
   ///
   /// If [bodyBytes] is null, this will also be null.
   String get body => bodyBytes == null ? null : _encodingForHeaders(headers).decode(bodyBytes);
@@ -25,6 +31,12 @@ class Response {
   /// The code 500 is used when no status code could be obtained from the host.
   final int statusCode;
 
+  /// Error information, if any. This may be populated if the [statusCode] is
+  /// 4xx or 5xx. This may be a string (e.g. the status line from the server) or
+  /// an [Exception], but in either case the object should have a useful
+  /// [toString] implementation that returns a human-readable value.
+  final dynamic error;
+
   /// The headers for this response.
   final Map<String, String> headers;
 }

packages/flutter/lib/src/services/asset_bundle.dart

@@ -4,6 +4,7 @@
 
 import 'dart:async';
 import 'dart:convert';
+import 'dart:io';
 import 'dart:ui' as ui;
 import 'dart:typed_data';
 
@@ -191,8 +192,26 @@ class MojoAssetBundle extends CachingAssetBundle {
 
 AssetBundle _initRootBundle() {
   int h = ui.MojoServices.takeRootBundle();
-  if (h == core.MojoHandle.INVALID)
+  if (h == core.MojoHandle.INVALID) {
+    assert(() {
+      if (!Platform.environment.containsKey('FLUTTER_TEST')) {
+        FlutterError.reportError(new FlutterErrorDetails(
+          exception:
+            'dart:ui MojoServices.takeRootBundle() returned an invalid handle.\n'
+            'This might happen if the Dart VM was restarted without restarting the underlying Flutter engine, '
+            'or if the Flutter framework\'s rootBundle object was first accessed after some other code called '
+            'takeRootBundle. The root bundle handle can only be obtained once in the lifetime of the Flutter '
+            'engine. Mojo handles cannot be shared.\n'
+            'The rootBundle object will be initialised with a NetworkAssetBundle instead of a MojoAssetBundle. '
+            'This may cause subsequent network errors.',
+          library: 'services library',
+          context: 'while initialising the root bundle'
+        ));
+      }
+      return true;
+    });
     return new NetworkAssetBundle(Uri.base);
+  }
   core.MojoHandle handle = new core.MojoHandle(h);
   return new MojoAssetBundle(new mojom.AssetBundleProxy.fromHandle(handle));
 }

packages/flutter/lib/src/services/image_provider.dart

@@ -221,7 +221,13 @@ abstract class DataPipeImageProvider<T> extends ImageProvider<T> {
 
   @override
   ImageStreamCompleter load(T key) {
-    return new OneFrameImageStreamCompleter(_loadAsync(key));
+    return new OneFrameImageStreamCompleter(
+      _loadAsync(key),
+      informationCollector: (StringBuffer information) {
+        information.writeln('Image provider: $this');
+        information.write('Image key: $key');
+      }
+    );
   }
 
   Future<ImageInfo> _loadAsync(T key) async {

packages/flutter/lib/src/services/image_stream.dart

@@ -223,8 +223,25 @@ class OneFrameImageStreamCompleter extends ImageStreamCompleter {
   /// The image resource awaits the given [Future]. When the future resolves,
   /// it notifies the [ImageListener]s that have been registered with
   /// [addListener].
-  OneFrameImageStreamCompleter(Future<ImageInfo> image) {
+  ///
+  /// The [InformationCollector], if provided, is invoked if the given [Future]
+  /// resolves with an error, and can be used to supplement the reported error
+  /// message (for example, giving the image's URL).
+  ///
+  /// Errors are reported using [FlutterError.reportError] with the `silent`
+  /// argument on [FlutterErrorDetails] set to true, meaning that by default the
+  /// message is only dumped to the console in debug mode (see [new
+  /// FlutterErrorDetails]).
+  OneFrameImageStreamCompleter(Future<ImageInfo> image, { InformationCollector informationCollector }) {
     assert(image != null);
-    image.then(setImage);
+    image.then(setImage, onError: (dynamic error, StackTrace stack) {
+      FlutterError.reportError(new FlutterErrorDetails(
+        exception: error,
+        stack: stack,
+        library: 'services',
+        context: 'resolving a single-frame image stream',
+        informationCollector: informationCollector
+      ));
+    });
   }
 }