From 4b77b4192454b79e41878760011f99826ecb868e Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Wed, 20 Nov 2024 16:16:32 +0100 Subject: [PATCH] provide sentry wrapped runZonedGuarded --- .../src/run_zoned_guarded_integration.dart | 112 ++-------------- dart/lib/src/sentry.dart | 16 +++ dart/lib/src/sentry_run_zoned_guarded.dart | 121 ++++++++++++++++++ ...art => sentry_run_zoned_guarded_test.dart} | 69 ++++++---- flutter/lib/src/sentry_flutter.dart | 17 +-- 5 files changed, 195 insertions(+), 140 deletions(-) create mode 100644 dart/lib/src/sentry_run_zoned_guarded.dart rename dart/test/{run_zoned_guarded_integration_test.dart => sentry_run_zoned_guarded_test.dart} (60%) diff --git a/dart/lib/src/run_zoned_guarded_integration.dart b/dart/lib/src/run_zoned_guarded_integration.dart index 6aa7b1a7f7..21abc5b6b2 100644 --- a/dart/lib/src/run_zoned_guarded_integration.dart +++ b/dart/lib/src/run_zoned_guarded_integration.dart @@ -1,13 +1,7 @@ import 'dart:async'; -import 'package:meta/meta.dart'; - import '../sentry.dart'; -import 'hub.dart'; -import 'integration.dart'; -import 'protocol.dart'; -import 'sentry_options.dart'; -import 'throwable_mechanism.dart'; +import 'sentry_run_zoned_guarded.dart'; /// Called inside of `runZonedGuarded` typedef RunZonedGuardedRunner = Future Function(); @@ -27,107 +21,17 @@ class RunZonedGuardedIntegration extends Integration { final RunZonedGuardedRunner _runner; final RunZonedGuardedOnError? _onError; - /// Needed to check if we somehow caused a `print()` recursion - bool _isPrinting = false; - - @visibleForTesting - Future captureError( - Hub hub, - SentryOptions options, - Object exception, - StackTrace stackTrace, - ) async { - options.logger( - SentryLevel.error, - 'Uncaught zone error', - logger: 'sentry.runZonedGuarded', - exception: exception, - stackTrace: stackTrace, - ); - - // runZonedGuarded doesn't crash the app, but is not handled by the user. - final mechanism = Mechanism(type: 'runZonedGuarded', handled: false); - final throwableMechanism = ThrowableMechanism(mechanism, exception); - - final event = SentryEvent( - throwable: throwableMechanism, - level: options.markAutomaticallyCollectedErrorsAsFatal - ? SentryLevel.fatal - : SentryLevel.error, - timestamp: hub.options.clock(), - ); - - // marks the span status if none to `internal_error` in case there's an - // unhandled error - hub.configureScope( - (scope) => scope.span?.status ??= const SpanStatus.internalError(), - ); - - await hub.captureEvent(event, stackTrace: stackTrace); - } - @override Future call(Hub hub, SentryOptions options) { final completer = Completer(); - runZonedGuarded( - () async { - try { - await _runner(); - } finally { - completer.complete(); - } - }, - (exception, stackTrace) async { - await captureError(hub, options, exception, stackTrace); - final onError = _onError; - if (onError != null) { - await onError(exception, stackTrace); - } - }, - zoneSpecification: ZoneSpecification( - print: (self, parent, zone, line) { - if (!options.enablePrintBreadcrumbs || !hub.isEnabled) { - // early bail out, in order to better guard against the recursion - // as described below. - parent.print(zone, line); - return; - } - - if (_isPrinting) { - // We somehow landed in a recursion. - // This happens for example if: - // - hub.addBreadcrumb() called print() itself - // - This happens for example if hub.isEnabled == false and - // options.logger == dartLogger - // - // Anyway, in order to not cause a stack overflow due to recursion - // we drop any further print() call while adding a breadcrumb. - parent.print( - zone, - 'Recursion during print() call. ' - 'Abort adding print() call as Breadcrumb.', - ); - return; - } - - _isPrinting = true; - - try { - hub.addBreadcrumb( - Breadcrumb.console( - message: line, - level: SentryLevel.debug, - ), - ); - - parent.print(zone, line); - } finally { - _isPrinting = false; - } - }, - ), - ); + SentryRunZonedGuarded.sentryRunZonedGuarded(() async { + try { + await _runner(); + } finally { + completer.complete(); + } + }, _onError); options.sdk.addIntegration('runZonedGuardedIntegration'); diff --git a/dart/lib/src/sentry.dart b/dart/lib/src/sentry.dart index cd89d2fd4a..490774bcb5 100644 --- a/dart/lib/src/sentry.dart +++ b/dart/lib/src/sentry.dart @@ -21,6 +21,7 @@ import 'noop_isolate_error_integration.dart' import 'protocol.dart'; import 'sentry_client.dart'; import 'sentry_options.dart'; +import 'sentry_run_zoned_guarded.dart'; import 'sentry_user_feedback.dart'; import 'tracing.dart'; import 'sentry_attachment/sentry_attachment.dart'; @@ -365,4 +366,19 @@ class Sentry { @internal static Hub get currentHub => _hub; + + /// TODO @denis Document me + static runZonedGuarded( + R Function() body, + void Function(Object error, StackTrace stack)? onError, { + Map? zoneValues, + ZoneSpecification? zoneSpecification, + }) { + return SentryRunZonedGuarded.sentryRunZonedGuarded( + body, + onError, + zoneValues: zoneValues, + zoneSpecification: zoneSpecification, + ); + } } diff --git a/dart/lib/src/sentry_run_zoned_guarded.dart b/dart/lib/src/sentry_run_zoned_guarded.dart new file mode 100644 index 0000000000..d21f3875ac --- /dev/null +++ b/dart/lib/src/sentry_run_zoned_guarded.dart @@ -0,0 +1,121 @@ +import 'dart:async'; + +import 'package:meta/meta.dart'; + +import '../sentry.dart'; + +/// Needed to check if we somehow caused a `print()` recursion +var _isPrinting = false; // TODO @denis Move to non-global? + +extension SentryRunZonedGuarded on Sentry { + @internal + static R? sentryRunZonedGuarded( + R Function() body, + void Function(Object error, StackTrace stack)? onError, { + Map? zoneValues, + ZoneSpecification? zoneSpecification, + }) { + final sentryOnError = (exception, stackTrace) async { + final hub = HubAdapter(); + final options = hub.options; + + await captureError(hub, options, exception, stackTrace); + + if (onError != null) { + onError(exception, stackTrace); + } + }; + + final userPrint = zoneSpecification?.print; + + final sentryZoneSpecification = ZoneSpecification.from( + zoneSpecification ?? ZoneSpecification(), + print: (self, parent, zone, line) async { + final hub = HubAdapter(); + final options = hub.options; + + if (userPrint != null) { + userPrint(self, parent, zone, line); + } + + if (!options.enablePrintBreadcrumbs || !hub.isEnabled) { + // early bail out, in order to better guard against the recursion + // as described below. + parent.print(zone, line); + return; + } + if (_isPrinting) { + // We somehow landed in a recursion. + // This happens for example if: + // - hub.addBreadcrumb() called print() itself + // - This happens for example if hub.isEnabled == false and + // options.logger == dartLogger + // + // Anyway, in order to not cause a stack overflow due to recursion + // we drop any further print() call while adding a breadcrumb. + parent.print( + zone, + 'Recursion during print() call.' + 'Abort adding print() call as Breadcrumb.', + ); + return; + } + + try { + _isPrinting = true; + await hub.addBreadcrumb( + Breadcrumb.console( + message: line, + level: SentryLevel.debug, + ), + ); + parent.print(zone, line); + } finally { + _isPrinting = false; + } + }, + ); + return runZonedGuarded( + body, + sentryOnError, + zoneValues: zoneValues, + zoneSpecification: sentryZoneSpecification, + ); + } + + @visibleForTesting + static Future captureError( + Hub hub, + SentryOptions options, + Object exception, + StackTrace stackTrace, + ) async { + options.logger( + SentryLevel.error, + 'Uncaught zone error', + logger: 'sentry.runZonedGuarded', + exception: exception, + stackTrace: stackTrace, + ); + + // runZonedGuarded doesn't crash the app, but is not handled by the user. + final mechanism = Mechanism(type: 'runZonedGuarded', handled: false); + final throwableMechanism = ThrowableMechanism(mechanism, exception); + + final event = SentryEvent( + throwable: throwableMechanism, + level: options.markAutomaticallyCollectedErrorsAsFatal + ? SentryLevel.fatal + : SentryLevel.error, + timestamp: hub.options.clock(), + ); + + // marks the span status if none to `internal_error` in case there's an + // unhandled error + hub.configureScope( + (scope) => scope.span?.status ??= const SpanStatus.internalError(), + ); + + await hub.captureEvent(event, stackTrace: stackTrace); + } +} diff --git a/dart/test/run_zoned_guarded_integration_test.dart b/dart/test/sentry_run_zoned_guarded_test.dart similarity index 60% rename from dart/test/run_zoned_guarded_integration_test.dart rename to dart/test/sentry_run_zoned_guarded_test.dart index 71f87e9d12..c05615ef01 100644 --- a/dart/test/run_zoned_guarded_integration_test.dart +++ b/dart/test/sentry_run_zoned_guarded_test.dart @@ -1,7 +1,10 @@ @TestOn('vm') library dart_test; +import 'dart:async'; + import 'package:sentry/sentry.dart'; +import 'package:sentry/src/sentry_run_zoned_guarded.dart'; import 'package:test/test.dart'; import 'mocks/mock_hub.dart'; @@ -9,7 +12,7 @@ import 'mocks/mock_sentry_client.dart'; import 'test_utils.dart'; void main() { - group(RunZonedGuardedIntegration, () { + group("SentryRunZonedGuarded", () { late Fixture fixture; setUp(() { @@ -26,11 +29,14 @@ void main() { final client = MockSentryClient(); hub.bindClient(client); - final sut = fixture.getSut(runner: () async {}); - hub.startTransaction('name', 'operation', bindToScope: true); - await sut.captureError(hub, fixture.options, exception, stackTrace); + await SentryRunZonedGuarded.captureError( + hub, + fixture.options, + exception, + stackTrace, + ); final span = hub.getSpan(); @@ -42,20 +48,42 @@ void main() { test('calls onError', () async { final error = StateError("StateError"); var onErrorCalled = false; - RunZonedGuardedRunner runner = () async { - throw error; - }; - RunZonedGuardedOnError onError = (error, stackTrace) async { - onErrorCalled = true; - }; - final sut = fixture.getSut(runner: runner, onError: onError); - - await sut.call(fixture.hub, fixture.options); + + SentryRunZonedGuarded.sentryRunZonedGuarded( + () { + throw error; + }, + (error, stackTrace) { + onErrorCalled = true; + }, + ); + await Future.delayed(Duration(milliseconds: 10)); expect(onErrorCalled, true); }); + test('calls zoneSpecification print', () async { + var printCalled = false; + + final zoneSpecification = ZoneSpecification( + print: (self, parent, zone, line) { + printCalled = true; + }, + ); + SentryRunZonedGuarded.sentryRunZonedGuarded( + () { + print('foo'); + }, + null, + zoneSpecification: zoneSpecification, + ); + + await Future.delayed(Duration(milliseconds: 10)); + + expect(printCalled, true); + }); + test('sets level to error instead of fatal', () async { final exception = StateError('error'); final stackTrace = StackTrace.current; @@ -64,10 +92,13 @@ void main() { final client = MockSentryClient(); hub.bindClient(client); - final sut = fixture.getSut(runner: () async {}); - fixture.options.markAutomaticallyCollectedErrorsAsFatal = false; - await sut.captureError(hub, fixture.options, exception, stackTrace); + await SentryRunZonedGuarded.captureError( + hub, + fixture.options, + exception, + stackTrace, + ); final capturedEvent = client.captureEventCalls.last.event; expect(capturedEvent.level, SentryLevel.error); @@ -78,10 +109,4 @@ void main() { class Fixture { final hub = MockHub(); final options = defaultTestOptions()..tracesSampleRate = 1.0; - - RunZonedGuardedIntegration getSut( - {required RunZonedGuardedRunner runner, - RunZonedGuardedOnError? onError}) { - return RunZonedGuardedIntegration(runner, onError); - } } diff --git a/flutter/lib/src/sentry_flutter.dart b/flutter/lib/src/sentry_flutter.dart index f4252fdda1..20797ab359 100644 --- a/flutter/lib/src/sentry_flutter.dart +++ b/flutter/lib/src/sentry_flutter.dart @@ -78,13 +78,8 @@ mixin SentryFlutter { // If onError is not supported and no custom zone exists, use runZonedGuarded to capture errors. final bool useRunZonedGuarded = !isOnErrorSupported && !customZoneExists; - // Retrieve the onError callback set by the user if a custom zone exists. - final RunZonedGuardedOnError? additionalOnError = - customZoneExists ? Zone.current.handleUncaughtError : null; - - RunZonedGuardedOnError? runZonedGuardedOnError = useRunZonedGuarded - ? _createRunZonedGuardedOnError(additionalOnError: additionalOnError) - : null; + RunZonedGuardedOnError? runZonedGuardedOnError = + useRunZonedGuarded ? _createRunZonedGuardedOnError() : null; // first step is to install the native integration and set default values, // so we are able to capture future errors. @@ -213,19 +208,13 @@ mixin SentryFlutter { return integrations; } - static RunZonedGuardedOnError _createRunZonedGuardedOnError({ - RunZonedGuardedOnError? additionalOnError, - }) { + static RunZonedGuardedOnError _createRunZonedGuardedOnError() { return (Object error, StackTrace stackTrace) async { final errorDetails = FlutterErrorDetails( exception: error, stack: stackTrace, ); FlutterError.dumpErrorToConsole(errorDetails, forceReport: true); - - if (additionalOnError != null) { - additionalOnError(error, stackTrace); - } }; }