Unverified Commit d84879d9 authored by Jonah Williams's avatar Jonah Williams Committed by GitHub

Ensure all errors thrown by image providers can be caught by developers. (#25980)

* Ensure all errors thrown by image providers can be caught by developers.

Add an `onError` parameter to the ImageCache.putIfAbsent method.
In the event that an error is thrown when resolving an image, catch if
this parameter is provided. Use the onError parameter to ensure that all
errors thrown are forwarded to the ImageStream error channel instead of
directly into the void.
parent 919f457a
......@@ -126,7 +126,12 @@ class ImageCache {
/// key is moved to the "most recently used" position.
///
/// The arguments must not be null. The `loader` cannot return null.
ImageStreamCompleter putIfAbsent(Object key, ImageStreamCompleter loader()) {
///
/// In the event that the loader throws an exception, it will be caught only if
/// `onError` is also provided. When an exception is caught resolving an image,
/// no completers are cached and `null` is returned instead of a new
/// completer.
ImageStreamCompleter putIfAbsent(Object key, ImageStreamCompleter loader(), { ImageErrorListener onError }) {
assert(key != null);
assert(loader != null);
ImageStreamCompleter result = _pendingImages[key];
......@@ -140,7 +145,16 @@ class ImageCache {
_cache[key] = image;
return image.completer;
}
result = loader();
try {
result = loader();
} catch (error, stackTrace) {
if (onError != null) {
onError(error, stackTrace);
return null;
} else {
rethrow;
}
}
void listener(ImageInfo info, bool syncCall) {
// Images that fail to load don't contribute to cache size.
final int imageSize = info?.image == null ? 0 : info.image.height * info.image.width * 4;
......
......@@ -262,27 +262,31 @@ abstract class ImageProvider<T> {
assert(configuration != null);
final ImageStream stream = ImageStream();
T obtainedKey;
Future<void> handleError(dynamic exception, StackTrace stack) async {
await null; // wait an event turn in case a listener has been added to the image stream.
final _ErrorImageCompleter imageCompleter = _ErrorImageCompleter();
stream.setCompleter(imageCompleter);
imageCompleter.setError(
exception: exception,
stack: stack,
context: 'while resolving an image',
silent: true, // could be a network error or whatnot
informationCollector: (StringBuffer information) {
information.writeln('Image provider: $this');
information.writeln('Image configuration: $configuration');
if (obtainedKey != null) {
information.writeln('Image key: $obtainedKey');
}
}
);
}
obtainKey(configuration).then<void>((T key) {
obtainedKey = key;
stream.setCompleter(PaintingBinding.instance.imageCache.putIfAbsent(key, () => load(key)));
}).catchError(
(dynamic exception, StackTrace stack) async {
FlutterError.reportError(FlutterErrorDetails(
exception: exception,
stack: stack,
library: 'services library',
context: 'while resolving an image',
silent: true, // could be a network error or whatnot
informationCollector: (StringBuffer information) {
information.writeln('Image provider: $this');
information.writeln('Image configuration: $configuration');
if (obtainedKey != null)
information.writeln('Image key: $obtainedKey');
}
));
return null;
final ImageStreamCompleter completer = PaintingBinding.instance.imageCache.putIfAbsent(key, () => load(key), onError: handleError);
if (completer != null) {
stream.setCompleter(completer);
}
);
}).catchError(handleError);
return stream;
}
......@@ -495,7 +499,7 @@ class NetworkImage extends ImageProvider<NetworkImage> {
if (bytes.lengthInBytes == 0)
throw Exception('NetworkImage is an empty file: $resolved');
return await PaintingBinding.instance.instantiateImageCodec(bytes);
return PaintingBinding.instance.instantiateImageCodec(bytes);
}
@override
......@@ -773,3 +777,24 @@ class ExactAssetImage extends AssetBundleImageProvider {
@override
String toString() => '$runtimeType(name: "$keyName", scale: $scale, bundle: $bundle)';
}
// A completer used when resolving an image fails sync.
class _ErrorImageCompleter extends ImageStreamCompleter {
_ErrorImageCompleter();
void setError({
String context,
dynamic exception,
StackTrace stack,
InformationCollector informationCollector,
bool silent = false,
}) {
reportError(
context: context,
exception: exception,
stack: stack,
informationCollector: informationCollector,
silent: silent,
);
}
}
......@@ -128,5 +128,18 @@ void main() {
expect(imageCache.currentSizeBytes, 256);
expect(imageCache.maximumSizeBytes, 256 + 1000);
});
test('Returns null if an error is caught resolving an image', () {
final ErrorImageProvider errorImage = ErrorImageProvider();
expect(() => imageCache.putIfAbsent(errorImage, () => errorImage.load(errorImage)), throwsA(isInstanceOf<Error>()));
bool caughtError = false;
final ImageStreamCompleter result = imageCache.putIfAbsent(errorImage, () => errorImage.load(errorImage), onError: (dynamic error, StackTrace stackTrace) {
caughtError = true;
});
expect(result, null);
expect(caughtError, true);
});
});
}
......@@ -5,11 +5,13 @@
import 'dart:async';
import 'dart:typed_data';
import 'package:flutter/foundation.dart';
import 'package:flutter/painting.dart';
import 'package:flutter_test/flutter_test.dart';
import '../rendering/rendering_tester.dart';
import 'image_data.dart';
import 'mocks_for_image_cache.dart';
void main() {
TestRenderingFlutterBinding(); // initializes the imageCache
......@@ -53,5 +55,20 @@ void main() {
expect(otherCache.currentSize, 0);
expect(imageCache.currentSize, 1);
});
test('ImageProvider errors can always be caught', () async {
final ErrorImageProvider imageProvider = ErrorImageProvider();
final Completer<bool> caughtError = Completer<bool>();
FlutterError.onError = (FlutterErrorDetails details) {
caughtError.complete(false);
};
final ImageStream stream = imageProvider.resolve(ImageConfiguration.empty);
stream.addListener((ImageInfo info, bool syncCall) {
caughtError.complete(false);
}, onError: (dynamic error, StackTrace stackTrace) {
caughtError.complete(true);
});
expect(await caughtError.future, true);
});
});
}
......@@ -72,3 +72,15 @@ class TestImage implements ui.Image {
throw UnimplementedError();
}
}
class ErrorImageProvider extends ImageProvider<ErrorImageProvider> {
@override
ImageStreamCompleter load(ErrorImageProvider key) {
throw Error();
}
@override
Future<ErrorImageProvider> obtainKey(ImageConfiguration configuration) {
return SynchronousFuture<ErrorImageProvider>(this);
}
}
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment