From 43760f991fd70985663607059a73897fe3123966 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto <5731772+marandaneto@users.noreply.github.com> Date: Wed, 24 May 2023 13:33:12 +0200 Subject: [PATCH] Wrapped methods return a Future instead of executing right away (#1476) --- CHANGELOG.md | 5 ++ flutter/lib/src/sentry_asset_bundle.dart | 30 +++------ sqflite/lib/src/sentry_batch.dart | 12 ++-- sqflite/lib/src/sentry_database.dart | 12 ++-- sqflite/lib/src/sentry_database_executor.dart | 66 +++++++------------ sqflite/lib/src/sentry_sqflite.dart | 6 +- .../src/sentry_sqflite_database_factory.dart | 6 +- sqflite/test/sentry_batch_test.dart | 4 +- sqflite/test/sentry_database_test.dart | 29 ++++---- 9 files changed, 67 insertions(+), 103 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6be2cd0deb..bbf9d038dc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,11 @@ ## Unreleased +### Fixes + +- Wrapped methods return a `Future` instead of executing right away ([#1476](https://github.com/getsentry/sentry-dart/pull/1476)) + - Relates to ([#1462](https://github.com/getsentry/sentry-dart/pull/1462)) + ### Dependencies - Bump Android SDK from v6.19.0 to v6.19.1 ([#1466](https://github.com/getsentry/sentry-dart/pull/1466)) diff --git a/flutter/lib/src/sentry_asset_bundle.dart b/flutter/lib/src/sentry_asset_bundle.dart index ab6996193e..10a3cd9bcc 100644 --- a/flutter/lib/src/sentry_asset_bundle.dart +++ b/flutter/lib/src/sentry_asset_bundle.dart @@ -54,7 +54,7 @@ class SentryAssetBundle implements AssetBundle { @override Future load(String key) { - Future future() async { + return Future(() async { final span = _hub.getSpan()?.startChild( 'file.read', description: 'AssetBundle.load: ${_fileName(key)}', @@ -75,9 +75,7 @@ class SentryAssetBundle implements AssetBundle { await span?.finish(); } return data; - } - - return future(); + }); } @override @@ -90,7 +88,7 @@ class SentryAssetBundle implements AssetBundle { Future _loadStructuredDataWithTracing( String key, _StringParser parser) { - Future future() async { + return Future(() async { final span = _hub.getSpan()?.startChild( 'file.read', description: @@ -127,14 +125,12 @@ class SentryAssetBundle implements AssetBundle { await span?.finish(); } return data; - } - - return future(); + }); } Future _loadStructuredBinaryDataWithTracing( String key, _ByteParser parser) { - Future future() async { + return Future(() async { final span = _hub.getSpan()?.startChild( 'file.read', description: @@ -171,14 +167,12 @@ class SentryAssetBundle implements AssetBundle { await span?.finish(); } return data; - } - - return future(); + }); } @override Future loadString(String key, {bool cache = true}) { - Future future() async { + return Future(() async { final span = _hub.getSpan()?.startChild( 'file.read', description: 'AssetBundle.loadString: ${_fileName(key)}', @@ -199,9 +193,7 @@ class SentryAssetBundle implements AssetBundle { await span?.finish(); } return data; - } - - return future(); + }); } void _setDataLength(dynamic data, ISentrySpan? span) { @@ -238,7 +230,7 @@ class SentryAssetBundle implements AssetBundle { // This is an override on Flutter greater than 3.1 // ignore: override_on_non_overriding_member Future loadBuffer(String key) { - Future future() async { + return Future(() async { final span = _hub.getSpan()?.startChild( 'file.read', description: 'AssetBundle.loadBuffer: ${_fileName(key)}', @@ -259,9 +251,7 @@ class SentryAssetBundle implements AssetBundle { await span?.finish(); } return data; - } - - return future(); + }); } Future _loadBuffer(String key) { diff --git a/sqflite/lib/src/sentry_batch.dart b/sqflite/lib/src/sentry_batch.dart index 79cabcdb08..f26c3218db 100644 --- a/sqflite/lib/src/sentry_batch.dart +++ b/sqflite/lib/src/sentry_batch.dart @@ -40,7 +40,7 @@ class SentryBatch implements Batch { @override Future> apply({bool? noResult, bool? continueOnError}) { - Future> future() async { + return Future>(() async { final currentSpan = _hub.getSpan(); final span = currentSpan?.startChild( @@ -65,9 +65,7 @@ class SentryBatch implements Batch { } finally { await span?.finish(); } - } - - return future(); + }); } @override @@ -76,7 +74,7 @@ class SentryBatch implements Batch { bool? noResult, bool? continueOnError, }) { - Future> future() async { + return Future>(() async { final currentSpan = _hub.getSpan(); final span = currentSpan?.startChild( @@ -102,9 +100,7 @@ class SentryBatch implements Batch { } finally { await span?.finish(); } - } - - return future(); + }); } @override diff --git a/sqflite/lib/src/sentry_database.dart b/sqflite/lib/src/sentry_database.dart index 3af501caf0..345924349d 100644 --- a/sqflite/lib/src/sentry_database.dart +++ b/sqflite/lib/src/sentry_database.dart @@ -54,7 +54,7 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { @override Future close() { - Future future() async { + return Future(() async { final currentSpan = _hub.getSpan(); final span = currentSpan?.startChild( dbOp, @@ -73,9 +73,7 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { } finally { await span?.finish(); } - } - - return future(); + }); } @override @@ -105,7 +103,7 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { Future Function(Transaction txn) action, { bool? exclusive, }) { - Future future() async { + return Future(() async { final currentSpan = _hub.getSpan(); final span = currentSpan?.startChild( _dbSqlOp, @@ -136,8 +134,6 @@ class SentryDatabase extends SentryDatabaseExecutor implements Database { } finally { await span?.finish(); } - } - - return future(); + }); } } diff --git a/sqflite/lib/src/sentry_database_executor.dart b/sqflite/lib/src/sentry_database_executor.dart index 4284de79f0..e00896eef6 100644 --- a/sqflite/lib/src/sentry_database_executor.dart +++ b/sqflite/lib/src/sentry_database_executor.dart @@ -31,7 +31,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { @override Future delete(String table, {String? where, List? whereArgs}) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final builder = SqlBuilder.delete(table, where: where, whereArgs: whereArgs); @@ -55,14 +55,12 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override Future execute(String sql, [List? arguments]) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final span = currentSpan?.startChild( SentryDatabase.dbSqlExecuteOp, @@ -81,9 +79,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override @@ -93,7 +89,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { String? nullColumnHack, ConflictAlgorithm? conflictAlgorithm, }) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final builder = SqlBuilder.insert( table, @@ -125,9 +121,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override @@ -143,7 +137,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { int? limit, int? offset, }) { - Future>> future() async { + return Future>>(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final builder = SqlBuilder.query( table, @@ -187,9 +181,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override @@ -206,7 +198,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { int? offset, int? bufferSize, }) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final builder = SqlBuilder.query( table, @@ -251,14 +243,12 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override Future rawDelete(String sql, [List? arguments]) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final span = currentSpan?.startChild( SentryDatabase.dbSqlExecuteOp, @@ -279,14 +269,12 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override Future rawInsert(String sql, [List? arguments]) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final span = currentSpan?.startChild( SentryDatabase.dbSqlExecuteOp, @@ -307,9 +295,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override @@ -317,7 +303,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { String sql, [ List? arguments, ]) { - Future>> future() async { + return Future>>(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final span = currentSpan?.startChild( SentryDatabase.dbSqlQueryOp, @@ -338,9 +324,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override @@ -349,7 +333,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { List? arguments, { int? bufferSize, }) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final span = currentSpan?.startChild( SentryDatabase.dbSqlQueryOp, @@ -374,14 +358,12 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override Future rawUpdate(String sql, [List? arguments]) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final span = currentSpan?.startChild( SentryDatabase.dbSqlExecuteOp, @@ -402,9 +384,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } @override @@ -415,7 +395,7 @@ class SentryDatabaseExecutor implements DatabaseExecutor { List? whereArgs, ConflictAlgorithm? conflictAlgorithm, }) { - Future future() async { + return Future(() async { final currentSpan = _parentSpan ?? _hub.getSpan(); final builder = SqlBuilder.update( table, @@ -449,8 +429,6 @@ class SentryDatabaseExecutor implements DatabaseExecutor { } finally { await span?.finish(); } - } - - return future(); + }); } } diff --git a/sqflite/lib/src/sentry_sqflite.dart b/sqflite/lib/src/sentry_sqflite.dart index ef8542186b..79c609df62 100644 --- a/sqflite/lib/src/sentry_sqflite.dart +++ b/sqflite/lib/src/sentry_sqflite.dart @@ -25,7 +25,7 @@ Future openDatabaseWithSentry( bool singleInstance = true, @internal Hub? hub, }) { - Future openDatabase() async { + return Future(() async { final dbOptions = OpenDatabaseOptions( version: version, onConfigure: onConfigure, @@ -61,9 +61,7 @@ Future openDatabaseWithSentry( } finally { await span?.finish(); } - } - - return openDatabase(); + }); } /// Opens a database with Sentry support. diff --git a/sqflite/lib/src/sentry_sqflite_database_factory.dart b/sqflite/lib/src/sentry_sqflite_database_factory.dart index c4c7dcd32e..8196e4f977 100644 --- a/sqflite/lib/src/sentry_sqflite_database_factory.dart +++ b/sqflite/lib/src/sentry_sqflite_database_factory.dart @@ -57,7 +57,7 @@ class SentrySqfliteDatabaseFactory with SqfliteDatabaseFactoryMixin { return databaseFactory.openDatabase(path, options: options); } - Future openDatabase() async { + return Future(() async { final currentSpan = _hub.getSpan(); final span = currentSpan?.startChild( SentryDatabase.dbOp, @@ -80,8 +80,6 @@ class SentrySqfliteDatabaseFactory with SqfliteDatabaseFactoryMixin { } finally { await span?.finish(); } - } - - return openDatabase(); + }); } } diff --git a/sqflite/test/sentry_batch_test.dart b/sqflite/test/sentry_batch_test.dart index 78b6bf6796..4cea3cbf7b 100644 --- a/sqflite/test/sentry_batch_test.dart +++ b/sqflite/test/sentry_batch_test.dart @@ -281,7 +281,7 @@ SELECT * FROM Product'''; batch.insert('Product', {'title': 'Product 1'}); - expect(() async => await batch.commit(), throwsException); + await expectLater(() async => await batch.commit(), throwsException); final span = fixture.tracer.children.last; expect(span.throwable, fixture.exception); @@ -295,7 +295,7 @@ SELECT * FROM Product'''; batch.insert('Product', {'title': 'Product 1'}); - expect(() async => await batch.apply(), throwsException); + await expectLater(() async => await batch.apply(), throwsException); final span = fixture.tracer.children.last; expect(span.throwable, fixture.exception); diff --git a/sqflite/test/sentry_database_test.dart b/sqflite/test/sentry_database_test.dart index b4aa459db9..3ec80568fc 100644 --- a/sqflite/test/sentry_database_test.dart +++ b/sqflite/test/sentry_database_test.dart @@ -137,7 +137,7 @@ void main() { final db = await fixture.getSut(database: fixture.database); - expect(() async => await db.close(), throwsException); + await expectLater(() async => await db.close(), throwsException); final span = fixture.tracer.children.last; expect(span.throwable, fixture.exception); @@ -150,7 +150,10 @@ void main() { final db = await fixture.getSut(database: fixture.database); - expect(() async => await db.transaction((txn) async {}), throwsException); + await expectLater( + () async => await db.transaction((txn) async {}), + throwsException, + ); final span = fixture.tracer.children.last; expect(span.throwable, fixture.exception); @@ -342,7 +345,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.delete('Product'), throwsException, ); @@ -357,7 +360,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.execute('sql'), throwsException, ); @@ -372,7 +375,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor .insert('Product', {'title': 'Product 1'}), throwsException, @@ -388,7 +391,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.query('sql'), throwsException, ); @@ -403,7 +406,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.queryCursor('sql'), throwsException, ); @@ -418,7 +421,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.rawDelete('sql'), throwsException, ); @@ -433,7 +436,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.rawInsert('sql'), throwsException, ); @@ -448,7 +451,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.rawQuery('sql'), throwsException, ); @@ -464,7 +467,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.rawQueryCursor('sql', []), throwsException, ); @@ -479,7 +482,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor.rawUpdate('sql'), throwsException, ); @@ -494,7 +497,7 @@ void main() { final executor = fixture.getExecutorSut(); - expect( + await expectLater( () async => await executor .update('Product', {'title': 'Product 1'}), throwsException,