Skip to content

[vector_graphics] handle errors from bytes loader #8080

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/vector_graphics/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 1.1.15

* Updates error handling in VectorGraphicWidget to handle errors when the bytes of the graphic cannot be loaded.

## 1.1.14

* Relaxes dependency constraint on vector_graphics_codec.
Expand Down
27 changes: 16 additions & 11 deletions packages/vector_graphics/lib/src/vector_graphics.dart
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

import 'dart:async';
import 'dart:math' as math;
import 'dart:ui' as ui;

Expand Down Expand Up @@ -315,14 +316,14 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
void didChangeDependencies() {
locale = Localizations.maybeLocaleOf(context);
textDirection = Directionality.maybeOf(context);
_loadAssetBytes();
unawaited(_loadAssetBytes());
super.didChangeDependencies();
}

@override
void didUpdateWidget(covariant VectorGraphic oldWidget) {
if (oldWidget.loader != widget.loader) {
_loadAssetBytes();
unawaited(_loadAssetBytes());
}
super.didUpdateWidget(oldWidget);
}
Expand Down Expand Up @@ -358,12 +359,6 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
textDirection: key.textDirection,
clipViewbox: key.clipViewbox,
loader: loader,
onError: (Object error, StackTrace? stackTrace) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handler has been moved higher up, so this one should not be needed anymore.

return _handleError(
error,
stackTrace,
);
},
);
}).then((PictureInfo pictureInfo) {
return _PictureData(pictureInfo, 0, key);
Expand All @@ -376,13 +371,17 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
}

void _handleError(Object error, StackTrace? stackTrace) {
if (!mounted) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this function is invoked after the future has been initiated, the widget could be unmounted.

return;
}

setState(() {
_error = error;
_stackTrace = stackTrace;
});
}

void _loadAssetBytes() {
Future<void> _loadAssetBytes() async {
Copy link
Contributor Author

@navaronbracke navaronbracke Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I initially tried using a catchError() on the _loadPicture() call, but that still caused the written test to fail.

Since async work in a void method smells of https://dart.dev/tools/linter-rules/avoid_void_async (even though we did not use async here) I opted to change this to Future<void>, which did fix my test.

// First check if we have an avilable picture and use this immediately.
final Object loaderKey = widget.loader.cacheKey(context);
final _PictureKey key =
Expand All @@ -398,7 +397,9 @@ 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) {

try {
final _PictureData data = await _loadPicture(context, key, loader);
data.count += 1;

// The widget may have changed, requesting a new vector graphic before
Expand All @@ -407,14 +408,18 @@ class _VectorGraphicWidgetState extends State<VectorGraphic> {
_maybeReleasePicture(data);
return;
}

if (data.count == 1) {
_livePictureCache[key] = data;
}

setState(() {
_maybeReleasePicture(_pictureInfo);
_pictureInfo = data;
});
});
} catch (error, stackTrace) {
_handleError(error, stackTrace);
}
}

static final bool _webRenderObject = useHtmlRenderObject();
Expand Down
2 changes: 1 addition & 1 deletion packages/vector_graphics/pubspec.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ name: vector_graphics
description: A vector graphics rendering package for Flutter using a binary encoding.
repository: https://github.com/flutter/packages/tree/main/packages/vector_graphics
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+vector_graphics%22
version: 1.1.14
version: 1.1.15

environment:
sdk: ^3.4.0
Expand Down
31 changes: 31 additions & 0 deletions packages/vector_graphics/test/vector_graphics_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,28 @@ void main() {
expect(matrix.row0.x, -1);
expect(matrix.row1.y, 1);
});

testWidgets('VectorGraphicsWidget can handle errors from bytes loader',
(WidgetTester tester) async {
await tester.pumpWidget(
VectorGraphic(
loader: const ThrowingBytesLoader(),
width: 100,
height: 100,
errorBuilder:
(BuildContext context, Object error, StackTrace stackTrace) {
return const Directionality(
textDirection: TextDirection.ltr,
child: Text('Error is handled'),
);
},
),
);
await tester.pumpAndSettle();

expect(find.text('Error is handled'), findsOneWidget);
expect(tester.takeException(), isNull);
});
}

class TestBundle extends Fake implements AssetBundle {
Expand Down Expand Up @@ -719,3 +741,12 @@ class TestBytesLoader extends BytesLoader {
@override
String toString() => 'TestBytesLoader: $source';
}

class ThrowingBytesLoader extends BytesLoader {
const ThrowingBytesLoader();

@override
Future<ByteData> loadBytes(BuildContext? context) {
throw UnimplementedError('Test exception');
}
}