diff --git a/.changeset/quiet-handlers-forward.md b/.changeset/quiet-handlers-forward.md new file mode 100644 index 00000000..af7d4cd5 --- /dev/null +++ b/.changeset/quiet-handlers-forward.md @@ -0,0 +1,5 @@ +--- +"posthog_flutter": patch +--- + +Preserve chained error handlers after teardown and prevent duplicate exception captures when error tracking restarts. diff --git a/posthog_flutter/lib/src/error_tracking/posthog_error_tracking_autocapture_integration.dart b/posthog_flutter/lib/src/error_tracking/posthog_error_tracking_autocapture_integration.dart index fa5d826d..5066902c 100644 --- a/posthog_flutter/lib/src/error_tracking/posthog_error_tracking_autocapture_integration.dart +++ b/posthog_flutter/lib/src/error_tracking/posthog_error_tracking_autocapture_integration.dart @@ -24,6 +24,8 @@ class PostHogErrorTrackingAutoCaptureIntegration { // Store original handlers (we'll chain with them from our handler) FlutterExceptionHandler? _originalFlutterErrorHandler; ErrorCallback? _originalPlatformErrorHandler; + FlutterExceptionHandler? _flutterErrorHandler; + ErrorCallback? _platformErrorHandler; // Isolate error handling final IsolateErrorHandler _isolateErrorHandler = IsolateErrorHandler(); @@ -103,10 +105,12 @@ class PostHogErrorTrackingAutoCaptureIntegration { _isEnabled = false; // Restore original handlers only if our own handler is still set - if (FlutterError.onError == _posthogFlutterErrorHandler) { + if (_flutterErrorHandler != null && + FlutterError.onError == _flutterErrorHandler) { FlutterError.onError = _originalFlutterErrorHandler; } - if (PlatformDispatcher.instance.onError == _posthogPlatformErrorHandler) { + if (_platformErrorHandler != null && + PlatformDispatcher.instance.onError == _platformErrorHandler) { PlatformDispatcher.instance.onError = _originalPlatformErrorHandler; } @@ -116,18 +120,25 @@ class PostHogErrorTrackingAutoCaptureIntegration { // release refs _originalFlutterErrorHandler = null; _originalPlatformErrorHandler = null; + _flutterErrorHandler = null; + _platformErrorHandler = null; } /// Flutter framework error handler void _setupFlutterErrorHandler() { - // prevent circular calls - if (FlutterError.onError == _posthogFlutterErrorHandler) { - return; - } - - _originalFlutterErrorHandler = FlutterError.onError; - - FlutterError.onError = _posthogFlutterErrorHandler; + final originalHandler = FlutterError.onError; + _originalFlutterErrorHandler = originalHandler; + + // Retained wrappers must keep their own delegate and stay inactive on restart. + late final FlutterExceptionHandler handler; + handler = (details) { + if (identical(_flutterErrorHandler, handler)) { + _posthogFlutterErrorHandler(details); + } + originalHandler?.call(details); + }; + _flutterErrorHandler = handler; + FlutterError.onError = handler; } void _posthogFlutterErrorHandler(FlutterErrorDetails details) { @@ -167,9 +178,6 @@ class PostHogErrorTrackingAutoCaptureIntegration { "Error not captured because FlutterErrorDetails.silent is true and captureSilentFlutterErrors is false", ); } - - // Call the original handler, if any - _originalFlutterErrorHandler?.call(details); } /// Platform error handler for Dart runtime errors @@ -180,16 +188,21 @@ class PostHogErrorTrackingAutoCaptureIntegration { return; } - // prevent circular calls - if (PlatformDispatcher.instance.onError == _posthogPlatformErrorHandler) { - return; - } - - _originalPlatformErrorHandler = PlatformDispatcher.instance.onError; - PlatformDispatcher.instance.onError = _posthogPlatformErrorHandler; + final originalHandler = PlatformDispatcher.instance.onError; + _originalPlatformErrorHandler = originalHandler; + + late final ErrorCallback handler; + handler = (error, stackTrace) { + if (identical(_platformErrorHandler, handler)) { + _posthogPlatformErrorHandler(error, stackTrace); + } + return originalHandler?.call(error, stackTrace) ?? false; + }; + _platformErrorHandler = handler; + PlatformDispatcher.instance.onError = handler; } - bool _posthogPlatformErrorHandler(Object error, StackTrace stackTrace) { + void _posthogPlatformErrorHandler(Object error, StackTrace stackTrace) { final wrappedError = PostHogException( source: error, mechanism: 'PlatformDispatcher', @@ -197,10 +210,6 @@ class PostHogErrorTrackingAutoCaptureIntegration { ); _captureException(error: wrappedError, stackTrace: stackTrace); - - // Call the original handler, if any - // False otherwise, so that default fallback mechanism is used - return _originalPlatformErrorHandler?.call(error, stackTrace) ?? false; } /// Isolate error handler for current isolate errors diff --git a/posthog_flutter/test/posthog_error_tracking_handler_lifecycle_test.dart b/posthog_flutter/test/posthog_error_tracking_handler_lifecycle_test.dart new file mode 100644 index 00000000..8971d01e --- /dev/null +++ b/posthog_flutter/test/posthog_error_tracking_handler_lifecycle_test.dart @@ -0,0 +1,191 @@ +import 'dart:ui'; + +import 'package:flutter/foundation.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:posthog_flutter/src/error_tracking/posthog_error_tracking_autocapture_integration.dart'; +import 'package:posthog_flutter/src/posthog_config.dart'; + +import 'posthog_flutter_platform_interface_fake.dart'; + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + + for (final platformHandler in [false, true]) { + group( + platformHandler ? 'PlatformDispatcher.onError' : 'FlutterError.onError', + () { + late FlutterExceptionHandler? savedFlutterHandler; + late ErrorCallback? savedPlatformHandler; + late PostHogErrorTrackingConfig config; + late PosthogFlutterPlatformFake posthog; + late PostHogErrorTrackingAutoCaptureIntegration integration; + late bool Function() dispatch; + var upstreamCalls = 0; + var wrapperCalls = 0; + + setUp(() { + savedFlutterHandler = FlutterError.onError; + savedPlatformHandler = PlatformDispatcher.instance.onError; + upstreamCalls = 0; + wrapperCalls = 0; + posthog = PosthogFlutterPlatformFake(); + config = PostHogErrorTrackingConfig() + ..captureFlutterErrors = !platformHandler + ..capturePlatformDispatcherErrors = platformHandler; + + if (platformHandler) { + PlatformDispatcher.instance.onError = (error, stack) { + upstreamCalls++; + return true; + }; + } else { + FlutterError.onError = (details) => upstreamCalls++; + } + integration = PostHogErrorTrackingAutoCaptureIntegration.install( + config: config, + posthog: posthog, + )!; + + if (platformHandler) { + final previous = PlatformDispatcher.instance.onError!; + PlatformDispatcher.instance.onError = (error, stack) { + if (++wrapperCalls > 1) { + throw StateError('handler chain recursed'); + } + return previous(error, stack); + }; + dispatch = () => PlatformDispatcher.instance.onError!( + StateError('handler lifecycle repro'), + StackTrace.current, + ); + } else { + final previous = FlutterError.onError!; + FlutterError.onError = (details) { + if (++wrapperCalls > 1) { + throw StateError('handler chain recursed'); + } + previous(details); + }; + dispatch = () { + FlutterError.onError!(FlutterErrorDetails( + exception: StateError('handler lifecycle repro'), + stack: StackTrace.current, + )); + return true; + }; + } + }); + + tearDown(() { + PostHogErrorTrackingAutoCaptureIntegration.uninstall(); + FlutterError.onError = savedFlutterHandler; + PlatformDispatcher.instance.onError = savedPlatformHandler; + }); + + test('installed handler captures once and preserves the upstream chain', + () { + final handled = dispatch(); + + expect(posthog.capturedExceptions, hasLength(1)); + expect(wrapperCalls, 1); + expect(upstreamCalls, 1); + expect(handled, isTrue); + }); + + test('uninstall restores a null upstream handler', () { + PostHogErrorTrackingAutoCaptureIntegration.uninstall(); + if (platformHandler) { + PlatformDispatcher.instance.onError = null; + } else { + FlutterError.onError = null; + } + PostHogErrorTrackingAutoCaptureIntegration.install( + config: config, + posthog: posthog, + ); + + final handled = dispatch(); + expect(posthog.capturedExceptions, hasLength(1)); + if (platformHandler) expect(handled, isFalse); + + PostHogErrorTrackingAutoCaptureIntegration.uninstall(); + + expect( + platformHandler + ? PlatformDispatcher.instance.onError + : FlutterError.onError, + isNull, + ); + }); + + test('uninstall preserves the upstream handler chain', () { + PostHogErrorTrackingAutoCaptureIntegration.uninstall(); + + final handled = dispatch(); + + expect(wrapperCalls, 1); + expect(upstreamCalls, 1); + expect(handled, isTrue); + }); + + test('uninstall stops captures from a retained handler', () { + PostHogErrorTrackingAutoCaptureIntegration.uninstall(); + + dispatch(); + + expect(posthog.capturedExceptions, isEmpty); + }); + + test('restart captures once without a circular handler chain', () { + integration.stop(); + integration.start(); + + final handled = dispatch(); + + expect(posthog.capturedExceptions, hasLength(1)); + expect(wrapperCalls, 1); + expect(upstreamCalls, 1); + expect(handled, isTrue); + }); + + test('repeated reinstalls leave only the current integration capturing', + () { + final retiredPlatforms = [posthog]; + for (var i = 0; i < 3; i++) { + PostHogErrorTrackingAutoCaptureIntegration.uninstall(); + posthog = PosthogFlutterPlatformFake(); + PostHogErrorTrackingAutoCaptureIntegration.install( + config: config, + posthog: posthog, + ); + if (i < 2) retiredPlatforms.add(posthog); + } + + final handled = dispatch(); + + expect(posthog.capturedExceptions, hasLength(1)); + for (final retired in retiredPlatforms) { + expect(retired.capturedExceptions, isEmpty); + } + expect(wrapperCalls, 1); + expect(upstreamCalls, 1); + expect(handled, isTrue); + }); + + test('reinstall captures once and preserves the upstream chain', () { + PostHogErrorTrackingAutoCaptureIntegration.uninstall(); + PostHogErrorTrackingAutoCaptureIntegration.install( + config: config, + posthog: posthog, + ); + + final handled = dispatch(); + + expect(posthog.capturedExceptions, hasLength(1)); + expect(wrapperCalls, 1); + expect(upstreamCalls, 1); + expect(handled, isTrue); + }); + }); + } +}