Catch errors in loadStructuredData (#130748)

Fixes https://github.com/flutter/flutter/issues/42390

packages/flutter/lib/src/foundation/synchronous_future.dart

@@ -14,6 +14,8 @@ import 'dart:async';
 /// rare occasions you want the ability to switch to an asynchronous model. **In
 /// general use of this class should be avoided as it is very difficult to debug
 /// such bimodal behavior.**
+///
+/// A [SynchronousFuture] will never complete with an error.
 class SynchronousFuture<T> implements Future<T> {
   /// Creates a synchronous future.
   ///

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

@@ -105,18 +105,21 @@ abstract class AssetBundle {
   /// Retrieve a string from the asset bundle, parse it with the given function,
   /// and return that function's result.
   ///
-  /// Implementations may cache the result, so a particular key should only be
-  /// used with one parser for the lifetime of the asset bundle.
-  Future<T> loadStructuredData<T>(String key, Future<T> Function(String value) parser);
+  /// The result is not cached by the default implementation; the parser is run
+  /// each time the resource is fetched. However, some subclasses may implement
+  /// caching (notably, subclasses of [CachingAssetBundle]).
+  Future<T> loadStructuredData<T>(String key, Future<T> Function(String value) parser) async {
+    return parser(await loadString(key));
+  }
 
   /// Retrieve [ByteData] from the asset bundle, parse it with the given function,
   /// and return that function's result.
   ///
-  /// Implementations may cache the result, so a particular key should only be
-  /// used with one parser for the lifetime of the asset bundle.
+  /// The result is not cached by the default implementation; the parser is run
+  /// each time the resource is fetched. However, some subclasses may implement
+  /// caching (notably, subclasses of [CachingAssetBundle]).
   Future<T> loadStructuredBinaryData<T>(String key, FutureOr<T> Function(ByteData data) parser) async {
-    final ByteData data = await load(key);
-    return parser(data);
+    return parser(await load(key));
   }
 
   /// If this is a caching asset bundle, and the given key describes a cached
@@ -161,26 +164,6 @@ class NetworkAssetBundle extends AssetBundle {
     return bytes.buffer.asByteData();
   }
 
-  /// Retrieve a string from the asset bundle, parse it with the given function,
-  /// and return the function's result.
-  ///
-  /// The result is not cached. The parser is run each time the resource is
-  /// fetched.
-  @override
-  Future<T> loadStructuredData<T>(String key, Future<T> Function(String value) parser) async {
-    return parser(await loadString(key));
-  }
-
-  /// Retrieve [ByteData] from the asset bundle, parse it with the given function,
-  /// and return the function's result.
-  ///
-  /// The result is not cached. The parser is run each time the resource is
-  /// fetched.
-  @override
-  Future<T> loadStructuredBinaryData<T>(String key, FutureOr<T> Function(ByteData data) parser) async {
-    return parser(await load(key));
-  }
-
   // TODO(ianh): Once the underlying network logic learns about caching, we
   // should implement evict().
 
@@ -217,30 +200,40 @@ abstract class CachingAssetBundle extends AssetBundle {
   /// unless you also fetch it with [loadString]). For any given `key`, the
   /// `parser` is only run the first time.
   ///
-  /// Once the value has been parsed, the future returned by this function for
-  /// subsequent calls will be a [SynchronousFuture], which resolves its
-  /// callback synchronously.
+  /// Once the value has been successfully parsed, the future returned by this
+  /// function for subsequent calls will be a [SynchronousFuture], which
+  /// resolves its callback synchronously.
+  ///
+  /// Failures are not cached, and are returned as [Future]s with errors.
   @override
   Future<T> loadStructuredData<T>(String key, Future<T> Function(String value) parser) {
     if (_structuredDataCache.containsKey(key)) {
       return _structuredDataCache[key]! as Future<T>;
     }
-    Completer<T>? completer;
-    Future<T>? result;
+    // loadString can return a SynchronousFuture in certain cases, like in the
+    // flutter_test framework. So, we need to support both async and sync flows.
+    Completer<T>? completer; // For async flow.
+    Future<T>? synchronousResult; // For sync flow.
     loadString(key, cache: false).then<T>(parser).then<void>((T value) {
-      result = SynchronousFuture<T>(value);
-      _structuredDataCache[key] = result!;
+      synchronousResult = SynchronousFuture<T>(value);
+      _structuredDataCache[key] = synchronousResult!;
       if (completer != null) {
         // We already returned from the loadStructuredData function, which means
         // we are in the asynchronous mode. Pass the value to the completer. The
         // completer's future is what we returned.
         completer.complete(value);
       }
+    }, onError: (Object error, StackTrace stack) {
+      assert(completer != null, 'unexpected synchronous failure');
+      // Either loading or parsing failed. We must report the error back to the
+      // caller and anyone waiting on this call. We clear the cache for this
+      // key, however, because we want future attempts to try again.
+      _structuredDataCache.remove(key);
+      completer!.completeError(error, stack);
     });
-    if (result != null) {
-      // The code above ran synchronously, and came up with an answer.
-      // Return the SynchronousFuture that we created above.
-      return result!;
+    if (synchronousResult != null) {
+      // The above code ran synchronously. We can synchronously return the result.
+      return synchronousResult!;
     }
     // The code above hasn't yet run its "then" handler yet. Let's prepare a
     // completer for it to use when it does run.
@@ -255,40 +248,41 @@ abstract class CachingAssetBundle extends AssetBundle {
   /// The result of parsing the bytedata is cached (the bytedata itself is not).
   /// For any given `key`, the `parser` is only run the first time.
   ///
-  /// Once the value has been parsed, the future returned by this function for
-  /// subsequent calls will be a [SynchronousFuture], which resolves its
-  /// callback synchronously.
+  /// Once the value has been successfully parsed, the future returned by this
+  /// function for subsequent calls will be a [SynchronousFuture], which
+  /// resolves its callback synchronously.
+  ///
+  /// Failures are not cached, and are returned as [Future]s with errors.
   @override
   Future<T> loadStructuredBinaryData<T>(String key, FutureOr<T> Function(ByteData data) parser) {
     if (_structuredBinaryDataCache.containsKey(key)) {
       return _structuredBinaryDataCache[key]! as Future<T>;
     }
-
     // load can return a SynchronousFuture in certain cases, like in the
     // flutter_test framework. So, we need to support both async and sync flows.
     Completer<T>? completer; // For async flow.
-    SynchronousFuture<T>? result; // For sync flow.
-
-    load(key)
-      .then<T>(parser)
-      .then<void>((T value) {
-        result = SynchronousFuture<T>(value);
-        _structuredBinaryDataCache[key] = result!;
-        if (completer != null) {
-          // The load and parse operation ran asynchronously. We already returned
-          // from the loadStructuredBinaryData function and therefore the caller
-          // was given the future of the completer.
-          completer.complete(value);
-        }
-      }, onError: (Object error, StackTrace stack) {
-        completer!.completeError(error, stack);
-      });
-
-    if (result != null) {
+    Future<T>? synchronousResult; // For sync flow.
+    load(key).then<T>(parser).then<void>((T value) {
+      synchronousResult = SynchronousFuture<T>(value);
+      _structuredBinaryDataCache[key] = synchronousResult!;
+      if (completer != null) {
+        // The load and parse operation ran asynchronously. We already returned
+        // from the loadStructuredBinaryData function and therefore the caller
+        // was given the future of the completer.
+        completer.complete(value);
+      }
+    }, onError: (Object error, StackTrace stack) {
+      assert(completer != null, 'unexpected synchronous failure');
+      // Either loading or parsing failed. We must report the error back to the
+      // caller and anyone waiting on this call. We clear the cache for this
+      // key, however, because we want future attempts to try again.
+      _structuredBinaryDataCache.remove(key);
+      completer!.completeError(error, stack);
+    });
+    if (synchronousResult != null) {
       // The above code ran synchronously. We can synchronously return the result.
-      return result!;
+      return synchronousResult!;
     }
-
     // Since the above code is being run asynchronously and thus hasn't run its
     // `then` handler yet, we'll return a completer that will be completed
     // when the handler does run.

packages/flutter/test/services/asset_bundle_test.dart

@@ -135,6 +135,26 @@ void main() {
       expect(data, isA<SynchronousFuture<int>>());
       expect(await data, 1);
     });
+
+    testWidgets('loadStructuredData handles exceptions correctly', (WidgetTester tester) async {
+      final TestAssetBundle bundle = TestAssetBundle();
+      try {
+        await bundle.loadStructuredData('AssetManifest.json', (String value) => Future<String>.error('what do they say?'));
+        fail('expected exception did not happen');
+      } catch (e) {
+        expect(e.toString(), contains('what do they say?'));
+      }
+    });
+
+    testWidgets('loadStructuredBinaryData handles exceptions correctly', (WidgetTester tester) async {
+      final TestAssetBundle bundle = TestAssetBundle();
+      try {
+        await bundle.loadStructuredBinaryData('AssetManifest.bin', (ByteData value) => Future<String>.error('buy more crystals'));
+        fail('expected exception did not happen');
+      } catch (e) {
+        expect(e.toString(), contains('buy more crystals'));
+      }
+    });
   });
 
   test('AssetImage.obtainKey succeeds with ImageConfiguration.empty', () async {