Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did a new release, pls update the changelog

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops didn't see it's automatic merged

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh sorry, enabled auto-merge when all criteria is met.


## 9.8.0

Expand Down
5 changes: 4 additions & 1 deletion packages/dart/lib/src/hub.dart
Original file line number Diff line number Diff line change
Expand Up @@ -399,7 +399,10 @@ class Hub {
final item = _peek();

try {
item.client.close();
final close = item.client.close();
if (close is Future<void>) {
await close;
}
} catch (exception, stackTrace) {
_options.log(
SentryLevel.error,
Expand Down
2 changes: 1 addition & 1 deletion packages/dart/lib/src/noop_log_batcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,5 @@ class NoopLogBatcher implements SentryLogBatcher {
FutureOr<void> addLog(SentryLog log) {}

@override
Future<void> flush() async {}
FutureOr<void> flush() {}
}
2 changes: 1 addition & 1 deletion packages/dart/lib/src/noop_sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ class NoOpSentryClient implements SentryClient {
SentryId.empty();

@override
Future<void> close() async {}
FutureOr<void> close() {}

@override
Future<SentryId> captureTransaction(
Expand Down
6 changes: 5 additions & 1 deletion packages/dart/lib/src/sentry_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,11 @@ class SentryClient {
}
}

void close() {
FutureOr<void> close() {
final flush = _options.logBatcher.flush();
if (flush is Future<void>) {
return flush.then((_) => _options.httpClient.close());
}
_options.httpClient.close();
}

Expand Down
27 changes: 12 additions & 15 deletions packages/dart/lib/src/sentry_log_batcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,7 @@ class SentryLogBatcher {
}

/// Flushes the buffer immediately, sending all buffered logs.
void flush() {
_performFlushLogs();
}
FutureOr<void> flush() => _performFlushLogs();

void _startTimer() {
_flushTimer = Timer(_flushTimeout, () {
Expand All @@ -66,7 +64,7 @@ class SentryLogBatcher {
});
}

void _performFlushLogs() {
FutureOr<void> _performFlushLogs() {
// Reset timer state first
_flushTimer?.cancel();
_flushTimer = null;
Expand All @@ -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',
);
}
}
}
}
2 changes: 1 addition & 1 deletion packages/dart/test/mocks/mock_log_batcher.dart
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class MockLogBatcher implements SentryLogBatcher {
}

@override
Future<void> flush() async {
FutureOr<void> flush() async {
flushCalls.add(null);
}
}
72 changes: 72 additions & 0 deletions packages/dart/test/sentry_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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<void>();
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<SentryEvent> eventFromEnvelope(SentryEnvelope envelope) async {
Expand Down Expand Up @@ -2804,6 +2857,25 @@ class Fixture {
}
}

class MockHttpClient extends Mock implements http.Client {}

class MockLogBatcherWithAsyncFlush implements SentryLogBatcher {
final Future<void> Function() onFlush;
final addLogCalls = <SentryLog>[];

MockLogBatcherWithAsyncFlush({required this.onFlush});

@override
void addLog(SentryLog log) {
addLogCalls.add(log);
}

@override
FutureOr<void> flush() async {
await onFlush();
}
}

class ExceptionWithCause {
ExceptionWithCause(this.cause, this.stackTrace);

Expand Down
3 changes: 2 additions & 1 deletion packages/flutter/test/widgets_binding_observer_test.dart
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -603,7 +604,7 @@ class MockLogBatcher implements SentryLogBatcher {
void addLog(SentryLog log) {}

@override
Future<void> flush() async {
FutureOr<void> flush() async {
flushCalled = true;
}
}
Loading