Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 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() async {}
}
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() async {}

@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() async {
final flush = _options.logBatcher.flush();
if (flush is Future<void>) {
await flush;
}
_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