From 51719f4869bf1b0ddf70a8c383d7754498d8fef6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Sat, 24 Jul 2021 17:24:07 +0200 Subject: [PATCH 1/7] WIP --- flutter/lib/src/default_integrations.dart | 19 +++++++++++++++++-- flutter/test/default_integrations_test.dart | 7 +++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/flutter/lib/src/default_integrations.dart b/flutter/lib/src/default_integrations.dart index e40056fe49..471287b457 100644 --- a/flutter/lib/src/default_integrations.dart +++ b/flutter/lib/src/default_integrations.dart @@ -44,7 +44,7 @@ class FlutterErrorIntegration extends Integration { void call(Hub hub, SentryFlutterOptions options) { _defaultOnError = FlutterError.onError; _integrationOnError = (FlutterErrorDetails errorDetails) async { - dynamic exception = errorDetails.exception; + final exception = errorDetails.exception; options.logger( SentryLevel.debug, @@ -52,8 +52,23 @@ class FlutterErrorIntegration extends Integration { ); if (errorDetails.silent != true || options.reportSilentFlutterErrors) { + final context = errorDetails.context?.toDescription(); + + final collector = errorDetails.informationCollector?.call() ?? []; + final information = + (StringBuffer()..writeAll(collector, '\n')).toString(); + final library = errorDetails.library; + // FlutterError doesn't crash the App. - final mechanism = Mechanism(type: 'FlutterError', handled: true); + final mechanism = Mechanism( + type: 'FlutterError', + handled: true, + data: { + if (context != null) 'context': context, + if (collector.isNotEmpty) 'information': information, + if (library != null) 'library': library, + }, + ); final throwableMechanism = ThrowableMechanism(mechanism, exception); var event = SentryEvent( diff --git a/flutter/test/default_integrations_test.dart b/flutter/test/default_integrations_test.dart index 3522489e02..30e7f0650a 100644 --- a/flutter/test/default_integrations_test.dart +++ b/flutter/test/default_integrations_test.dart @@ -46,6 +46,9 @@ void main() { final details = FlutterErrorDetails( exception: throwable, silent: silent, + context: DiagnosticsNode.message('while handling a gesture'), + library: 'sentry', + informationCollector: () => [DiagnosticsNode.message('foo bar')], ); FlutterError.reportError(details); } @@ -64,6 +67,10 @@ void main() { final throwableMechanism = event.throwableMechanism as ThrowableMechanism; expect(throwableMechanism.mechanism.type, 'FlutterError'); expect(throwableMechanism.mechanism.handled, true); + expect(throwableMechanism.mechanism.data['library'], 'sentry'); + expect(throwableMechanism.mechanism.data['context'], + 'while handling a gesture'); + expect(throwableMechanism.mechanism.data['information'], 'foo bar'); expect(throwableMechanism.throwable, exception); }); From e22e34669e92b5dce6efeac3d530c92c8d36a09e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Sat, 24 Jul 2021 17:25:10 +0200 Subject: [PATCH 2/7] Changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 07fd428c17..c9e9850349 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ * Enhancement: Call `toString()` on all non-serializable fields (#528) * Fix: Always call `Flutter.onError` in order to not swallow messages (#533) * Bump: Android SDK to 5.1.0-beta.6 (#535) +* Feat: Collect more information for exceptions collected via `FlutterError.onError` # 6.0.0-beta.2 From fdf86e9613a20aca7300a286da8876124a32e701 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Sat, 24 Jul 2021 17:30:39 +0200 Subject: [PATCH 3/7] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c9e9850349..dba2e6e8c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,7 +3,7 @@ * Enhancement: Call `toString()` on all non-serializable fields (#528) * Fix: Always call `Flutter.onError` in order to not swallow messages (#533) * Bump: Android SDK to 5.1.0-beta.6 (#535) -* Feat: Collect more information for exceptions collected via `FlutterError.onError` +* Feat: Collect more information for exceptions collected via `FlutterError.onError` (#538) # 6.0.0-beta.2 From 745c1f3cef79db86a7bfb1660b011cbb814978b7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Uek=C3=B6tter?= Date: Sun, 25 Jul 2021 18:55:58 +0200 Subject: [PATCH 4/7] Update flutter/lib/src/default_integrations.dart Co-authored-by: Bruno Garcia --- flutter/lib/src/default_integrations.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/flutter/lib/src/default_integrations.dart b/flutter/lib/src/default_integrations.dart index 471287b457..47dfed5787 100644 --- a/flutter/lib/src/default_integrations.dart +++ b/flutter/lib/src/default_integrations.dart @@ -64,7 +64,8 @@ class FlutterErrorIntegration extends Integration { type: 'FlutterError', handled: true, data: { - if (context != null) 'context': context, + // This is a message which should make sense if written after the word `thrown`: https://api.flutter.dev/flutter/foundation/FlutterErrorDetails/context.html + if (context != null) 'context': 'thrown $context', if (collector.isNotEmpty) 'information': information, if (library != null) 'library': library, }, From 7de2d6d14b80847fab8ac94a13dd23bed3c329ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Mon, 26 Jul 2021 08:53:13 +0200 Subject: [PATCH 5/7] wip --- flutter/example/lib/main.dart | 20 +++++++++++ flutter/lib/src/default_integrations.dart | 4 ++- flutter/test/default_integrations_test.dart | 37 +++++++++++++++++++-- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index 8abb7580c1..2790265c27 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -146,6 +146,26 @@ class MainScaffold extends StatelessWidget { onPressed: () => Future.delayed(Duration(milliseconds: 100), () => throw Exception('Throws in Future.delayed')), ), + RaisedButton( + child: const Text('Capture from FlutterError.onError'), + onPressed: () { + // modeled after a real exception + FlutterError.onError?.call(FlutterErrorDetails( + exception: Exception('A really bad exception'), + silent: false, + context: DiagnosticsNode.message('while handling a gesture'), + library: 'gesture', + informationCollector: () => [ + DiagnosticsNode.message( + 'Handler: "onTap" Recognizer: TapGestureRecognizer'), + DiagnosticsNode.message( + 'Handler: "onTap" Recognizer: TapGestureRecognizer'), + DiagnosticsNode.message( + 'Handler: "onTap" Recognizer: TapGestureRecognizer'), + ], + )); + }, + ), RaisedButton( child: const Text('Dart: Web request'), onPressed: () => makeWebRequest(context), diff --git a/flutter/lib/src/default_integrations.dart b/flutter/lib/src/default_integrations.dart index 47dfed5787..c43d701167 100644 --- a/flutter/lib/src/default_integrations.dart +++ b/flutter/lib/src/default_integrations.dart @@ -64,7 +64,9 @@ class FlutterErrorIntegration extends Integration { type: 'FlutterError', handled: true, data: { - // This is a message which should make sense if written after the word `thrown`: https://api.flutter.dev/flutter/foundation/FlutterErrorDetails/context.html + // This is a message which should make sense if written after the + // word `thrown`: + // https://api.flutter.dev/flutter/foundation/FlutterErrorDetails/context.html if (context != null) 'context': 'thrown $context', if (collector.isNotEmpty) 'information': information, if (library != null) 'library': library, diff --git a/flutter/test/default_integrations_test.dart b/flutter/test/default_integrations_test.dart index 30e7f0650a..30215735c9 100644 --- a/flutter/test/default_integrations_test.dart +++ b/flutter/test/default_integrations_test.dart @@ -32,6 +32,7 @@ void main() { bool silent = false, FlutterExceptionHandler? handler, dynamic exception, + FlutterErrorDetails? optionalDetails, }) { // replace default error otherwise it fails on testing FlutterError.onError = @@ -50,7 +51,7 @@ void main() { library: 'sentry', informationCollector: () => [DiagnosticsNode.message('foo bar')], ); - FlutterError.reportError(details); + FlutterError.reportError(optionalDetails ?? details); } test('FlutterError capture errors', () async { @@ -69,11 +70,43 @@ void main() { expect(throwableMechanism.mechanism.handled, true); expect(throwableMechanism.mechanism.data['library'], 'sentry'); expect(throwableMechanism.mechanism.data['context'], - 'while handling a gesture'); + 'thrown while handling a gesture'); expect(throwableMechanism.mechanism.data['information'], 'foo bar'); expect(throwableMechanism.throwable, exception); }); + test('FlutterError capture errors with long FlutterErrorDetails.information', + () async { + final details = FlutterErrorDetails( + exception: StateError('error'), + silent: false, + context: DiagnosticsNode.message('while handling a gesture'), + library: 'sentry', + informationCollector: () => [ + DiagnosticsNode.message('foo bar'), + DiagnosticsNode.message('Hello World!') + ], + ); + + // exception is ignored in this case + _reportError(exception: StateError('error'), optionalDetails: details); + + final event = verify( + await fixture.hub.captureEvent(captureAny), + ).captured.first as SentryEvent; + + expect(event.level, SentryLevel.fatal); + + final throwableMechanism = event.throwableMechanism as ThrowableMechanism; + expect(throwableMechanism.mechanism.type, 'FlutterError'); + expect(throwableMechanism.mechanism.handled, true); + expect(throwableMechanism.mechanism.data['library'], 'sentry'); + expect(throwableMechanism.mechanism.data['context'], + 'thrown while handling a gesture'); + expect(throwableMechanism.mechanism.data['information'], + 'foo bar\nHello World!'); + }); + test('FlutterError calls default error', () async { var called = false; final defaultError = (FlutterErrorDetails errorDetails) async { From 5e52b9228963dd68133600d3087e414ca4c6592d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Sat, 7 Aug 2021 09:47:14 +0200 Subject: [PATCH 6/7] More Tests, other UI --- flutter/lib/src/default_integrations.dart | 23 +++++++++--- flutter/test/default_integrations_test.dart | 41 +++++++++++++++++---- 2 files changed, 51 insertions(+), 13 deletions(-) diff --git a/flutter/lib/src/default_integrations.dart b/flutter/lib/src/default_integrations.dart index c43d701167..e22f552375 100644 --- a/flutter/lib/src/default_integrations.dart +++ b/flutter/lib/src/default_integrations.dart @@ -57,19 +57,27 @@ class FlutterErrorIntegration extends Integration { final collector = errorDetails.informationCollector?.call() ?? []; final information = (StringBuffer()..writeAll(collector, '\n')).toString(); + // errorDetails.library defaults to 'Flutter framework' even though it + // is nullable. We do null checks anyway, just to be sure. final library = errorDetails.library; + final flutterErrorDetails = { + // This is a message which should make sense if written after the + // word `thrown`: + // https://api.flutter.dev/flutter/foundation/FlutterErrorDetails/context.html + if (context != null) 'context': 'thrown $context', + if (collector.isNotEmpty) 'information': information, + if (library != null) 'library': library, + }; + // FlutterError doesn't crash the App. final mechanism = Mechanism( type: 'FlutterError', handled: true, data: { - // This is a message which should make sense if written after the - // word `thrown`: - // https://api.flutter.dev/flutter/foundation/FlutterErrorDetails/context.html - if (context != null) 'context': 'thrown $context', - if (collector.isNotEmpty) 'information': information, - if (library != null) 'library': library, + if (flutterErrorDetails.isNotEmpty) + 'hint': + 'See "flutter_error_details" down below for more information' }, ); final throwableMechanism = ThrowableMechanism(mechanism, exception); @@ -77,6 +85,9 @@ class FlutterErrorIntegration extends Integration { var event = SentryEvent( throwable: throwableMechanism, level: SentryLevel.fatal, + contexts: flutterErrorDetails.isNotEmpty + ? (Contexts()..['flutter_error_details'] = flutterErrorDetails) + : null, ); await hub.captureEvent(event, stackTrace: errorDetails.stack); diff --git a/flutter/test/default_integrations_test.dart b/flutter/test/default_integrations_test.dart index 30215735c9..2f2be6e499 100644 --- a/flutter/test/default_integrations_test.dart +++ b/flutter/test/default_integrations_test.dart @@ -68,11 +68,14 @@ void main() { final throwableMechanism = event.throwableMechanism as ThrowableMechanism; expect(throwableMechanism.mechanism.type, 'FlutterError'); expect(throwableMechanism.mechanism.handled, true); - expect(throwableMechanism.mechanism.data['library'], 'sentry'); - expect(throwableMechanism.mechanism.data['context'], - 'thrown while handling a gesture'); - expect(throwableMechanism.mechanism.data['information'], 'foo bar'); + expect(throwableMechanism.mechanism.data['hint'], + 'See "flutter_error_details" down below for more information'); expect(throwableMechanism.throwable, exception); + + expect(event.contexts['flutter_error_details']['library'], 'sentry'); + expect(event.contexts['flutter_error_details']['context'], + 'thrown while handling a gesture'); + expect(event.contexts['flutter_error_details']['information'], 'foo bar'); }); test('FlutterError capture errors with long FlutterErrorDetails.information', @@ -100,13 +103,37 @@ void main() { final throwableMechanism = event.throwableMechanism as ThrowableMechanism; expect(throwableMechanism.mechanism.type, 'FlutterError'); expect(throwableMechanism.mechanism.handled, true); - expect(throwableMechanism.mechanism.data['library'], 'sentry'); - expect(throwableMechanism.mechanism.data['context'], + expect(throwableMechanism.mechanism.data['hint'], + 'See "flutter_error_details" down below for more information'); + + expect(event.contexts['flutter_error_details']['library'], 'sentry'); + expect(event.contexts['flutter_error_details']['context'], 'thrown while handling a gesture'); - expect(throwableMechanism.mechanism.data['information'], + expect(event.contexts['flutter_error_details']['information'], 'foo bar\nHello World!'); }); + test('FlutterError capture errors with no FlutterErrorDetails', () async { + final details = FlutterErrorDetails( + exception: StateError('error'), silent: false, library: null); + + // exception is ignored in this case + _reportError(exception: StateError('error'), optionalDetails: details); + + final event = verify( + await fixture.hub.captureEvent(captureAny), + ).captured.first as SentryEvent; + + expect(event.level, SentryLevel.fatal); + + final throwableMechanism = event.throwableMechanism as ThrowableMechanism; + expect(throwableMechanism.mechanism.type, 'FlutterError'); + expect(throwableMechanism.mechanism.handled, true); + expect(throwableMechanism.mechanism.data['hint'], isNull); + + expect(event.contexts['flutter_error_details'], isNull); + }); + test('FlutterError calls default error', () async { var called = false; final defaultError = (FlutterErrorDetails errorDetails) async { From 63e5c94d4e6e9bf1636769b1a953079fa88ac173 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jonas=20Ueko=CC=88tter?= Date: Sat, 7 Aug 2021 09:48:28 +0200 Subject: [PATCH 7/7] fix changelog --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ccbe22e96..433bd0e240 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,12 +1,13 @@ # Unreleased +* Feat: Collect more information for exceptions collected via `FlutterError.onError` (#538) + # 6.0.0-beta.3 * Fix: Re-initialization of Flutter SDK (#526) * Enhancement: Call `toString()` on all non-serializable fields (#528) * Fix: Always call `Flutter.onError` in order to not swallow messages (#533) * Bump: Android SDK to 5.1.0-beta.6 (#535) -* Feat: Collect more information for exceptions collected via `FlutterError.onError` (#538) # 6.0.0-beta.2