Skip to content

Commit 8775665

Browse files
authored
Flush logs if client/hub/sdk is closed (#3335)
1 parent e5b87f8 commit 8775665

File tree

9 files changed

+99
-21
lines changed

9 files changed

+99
-21
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
- Refactor `captureReplay` and `setReplayConfig` to use FFI/JNI ([#3318](https://github.com/getsentry/sentry-dart/pull/3318))
1212
- Refactor `init` to use FFI/JNI ([#3324](https://github.com/getsentry/sentry-dart/pull/3324))
13+
- Flush logs if client/hub/sdk is closed ([#3335](https://github.com/getsentry/sentry-dart/pull/3335))
1314

1415
## 9.8.0
1516

packages/dart/lib/src/hub.dart

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -399,7 +399,10 @@ class Hub {
399399
final item = _peek();
400400

401401
try {
402-
item.client.close();
402+
final close = item.client.close();
403+
if (close is Future<void>) {
404+
await close;
405+
}
403406
} catch (exception, stackTrace) {
404407
_options.log(
405408
SentryLevel.error,

packages/dart/lib/src/noop_log_batcher.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ class NoopLogBatcher implements SentryLogBatcher {
88
FutureOr<void> addLog(SentryLog log) {}
99

1010
@override
11-
Future<void> flush() async {}
11+
FutureOr<void> flush() {}
1212
}

packages/dart/lib/src/noop_sentry_client.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class NoOpSentryClient implements SentryClient {
5151
SentryId.empty();
5252

5353
@override
54-
Future<void> close() async {}
54+
FutureOr<void> close() {}
5555

5656
@override
5757
Future<SentryId> captureTransaction(

packages/dart/lib/src/sentry_client.dart

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -582,7 +582,11 @@ class SentryClient {
582582
}
583583
}
584584

585-
void close() {
585+
FutureOr<void> close() {
586+
final flush = _options.logBatcher.flush();
587+
if (flush is Future<void>) {
588+
return flush.then((_) => _options.httpClient.close());
589+
}
586590
_options.httpClient.close();
587591
}
588592

packages/dart/lib/src/sentry_log_batcher.dart

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,7 @@ class SentryLogBatcher {
5252
}
5353

5454
/// Flushes the buffer immediately, sending all buffered logs.
55-
void flush() {
56-
_performFlushLogs();
57-
}
55+
FutureOr<void> flush() => _performFlushLogs();
5856

5957
void _startTimer() {
6058
_flushTimer = Timer(_flushTimeout, () {
@@ -66,7 +64,7 @@ class SentryLogBatcher {
6664
});
6765
}
6866

69-
void _performFlushLogs() {
67+
FutureOr<void> _performFlushLogs() {
7068
// Reset timer state first
7169
_flushTimer?.cancel();
7270
_flushTimer = null;
@@ -81,17 +79,16 @@ class SentryLogBatcher {
8179
SentryLevel.debug,
8280
'SentryLogBatcher: No logs to flush.',
8381
);
84-
return;
85-
}
86-
87-
try {
88-
final envelope = SentryEnvelope.fromLogsData(logsToSend, _options.sdk);
89-
_options.transport.send(envelope);
90-
} catch (error) {
91-
_options.log(
92-
SentryLevel.error,
93-
'Failed to send batched logs: $error',
94-
);
82+
} else {
83+
try {
84+
final envelope = SentryEnvelope.fromLogsData(logsToSend, _options.sdk);
85+
return _options.transport.send(envelope).then((_) => null);
86+
} catch (error) {
87+
_options.log(
88+
SentryLevel.error,
89+
'Failed to create envelope for batched logs: $error',
90+
);
91+
}
9592
}
9693
}
9794
}

packages/dart/test/mocks/mock_log_batcher.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ class MockLogBatcher implements SentryLogBatcher {
1313
}
1414

1515
@override
16-
Future<void> flush() async {
16+
FutureOr<void> flush() async {
1717
flushCalls.add(null);
1818
}
1919
}

packages/dart/test/sentry_client_test.dart

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ import 'package:sentry/src/transport/spotlight_http_transport.dart';
1818
import 'package:sentry/src/utils/iterable_utils.dart';
1919
import 'package:test/test.dart';
2020
import 'package:sentry/src/noop_log_batcher.dart';
21+
import 'package:sentry/src/sentry_log_batcher.dart';
22+
import 'package:mockito/mockito.dart';
23+
import 'package:http/http.dart' as http;
2124

2225
import 'mocks.dart';
2326
import 'mocks/mock_client_report_recorder.dart';
@@ -2610,6 +2613,56 @@ void main() {
26102613
await client.captureEvent(fakeEvent, stackTrace: StackTrace.current);
26112614
});
26122615
});
2616+
2617+
group('SentryClient close', () {
2618+
late Fixture fixture;
2619+
2620+
setUp(() {
2621+
fixture = Fixture();
2622+
});
2623+
2624+
test('waits for log batcher flush before closing http client', () async {
2625+
// Create a mock HTTP client that tracks when close is called
2626+
final mockHttpClient = MockHttpClient();
2627+
fixture.options.httpClient = mockHttpClient;
2628+
2629+
fixture.options.enableLogs = true;
2630+
final client = fixture.getSut();
2631+
2632+
// Create a completer to control when flush completes
2633+
final flushCompleter = Completer<void>();
2634+
bool flushStarted = false;
2635+
2636+
// Create a mock log batcher with async flush
2637+
final mockLogBatcher = MockLogBatcherWithAsyncFlush(
2638+
onFlush: () async {
2639+
flushStarted = true;
2640+
// Wait for the completer to complete
2641+
await flushCompleter.future;
2642+
},
2643+
);
2644+
fixture.options.logBatcher = mockLogBatcher;
2645+
2646+
// Start close() in the background
2647+
final closeFuture = client.close();
2648+
2649+
// Wait a bit longer to ensure flush has started
2650+
await Future.delayed(Duration(milliseconds: 50));
2651+
2652+
// Verify flush has started but HTTP client is not closed yet
2653+
expect(flushStarted, true, reason: 'Flush should have started');
2654+
verifyNever(mockHttpClient.close());
2655+
2656+
// Complete the flush
2657+
flushCompleter.complete();
2658+
2659+
// Wait for close to complete
2660+
await closeFuture;
2661+
2662+
// Now verify HTTP client was closed
2663+
verify(mockHttpClient.close()).called(1);
2664+
});
2665+
});
26132666
}
26142667

26152668
Future<SentryEvent> eventFromEnvelope(SentryEnvelope envelope) async {
@@ -2804,6 +2857,25 @@ class Fixture {
28042857
}
28052858
}
28062859

2860+
class MockHttpClient extends Mock implements http.Client {}
2861+
2862+
class MockLogBatcherWithAsyncFlush implements SentryLogBatcher {
2863+
final Future<void> Function() onFlush;
2864+
final addLogCalls = <SentryLog>[];
2865+
2866+
MockLogBatcherWithAsyncFlush({required this.onFlush});
2867+
2868+
@override
2869+
void addLog(SentryLog log) {
2870+
addLogCalls.add(log);
2871+
}
2872+
2873+
@override
2874+
FutureOr<void> flush() async {
2875+
await onFlush();
2876+
}
2877+
}
2878+
28072879
class ExceptionWithCause {
28082880
ExceptionWithCause(this.cause, this.stackTrace);
28092881

packages/flutter/test/widgets_binding_observer_test.dart

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// ignore_for_file: invalid_use_of_internal_member
22

33
import 'dart:ui';
4+
import 'dart:async';
45

56
import 'package:flutter/foundation.dart';
67
import 'package:flutter/services.dart';
@@ -603,7 +604,7 @@ class MockLogBatcher implements SentryLogBatcher {
603604
void addLog(SentryLog log) {}
604605

605606
@override
606-
Future<void> flush() async {
607+
FutureOr<void> flush() async {
607608
flushCalled = true;
608609
}
609610
}

0 commit comments

Comments
 (0)