diff --git a/CHANGELOG.md b/CHANGELOG.md index 2fc3ae4858..68042740e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Refactor `captureReplay` and `setReplayConfig` to use FFI/JNI ([#3318](https://github.com/getsentry/sentry-dart/pull/3318)) - Refactor `init` to use FFI/JNI ([#3324](https://github.com/getsentry/sentry-dart/pull/3324)) +- Flush logs if client/hub/sdk is closed ([#3335](https://github.com/getsentry/sentry-dart/pull/3335)) ## 9.8.0 diff --git a/packages/dart/lib/src/hub.dart b/packages/dart/lib/src/hub.dart index 72ef667167..2ceb3006ce 100644 --- a/packages/dart/lib/src/hub.dart +++ b/packages/dart/lib/src/hub.dart @@ -399,7 +399,10 @@ class Hub { final item = _peek(); try { - item.client.close(); + final close = item.client.close(); + if (close is Future) { + await close; + } } catch (exception, stackTrace) { _options.log( SentryLevel.error, diff --git a/packages/dart/lib/src/noop_log_batcher.dart b/packages/dart/lib/src/noop_log_batcher.dart index 7d5c94523d..52242b62a5 100644 --- a/packages/dart/lib/src/noop_log_batcher.dart +++ b/packages/dart/lib/src/noop_log_batcher.dart @@ -8,5 +8,5 @@ class NoopLogBatcher implements SentryLogBatcher { FutureOr addLog(SentryLog log) {} @override - Future flush() async {} + FutureOr flush() {} } diff --git a/packages/dart/lib/src/noop_sentry_client.dart b/packages/dart/lib/src/noop_sentry_client.dart index d071edbf16..05e526e393 100644 --- a/packages/dart/lib/src/noop_sentry_client.dart +++ b/packages/dart/lib/src/noop_sentry_client.dart @@ -51,7 +51,7 @@ class NoOpSentryClient implements SentryClient { SentryId.empty(); @override - Future close() async {} + FutureOr close() {} @override Future captureTransaction( diff --git a/packages/dart/lib/src/sentry_client.dart b/packages/dart/lib/src/sentry_client.dart index b8b1e7e5d0..fc2ec9a092 100644 --- a/packages/dart/lib/src/sentry_client.dart +++ b/packages/dart/lib/src/sentry_client.dart @@ -582,7 +582,11 @@ class SentryClient { } } - void close() { + FutureOr close() { + final flush = _options.logBatcher.flush(); + if (flush is Future) { + return flush.then((_) => _options.httpClient.close()); + } _options.httpClient.close(); } diff --git a/packages/dart/lib/src/sentry_log_batcher.dart b/packages/dart/lib/src/sentry_log_batcher.dart index 8fa4566a11..4c5867fc16 100644 --- a/packages/dart/lib/src/sentry_log_batcher.dart +++ b/packages/dart/lib/src/sentry_log_batcher.dart @@ -52,9 +52,7 @@ class SentryLogBatcher { } /// Flushes the buffer immediately, sending all buffered logs. - void flush() { - _performFlushLogs(); - } + FutureOr flush() => _performFlushLogs(); void _startTimer() { _flushTimer = Timer(_flushTimeout, () { @@ -66,7 +64,7 @@ class SentryLogBatcher { }); } - void _performFlushLogs() { + FutureOr _performFlushLogs() { // Reset timer state first _flushTimer?.cancel(); _flushTimer = null; @@ -81,17 +79,16 @@ class SentryLogBatcher { SentryLevel.debug, 'SentryLogBatcher: No logs to flush.', ); - return; - } - - try { - final envelope = SentryEnvelope.fromLogsData(logsToSend, _options.sdk); - _options.transport.send(envelope); - } catch (error) { - _options.log( - SentryLevel.error, - 'Failed to send batched logs: $error', - ); + } else { + try { + final envelope = SentryEnvelope.fromLogsData(logsToSend, _options.sdk); + return _options.transport.send(envelope).then((_) => null); + } catch (error) { + _options.log( + SentryLevel.error, + 'Failed to create envelope for batched logs: $error', + ); + } } } } diff --git a/packages/dart/test/mocks/mock_log_batcher.dart b/packages/dart/test/mocks/mock_log_batcher.dart index 9de9a8ae5d..3c3d79cbbb 100644 --- a/packages/dart/test/mocks/mock_log_batcher.dart +++ b/packages/dart/test/mocks/mock_log_batcher.dart @@ -13,7 +13,7 @@ class MockLogBatcher implements SentryLogBatcher { } @override - Future flush() async { + FutureOr flush() async { flushCalls.add(null); } } diff --git a/packages/dart/test/sentry_client_test.dart b/packages/dart/test/sentry_client_test.dart index d0423fc2fd..9fa77334ea 100644 --- a/packages/dart/test/sentry_client_test.dart +++ b/packages/dart/test/sentry_client_test.dart @@ -18,6 +18,9 @@ import 'package:sentry/src/transport/spotlight_http_transport.dart'; import 'package:sentry/src/utils/iterable_utils.dart'; import 'package:test/test.dart'; import 'package:sentry/src/noop_log_batcher.dart'; +import 'package:sentry/src/sentry_log_batcher.dart'; +import 'package:mockito/mockito.dart'; +import 'package:http/http.dart' as http; import 'mocks.dart'; import 'mocks/mock_client_report_recorder.dart'; @@ -2610,6 +2613,56 @@ void main() { await client.captureEvent(fakeEvent, stackTrace: StackTrace.current); }); }); + + group('SentryClient close', () { + late Fixture fixture; + + setUp(() { + fixture = Fixture(); + }); + + test('waits for log batcher flush before closing http client', () async { + // Create a mock HTTP client that tracks when close is called + final mockHttpClient = MockHttpClient(); + fixture.options.httpClient = mockHttpClient; + + fixture.options.enableLogs = true; + final client = fixture.getSut(); + + // Create a completer to control when flush completes + final flushCompleter = Completer(); + bool flushStarted = false; + + // Create a mock log batcher with async flush + final mockLogBatcher = MockLogBatcherWithAsyncFlush( + onFlush: () async { + flushStarted = true; + // Wait for the completer to complete + await flushCompleter.future; + }, + ); + fixture.options.logBatcher = mockLogBatcher; + + // Start close() in the background + final closeFuture = client.close(); + + // Wait a bit longer to ensure flush has started + await Future.delayed(Duration(milliseconds: 50)); + + // Verify flush has started but HTTP client is not closed yet + expect(flushStarted, true, reason: 'Flush should have started'); + verifyNever(mockHttpClient.close()); + + // Complete the flush + flushCompleter.complete(); + + // Wait for close to complete + await closeFuture; + + // Now verify HTTP client was closed + verify(mockHttpClient.close()).called(1); + }); + }); } Future eventFromEnvelope(SentryEnvelope envelope) async { @@ -2804,6 +2857,25 @@ class Fixture { } } +class MockHttpClient extends Mock implements http.Client {} + +class MockLogBatcherWithAsyncFlush implements SentryLogBatcher { + final Future Function() onFlush; + final addLogCalls = []; + + MockLogBatcherWithAsyncFlush({required this.onFlush}); + + @override + void addLog(SentryLog log) { + addLogCalls.add(log); + } + + @override + FutureOr flush() async { + await onFlush(); + } +} + class ExceptionWithCause { ExceptionWithCause(this.cause, this.stackTrace); diff --git a/packages/flutter/test/widgets_binding_observer_test.dart b/packages/flutter/test/widgets_binding_observer_test.dart index 063ffff853..d4ed7e8a52 100644 --- a/packages/flutter/test/widgets_binding_observer_test.dart +++ b/packages/flutter/test/widgets_binding_observer_test.dart @@ -1,6 +1,7 @@ // ignore_for_file: invalid_use_of_internal_member import 'dart:ui'; +import 'dart:async'; import 'package:flutter/foundation.dart'; import 'package:flutter/services.dart'; @@ -603,7 +604,7 @@ class MockLogBatcher implements SentryLogBatcher { void addLog(SentryLog log) {} @override - Future flush() async { + FutureOr flush() async { flushCalled = true; } }