make sure the CachedAssetReader/CachedAssetWriter dont throw synchronously

Author: jakemac53

lib/src/asset/cache.dart

@@ -57,28 +57,46 @@ class CachedAssetReader extends AssetReader {
   }
 
   @override
+  // Can't use async keyword here because we need to update
+  // `_pendingHasInputChecks` synchronously.
   Future<bool> hasInput(AssetId id) {
-    if (_cache.contains(id)) return new Future.value(true);
-
-    return _pendingHasInputChecks.putIfAbsent(id, () async {
-      var exists = await _reader.hasInput(id);
-      _pendingHasInputChecks.remove(id);
-      return exists;
-    });
+    try {
+      if (_cache.contains(id)) return new Future.value(true);
+
+      return _pendingHasInputChecks.putIfAbsent(id, () async {
+        try {
+          return await _reader.hasInput(id);
+        } finally {
+          _pendingHasInputChecks.remove(id);
+        }
+      });
+    } catch (e, s) {
+      return new Future.error(e, s);
+    }
   }
 
   @override
+  // Can't use async keyword here because we need to update
+  // `_pendingReads` synchronously.
   Future<String> readAsString(AssetId id, {Encoding encoding: UTF8}) {
-    if (_cache.contains(id)) {
-      return new Future.value(_cache.get(id).stringContents);
+    try {
+      if (_cache.contains(id)) {
+        return new Future.value(_cache.get(id).stringContents);
+      }
+
+      return _pendingReads.putIfAbsent(id, () async {
+        try {
+          var content = await _reader.readAsString(id, encoding: encoding);
+          _cache.put(new Asset(id, content));
+          return content;
+        } finally {
+          // Make sure we always remove the pending read
+          _pendingReads.remove(id);
+        }
+      });
+    } catch (e, s) {
+      return new Future.error(e, s);
     }
-
-    return _pendingReads.putIfAbsent(id, () async {
-      var content = await _reader.readAsString(id, encoding: encoding);
-      _cache.put(new Asset(id, content));
-      _pendingReads.remove(id);
-      return content;
-    });
   }
 
   @override
@@ -101,14 +119,26 @@ class CachedAssetWriter extends AssetWriter {
   CachedAssetWriter(this._cache, this._writer);
 
   @override
+  // Can't use async keyword here because we need to update `_cache`
+  // synchronously.
   Future writeAsString(Asset asset, {Encoding encoding: UTF8}) {
-    _cache.put(asset);
-    return _writer.writeAsString(asset, encoding: encoding);
+    try {
+      _cache.put(asset);
+      return _writer.writeAsString(asset, encoding: encoding);
+    } catch (e, s) {
+      return new Future.error(e, s);
+    }
   }
 
   @override
+  // Can't use async keyword here because we need to update `_cache`
+  // synchronously.
   Future delete(AssetId id) {
-    _cache.remove(id);
-    return _writer.delete(id);
+    try {
+      _cache.remove(id);
+      return _writer.delete(id);
+    } catch (e, s) {
+      return new Future.error(e, s);
+    }
   }
 }

test/asset/cache_test.dart

@@ -139,6 +139,23 @@ main() {
       childReaderAssets[b.id] = new DatedString(b.stringContents, time);
       expect(await reader.lastModified(b.id), time);
     });
+
+    group('async methods will never throw synchronously', () {
+      var cache = new _ThrowingCache();
+      var reader = new CachedAssetReader(cache, childReader);
+
+      test('hasInput', () {
+        return reader.hasInput(a.id).then((_) {
+          throw 'future should not succeed';
+        }, onError: (_) {});
+      });
+
+      test('readAsString', () {
+        return reader.readAsString(a.id).then((_) {
+          throw 'future should not succeed';
+        }, onError: (_) {});
+      });
+    });
   });
 
   group('CachedAssetWriter', () {
@@ -171,12 +188,12 @@ main() {
 
     test('multiple sync writes for the same asset throws', () async {
       writer.writeAsString(a);
-      expect(() => writer.writeAsString(a), throwsArgumentError);
+      expect(writer.writeAsString(a), throwsArgumentError);
     });
 
     test('multiple async writes for the same asset throws', () async {
       await writer.writeAsString(a);
-      expect(() => writer.writeAsString(a), throwsArgumentError);
+      expect(writer.writeAsString(a), throwsArgumentError);
     });
 
     test('delete deletes from cache and writer', () async {
@@ -185,5 +202,36 @@ main() {
       expect(cache.get(a.id), isNull);
       expect(childWriterAssets[a.id], isNull);
     });
+
+    group('async methods will never throw synchronously', () {
+      var cache = new _ThrowingCache();
+      var writer = new CachedAssetWriter(cache, childWriter);
+
+      test('delete', () {
+        return writer.delete(a.id).then((_) {
+          throw 'future should not succeed';
+        }, onError: (_) {});
+      });
+
+      test('writeAsString', () {
+        return writer.writeAsString(a).then((_) {
+          throw 'future should not succeed';
+        }, onError: (_) {});
+      });
+    });
   });
 }
+
+class _ThrowingCache implements AssetCache {
+  @override
+  bool contains(AssetId id) => throw new UnimplementedError();
+
+  @override
+  Asset remove(AssetId id) => throw new UnimplementedError();
+
+  @override
+  Asset get(AssetId id) => throw new UnimplementedError();
+
+  @override
+  void put(Asset id, {bool overwrite: false}) => throw new UnimplementedError();
+}