From 2d3b03d17e0b28a698d4de81ab6ac6b586c4533a Mon Sep 17 00:00:00 2001 From: Arne Molland Date: Tue, 28 Feb 2023 15:21:52 +0100 Subject: [PATCH] Implement `loadStructuredBinaryData` from updated AssetBundle (#1272) --- CHANGELOG.md | 4 + flutter/lib/src/sentry_asset_bundle.dart | 104 ++++++++++++- flutter/test/sentry_asset_bundle_test.dart | 167 +++++++++++++++++++++ 3 files changed, 271 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f8684bb151..b69ec23ddc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Implement `loadStructuredBinaryData` from updated AssetBundle ([#1272](https://github.com/getsentry/sentry-dart/pull/1272)) + ### Dependencies - Bump Android SDK from v6.13.0 to v6.13.1 ([#1273](https://github.com/getsentry/sentry-dart/pull/1273)) diff --git a/flutter/lib/src/sentry_asset_bundle.dart b/flutter/lib/src/sentry_asset_bundle.dart index df44031988..8849882aa4 100644 --- a/flutter/lib/src/sentry_asset_bundle.dart +++ b/flutter/lib/src/sentry_asset_bundle.dart @@ -9,7 +9,8 @@ import 'package:flutter/services.dart'; import 'package:flutter/material.dart'; import 'package:sentry/sentry.dart'; -typedef _Parser = Future Function(String value); +typedef _StringParser = Future Function(String value); +typedef _ByteParser = FutureOr Function(ByteData value); /// An [AssetBundle] which creates automatic performance traces for loading /// assets. @@ -79,7 +80,7 @@ class SentryAssetBundle implements AssetBundle { } @override - Future loadStructuredData(String key, _Parser parser) { + Future loadStructuredData(String key, _StringParser parser) { if (_enableStructuredDataTracing) { return _loadStructuredDataWithTracing(key, parser); } @@ -87,7 +88,7 @@ class SentryAssetBundle implements AssetBundle { } Future _loadStructuredDataWithTracing( - String key, _Parser parser) async { + String key, _StringParser parser) async { final span = _hub.getSpan()?.startChild( 'file.read', description: 'AssetBundle.loadStructuredData<$T>: ${_fileName(key)}', @@ -125,6 +126,46 @@ class SentryAssetBundle implements AssetBundle { return data; } + FutureOr _loadStructuredBinaryDataWithTracing( + String key, _ByteParser parser) async { + final span = _hub.getSpan()?.startChild( + 'file.read', + description: + 'AssetBundle.loadStructuredBinaryData<$T>: ${_fileName(key)}', + ); + span?.setData('file.path', key); + + final completer = Completer(); + + // This future is intentionally not awaited. Otherwise we deadlock with + // the completer. + // ignore: unawaited_futures + runZonedGuarded(() async { + final data = await _loadStructuredBinaryDataWrapper( + key, + (value) async => await _wrapBinaryParsing(parser, value, key, span), + ); + span?.status = SpanStatus.ok(); + completer.complete(data); + }, (exception, stackTrace) { + completer.completeError(exception, stackTrace); + }); + + T data; + try { + data = await completer.future; + _setDataLength(data, span); + span?.status = const SpanStatus.ok(); + } catch (e) { + span?.throwable = e; + span?.status = const SpanStatus.internalError(); + rethrow; + } finally { + await span?.finish(); + } + return data; + } + @override Future loadString(String key, {bool cache = true}) async { final span = _hub.getSpan()?.startChild( @@ -236,7 +277,7 @@ class SentryAssetBundle implements AssetBundle { } static Future _wrapParsing( - _Parser parser, + _StringParser parser, String value, String key, ISentrySpan? outerSpan, @@ -259,4 +300,59 @@ class SentryAssetBundle implements AssetBundle { return data; } + + static FutureOr _wrapBinaryParsing( + _ByteParser parser, + ByteData value, + String key, + ISentrySpan? outerSpan, + ) async { + final span = outerSpan?.startChild( + 'serialize.file.read', + description: 'parsing "$key" to "$T"', + ); + T data; + try { + data = await parser(value); + span?.status = const SpanStatus.ok(); + } catch (e) { + span?.throwable = e; + span?.status = const SpanStatus.internalError(); + rethrow; + } finally { + await span?.finish(); + } + + return data; + } + + @override + // ignore: override_on_non_overriding_member + Future loadStructuredBinaryData( + String key, + FutureOr Function(ByteData data) parser, + ) async { + if (_enableStructuredDataTracing) { + return _loadStructuredBinaryDataWithTracing(key, parser); + } + + return _loadStructuredBinaryDataWrapper(key, parser); + } + + // helper method to have a "typesafe" method + Future _loadStructuredBinaryDataWrapper( + String key, + FutureOr Function(ByteData data) parser, + ) async { + // The loadStructuredBinaryData method exists as of Flutter greater than 3.8 + // Previous versions don't have it, but later versions do. + // We can't use `extends` in order to provide this method because this is + // a wrapper and thus the method call must be forwarded. + // On Flutter versions <=3.8 we can't forward this call. + // On later version the call gets correctly forwarded. + // The error doesn't need to handled since it can't be called on earlier versions, + // and it's correctly forwarded on later versions. + return (_bundle as dynamic).loadStructuredBinaryData(key, parser) + as Future; + } } diff --git a/flutter/test/sentry_asset_bundle_test.dart b/flutter/test/sentry_asset_bundle_test.dart index 281e3ebce3..34972544f6 100644 --- a/flutter/test/sentry_asset_bundle_test.dart +++ b/flutter/test/sentry_asset_bundle_test.dart @@ -1,5 +1,6 @@ // ignore_for_file: invalid_use_of_internal_member // The lint above is okay, because we're using another Sentry package +import 'dart:async'; import 'dart:convert'; // backcompatibility for Flutter < 3.3 // ignore: unnecessary_import @@ -333,6 +334,158 @@ void main() { }, ); + test( + 'loadStructuredBinaryData: does not create any spans and just forwords the call to the underlying assetbundle if disabled', + () async { + final sut = fixture.getSut(structuredDataTracing: false); + final tr = fixture._hub.startTransaction( + 'name', + 'op', + bindToScope: true, + ); + + final data = await sut.loadStructuredBinaryData( + _testFileName, + (value) async => utf8.decode( + value.buffer.asUint8List(value.offsetInBytes, value.lengthInBytes), + ), + ); + expect(data, 'Hello World!'); + + await tr.finish(); + + final tracer = (tr as SentryTracer); + + expect(tracer.children.length, 0); + }, + ); + + test( + 'loadStructuredBinaryData: finish with errored span if loading fails', + () async { + final sut = fixture.getSut(throwException: true); + final tr = fixture._hub.startTransaction( + 'name', + 'op', + bindToScope: true, + ); + await expectLater( + sut.loadStructuredBinaryData( + _testFileName, + (value) async => utf8.decode( + value.buffer + .asUint8List(value.offsetInBytes, value.lengthInBytes), + ), + ), + throwsA(isA()), + ); + + await tr.finish(); + + final tracer = (tr as SentryTracer); + final span = tracer.children.first; + + expect(span.status, SpanStatus.internalError()); + expect(span.finished, true); + expect(span.throwable, isA()); + expect(span.context.operation, 'file.read'); + expect( + span.context.description, + 'AssetBundle.loadStructuredBinaryData: test.txt', + ); + }, + ); + + test( + 'loadStructuredBinaryData: finish with errored span if parsing fails', + () async { + final sut = fixture.getSut(throwException: false); + final tr = fixture._hub.startTransaction( + 'name', + 'op', + bindToScope: true, + ); + await expectLater( + sut.loadStructuredBinaryData( + _testFileName, + (value) async => throw Exception('error while parsing'), + ), + throwsA(isA()), + ); + + await tr.finish(); + + final tracer = (tr as SentryTracer); + var span = tracer.children.first; + + expect(tracer.children.length, 2); + + expect(span.status, SpanStatus.internalError()); + expect(span.finished, true); + expect(span.throwable, isA()); + expect(span.context.operation, 'file.read'); + expect( + span.context.description, + 'AssetBundle.loadStructuredBinaryData: test.txt', + ); + + span = tracer.children[1]; + + expect(span.status, SpanStatus.internalError()); + expect(span.finished, true); + expect(span.throwable, isA()); + expect(span.context.operation, 'serialize.file.read'); + expect( + span.context.description, + 'parsing "resources/test.txt" to "String"', + ); + }, + ); + + test( + 'loadStructuredBinaryData: finish with successfully', + () async { + final sut = fixture.getSut(throwException: false); + final tr = fixture._hub.startTransaction( + 'name', + 'op', + bindToScope: true, + ); + + await sut.loadStructuredBinaryData( + _testFileName, + (value) async => utf8.decode( + value.buffer.asUint8List(value.offsetInBytes, value.lengthInBytes), + ), + ); + + await tr.finish(); + + final tracer = (tr as SentryTracer); + var span = tracer.children.first; + + expect(tracer.children.length, 2); + + expect(span.status, SpanStatus.ok()); + expect(span.finished, true); + expect(span.context.operation, 'file.read'); + expect( + span.context.description, + 'AssetBundle.loadStructuredBinaryData: test.txt', + ); + + span = tracer.children[1]; + + expect(span.status, SpanStatus.ok()); + expect(span.finished, true); + expect(span.context.operation, 'serialize.file.read'); + expect( + span.context.description, + 'parsing "resources/test.txt" to "String"', + ); + }, + ); + test( 'evict call gets forwarded', () { @@ -393,6 +546,20 @@ class TestAssetBundle extends CachingAssetBundle { bool throwException = false; String? evictKey; + @override + // ignore: override_on_non_overriding_member + Future loadStructuredBinaryData( + String key, FutureOr Function(ByteData data) parser) async { + if (throwException) { + throw Exception('exception thrown for testing purposes'); + } + if (key == _testFileName) { + return parser(ByteData.view( + Uint8List.fromList(utf8.encode('Hello World!')).buffer)); + } + return parser(ByteData(0)); + } + @override Future load(String key) async { if (throwException) {