feat: Error handling enhancement for vector_graphics

- copy from this PR: dnfield/vector_graphics#258
minhhung2556 · Nov 4, 2024 · d968c1f · d968c1f
1 parent aaa1b3f
commit d968c1f
Show file tree

Hide file tree

Showing 4 changed files with 69 additions and 57 deletions.
diff --git a/packages/vector_graphics/lib/src/listener.dart b/packages/vector_graphics/lib/src/listener.dart
@@ -55,7 +55,7 @@ final Map<BytesLoader, Completer<void>> _pendingDecodes =
 /// Decode a vector graphics binary asset into a [Picture].
 ///
 /// Throws a [StateError] if the data is invalid.
-Future<PictureInfo> decodeVectorGraphics(
+Future<PictureInfo?> decodeVectorGraphics(
   ByteData data, {
   required Locale? locale,
   required TextDirection? textDirection,

diff --git a/packages/vector_graphics/lib/src/vector_graphics.dart b/packages/vector_graphics/lib/src/vector_graphics.dart
@@ -5,6 +5,7 @@
 import 'dart:math' as math;
 import 'dart:ui' as ui;
 
+import 'package:flutter/cupertino.dart';
 import 'package:flutter/foundation.dart';
 import 'package:flutter/widgets.dart';
 import 'package:vector_graphics_codec/vector_graphics_codec.dart';
@@ -279,8 +280,7 @@ class _PictureData {
 
 @immutable
 class _PictureKey {
-  const _PictureKey(
-      this.cacheKey, this.locale, this.textDirection, this.clipViewbox);
+  const _PictureKey(this.cacheKey, this.locale, this.textDirection, this.clipViewbox);
 
   final Object cacheKey;
   final Locale? locale;
@@ -306,10 +306,8 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
   Locale? locale;
   TextDirection? textDirection;
 
-  static final Map<_PictureKey, _PictureData> _livePictureCache =
-      <_PictureKey, _PictureData>{};
-  static final Map<_PictureKey, Future<_PictureData>> _pendingPictures =
-      <_PictureKey, Future<_PictureData>>{};
+  static final Map<_PictureKey, _PictureData?> _livePictureCache = <_PictureKey, _PictureData?>{};
+  static final Map<_PictureKey, Future<_PictureData?>> _pendingPictures = <_PictureKey, Future<_PictureData?>>{};
 
   @override
   void didChangeDependencies() {
@@ -345,29 +343,38 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
     }
   }
 
-  Future<_PictureData> _loadPicture(
-      BuildContext context, _PictureKey key, BytesLoader loader) {
+  Future<_PictureData?> _loadPicture(BuildContext context, _PictureKey key, BytesLoader loader) {
     if (_pendingPictures.containsKey(key)) {
       return _pendingPictures[key]!;
     }
-    final Future<_PictureData> result =
-        loader.loadBytes(context).then((ByteData data) {
-      return decodeVectorGraphics(
-        data,
-        locale: key.locale,
-        textDirection: key.textDirection,
-        clipViewbox: key.clipViewbox,
-        loader: loader,
-        onError: (Object error, StackTrace? stackTrace) {
-          return _handleError(
-            error,
-            stackTrace,
-          );
-        },
-      );
-    }).then((PictureInfo pictureInfo) {
+
+    final Future<_PictureData?> result = loader.loadBytes(context).then((ByteData data) async {
+      if (data.lengthInBytes == 0) {
+        debugPrint('_VectorGraphicWidgetState.decodeVectorGraphics: empty');
+        _handleError(const FormatException('Empty SVG xml content'), null);
+        return null;
+      } else {
+        return decodeVectorGraphics(data,
+            locale: key.locale,
+            textDirection: key.textDirection,
+            clipViewbox: key.clipViewbox,
+            loader: loader, onError: (Object error, StackTrace? stackTrace) {
+          debugPrintStack(
+              stackTrace: stackTrace, label: '_VectorGraphicWidgetState.decodeVectorGraphics.onError: $error');
+          _handleError(error, stackTrace);
+        });
+      }
+    }).onError((Object? error, StackTrace stackTrace) {
+      debugPrintStack(stackTrace: stackTrace, label: '_VectorGraphicWidgetState._loadPictureInfo.onError: $error');
+      _handleError(error ?? '', stackTrace);
+      return null;
+    }).then((PictureInfo? pictureInfo) {
+      if (pictureInfo == null) {
+        return null;
+      }
       return _PictureData(pictureInfo, 0, key);
     });
+
     _pendingPictures[key] = result;
     result.whenComplete(() {
       _pendingPictures.remove(key);
@@ -376,6 +383,9 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
   }
 
   void _handleError(Object error, StackTrace? stackTrace) {
+    if (!mounted) {
+      return;
+    }
     setState(() {
       _error = error;
       _stackTrace = stackTrace;
@@ -385,8 +395,7 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
   void _loadAssetBytes() {
     // First check if we have an avilable picture and use this immediately.
     final Object loaderKey = widget.loader.cacheKey(context);
-    final _PictureKey key =
-        _PictureKey(loaderKey, locale, textDirection, widget.clipViewbox);
+    final _PictureKey key = _PictureKey(loaderKey, locale, textDirection, widget.clipViewbox);
     final _PictureData? data = _livePictureCache[key];
     if (data != null) {
       data.count += 1;
@@ -398,7 +407,10 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
     }
     // If not, then check if there is a pending load.
     final BytesLoader loader = widget.loader;
-    _loadPicture(context, key, loader).then((_PictureData data) {
+    _loadPicture(context, key, loader).then((_PictureData? data) {
+      if (data == null) {
+        return;
+      }
       data.count += 1;
 
       // The widget may have changed, requesting a new vector graphic before
@@ -674,7 +686,7 @@ class VectorGraphicUtilities {
   ///
   /// It is the caller's responsibility to handle disposing the picture when
   /// they are done with it.
-  Future<PictureInfo> loadPicture(
+  Future<PictureInfo?> loadPicture(
     BytesLoader loader,
     BuildContext? context, {
     bool clipViewbox = true,

diff --git a/packages/vector_graphics/test/listener_test.dart b/packages/vector_graphics/test/listener_test.dart
@@ -43,31 +43,31 @@ void main() {
   });
 
   test('decode without clip', () async {
-    final PictureInfo info = await decodeVectorGraphics(
+    final PictureInfo? info = await decodeVectorGraphics(
       vectorGraphicBuffer,
       locale: ui.PlatformDispatcher.instance.locale,
       textDirection: ui.TextDirection.ltr,
       clipViewbox: true,
       loader: const AssetBytesLoader('test'),
     );
-    final ui.Image image = info.picture.toImageSync(15, 15);
-    final Uint32List imageBytes =
-        (await image.toByteData())!.buffer.asUint32List();
+    expect(info, isNotNull);
+    final ui.Image image = info!.picture.toImageSync(15, 15);
+    final Uint32List imageBytes = (await image.toByteData())!.buffer.asUint32List();
     expect(imageBytes.first, 0xFF000000);
     expect(imageBytes.last, 0x00000000);
   }, skip: kIsWeb);
 
   test('decode with clip', () async {
-    final PictureInfo info = await decodeVectorGraphics(
+    final PictureInfo? info = await decodeVectorGraphics(
       vectorGraphicBuffer,
       locale: ui.PlatformDispatcher.instance.locale,
       textDirection: ui.TextDirection.ltr,
       clipViewbox: false,
       loader: const AssetBytesLoader('test'),
     );
-    final ui.Image image = info.picture.toImageSync(15, 15);
-    final Uint32List imageBytes =
-        (await image.toByteData())!.buffer.asUint32List();
+    expect(info, isNotNull);
+    final ui.Image image = info!.picture.toImageSync(15, 15);
+    final Uint32List imageBytes = (await image.toByteData())!.buffer.asUint32List();
     expect(imageBytes.first, 0xFF000000);
     expect(imageBytes.last, 0xFF000000);
   }, skip: kIsWeb);

diff --git a/packages/vector_graphics/test/render_vector_graphics_test.dart b/packages/vector_graphics/test/render_vector_graphics_test.dart
@@ -18,7 +18,7 @@ import 'package:vector_graphics/vector_graphics.dart';
 import 'package:vector_graphics_codec/vector_graphics_codec.dart';
 
 void main() {
-  late PictureInfo pictureInfo;
+  late PictureInfo? pictureInfo;
 
   tearDown(() {
     // Since we don't always explicitly dispose render objects in unit tests, manually clear
@@ -41,7 +41,7 @@ void main() {
 
   test('Rasterizes a picture to a draw image call', () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -62,15 +62,15 @@ void main() {
 
   test('Multiple render objects with the same scale share a raster', () async {
     final RenderVectorGraphic renderVectorGraphicA = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
       null,
       1.0,
     );
     final RenderVectorGraphic renderVectorGraphicB = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -91,15 +91,15 @@ void main() {
 
   test('disposing render object release raster', () async {
     final RenderVectorGraphic renderVectorGraphicA = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
       null,
       1.0,
     );
     final RenderVectorGraphic renderVectorGraphicB = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -126,15 +126,15 @@ void main() {
       'Multiple render objects with the same scale share a raster, different load order',
       () async {
     final RenderVectorGraphic renderVectorGraphicA = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
       null,
       1.0,
     );
     final RenderVectorGraphic renderVectorGraphicB = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -158,7 +158,7 @@ void main() {
 
   test('Changing color filter does not re-rasterize', () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -185,7 +185,7 @@ void main() {
   test('Changing device pixel ratio does re-rasterize and dispose old raster',
       () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -210,7 +210,7 @@ void main() {
 
   test('Changing scale does re-rasterize and dispose old raster', () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -235,7 +235,7 @@ void main() {
 
   test('The raster size is increased by the inverse picture scale', () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -254,7 +254,7 @@ void main() {
 
   test('The raster size is increased by the device pixel ratio', () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       2.0,
@@ -273,7 +273,7 @@ void main() {
   test('The raster size is increased by the device pixel ratio and ratio',
       () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       2.0,
@@ -292,7 +292,7 @@ void main() {
   test('Changing size asserts if it is different from the picture size',
       () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -313,7 +313,7 @@ void main() {
   test('Does not rasterize a picture when fully transparent', () async {
     final FixedOpacityAnimation opacity = FixedOpacityAnimation(0.0);
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -339,7 +339,7 @@ void main() {
   test('paints partially opaque picture', () async {
     final FixedOpacityAnimation opacity = FixedOpacityAnimation(0.5);
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -355,7 +355,7 @@ void main() {
 
   test('Disposing render object disposes picture', () async {
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -376,7 +376,7 @@ void main() {
   test('Removes listeners on detach, dispose, adds then on attach', () async {
     final FixedOpacityAnimation opacity = FixedOpacityAnimation(0.5);
     final RenderVectorGraphic renderVectorGraphic = RenderVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       'test',
       null,
       1.0,
@@ -412,7 +412,7 @@ void main() {
 
   test('Color filter applies clip', () async {
     final RenderPictureVectorGraphic render = RenderPictureVectorGraphic(
-      pictureInfo,
+      pictureInfo!,
       const ui.ColorFilter.mode(Colors.green, ui.BlendMode.difference),
       null,
     );