Skip to content

Commit b6df2e3

Browse files
buenaflorclaude
buenaflor
and
claude
authored
fix(dart): Catch client exceptions in HttpTransport.send (#3490)
* fix(dart): Catch client exceptions in HttpTransport.send Prevents unhandled exceptions like `ClientException: Connection closed before full header was received` from crashing the host application. The error is logged via internalLogger and only rethrown in automatedTestMode. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> * Update * Update CHANGELOG * Update * Update * Implement lost event recording in HttpTransport for network errors Enhance the HttpTransport class to record lost events when a network error occurs during envelope transmission. This includes logging discarded events for both Sentry transactions and spans. Add corresponding tests to verify the functionality when client exceptions are thrown. * Refactor lost event handling in HttpTransport and TransportUtils Consolidate lost event recording logic into TransportUtils for better reusability. Update HttpTransport to utilize the new method for recording lost events on network errors. Enhance logging for response statuses, including rate limit handling. Remove redundant lost event recording method from HttpTransport. * Update * Update * Rename to recordLostEvents --------- Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 47e5157 commit b6df2e3

5 files changed

Lines changed: 139 additions & 61 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,10 @@
22

33
## Unreleased
44

5+
### Fixes
6+
7+
- Catch client exceptions in HttpTransport.send ([#3490](https://github.com/getsentry/sentry-dart/pull/3490))
8+
59
### Internals
610

711
- Remove deprecated `beforeMetricCallback` from options ([#3484](https://github.com/getsentry/sentry-dart/pull/3450))

‎packages/dart/lib/src/transport/http_transport.dart‎

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,12 @@ import 'package:http/http.dart';
55

66
import '../http_client/client_provider.dart'
77
if (dart.library.io) '../http_client/io_client_provider.dart';
8+
import '../client_reports/discard_reason.dart';
89
import '../noop_client.dart';
910
import '../protocol.dart';
1011
import '../sentry_envelope.dart';
1112
import '../sentry_options.dart';
13+
import '../utils/internal_logger.dart';
1214
import '../utils/transport_utils.dart';
1315
import 'http_transport_request_handler.dart';
1416
import 'rate_limiter.dart';
@@ -39,20 +41,35 @@ class HttpTransport implements Transport {
3941

4042
final streamedRequest = await _requestHandler.createRequest(envelope);
4143

42-
final response = await _options.httpClient
43-
.send(streamedRequest)
44-
.then(Response.fromStream);
44+
final Response response;
45+
try {
46+
response = await _options.httpClient
47+
.send(streamedRequest)
48+
.then(Response.fromStream);
49+
} catch (error, stackTrace) {
50+
internalLogger.error('Failed to send envelope',
51+
error: error, stackTrace: stackTrace);
52+
TransportUtils.recordLostEvents(
53+
_options, envelope, DiscardReason.networkError);
54+
if (_options.automatedTestMode) {
55+
rethrow;
56+
}
57+
return SentryId.empty();
58+
}
4559

4660
_updateRetryAfterLimits(response);
4761

48-
TransportUtils.logResponse(_options, envelope, response, target: 'Sentry');
62+
TransportUtils.logResponse(envelope, response, target: 'Sentry');
4963

5064
if (response.statusCode == 200) {
5165
return _parseEventId(response);
5266
}
67+
if (response.statusCode >= 400 && response.statusCode != 429) {
68+
TransportUtils.recordLostEvents(
69+
_options, envelope, DiscardReason.networkError);
70+
}
5371
if (response.statusCode == 429) {
54-
_options.log(
55-
SentryLevel.warning, 'Rate limit reached, failed to send envelope');
72+
internalLogger.warning('Rate limit reached, failed to send envelope');
5673
}
5774
return SentryId.empty();
5875
}
@@ -61,8 +78,9 @@ class HttpTransport implements Transport {
6178
try {
6279
final eventId = json.decode(response.body)['id'];
6380
return eventId != null ? SentryId.fromId(eventId) : null;
64-
} catch (e) {
65-
_options.log(SentryLevel.error, 'Error parsing response: $e');
81+
} catch (error, stackTrace) {
82+
internalLogger.error('Error parsing response',
83+
error: error, stackTrace: stackTrace);
6684
if (_options.automatedTestMode) {
6785
rethrow;
6886
}

‎packages/dart/lib/src/transport/spotlight_http_transport.dart‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,7 @@ class SpotlightHttpTransport extends Transport {
4949
.send(spotlightRequest)
5050
.then(Response.fromStream);
5151

52-
TransportUtils.logResponse(_options, envelope, response,
53-
target: 'Spotlight');
52+
TransportUtils.logResponse(envelope, response, target: 'Spotlight');
5453
}
5554
}
5655

‎packages/dart/lib/src/utils/transport_utils.dart‎

Lines changed: 25 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,41 +5,38 @@ import '../protocol.dart';
55
import '../sentry_envelope.dart';
66
import '../sentry_options.dart';
77
import '../transport/data_category.dart';
8+
import 'internal_logger.dart';
89

910
class TransportUtils {
10-
static void logResponse(
11-
SentryOptions options, SentryEnvelope envelope, Response response,
11+
static void logResponse(SentryEnvelope envelope, Response response,
1212
{required String target}) {
1313
if (response.statusCode != 200) {
14-
if (options.debug) {
15-
options.log(
16-
SentryLevel.error,
17-
'Error, statusCode = ${response.statusCode}, body = ${response.body}',
18-
);
19-
}
14+
internalLogger.error(() =>
15+
'Failed to send envelope, statusCode = ${response.statusCode}, body = ${response.body}');
16+
} else {
17+
internalLogger.debug(
18+
() =>
19+
'Envelope ${envelope.header.eventId ?? "--"} was sent successfully to $target.',
20+
);
21+
}
22+
}
2023

21-
if (response.statusCode >= 400 && response.statusCode != 429) {
22-
for (final item in envelope.items) {
23-
options.recorder.recordLostEvent(
24-
DiscardReason.networkError,
25-
DataCategory.fromItemType(item.header.type),
26-
);
24+
static void recordLostEvents(
25+
SentryOptions options, SentryEnvelope envelope, DiscardReason reason) {
26+
for (final item in envelope.items) {
27+
options.recorder.recordLostEvent(
28+
reason,
29+
DataCategory.fromItemType(item.header.type),
30+
);
2731

28-
final originalObject = item.originalObject;
29-
if (originalObject is SentryTransaction) {
30-
options.recorder.recordLostEvent(
31-
DiscardReason.networkError,
32-
DataCategory.span,
33-
count: originalObject.spans.length + 1,
34-
);
35-
}
36-
}
32+
final originalObject = item.originalObject;
33+
if (originalObject is SentryTransaction) {
34+
options.recorder.recordLostEvent(
35+
reason,
36+
DataCategory.span,
37+
count: originalObject.spans.length + 1,
38+
);
3739
}
38-
} else {
39-
options.log(
40-
SentryLevel.debug,
41-
'Envelope ${envelope.header.eventId ?? "--"} was sent successfully to $target.',
42-
);
4340
}
4441
}
4542
}

‎packages/dart/test/transport/http_transport_test.dart‎

Lines changed: 83 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,89 @@ void main() {
5050

5151
expect(body, envelopeData);
5252
});
53+
54+
test('returns empty SentryId when client throws exception', () async {
55+
final httpMock = MockClient((http.Request request) async {
56+
throw http.ClientException(
57+
'Connection closed before full header was received');
58+
});
59+
60+
fixture.options.automatedTestMode = false;
61+
final sut = fixture.getSut(httpMock, MockRateLimiter());
62+
63+
final sentryEvent = SentryEvent();
64+
final envelope = SentryEnvelope.fromEvent(
65+
sentryEvent,
66+
fixture.options.sdk,
67+
dsn: fixture.options.dsn,
68+
);
69+
70+
final result = await sut.send(envelope);
71+
72+
expect(result, SentryId.empty());
73+
});
74+
75+
test('records lost event when client throws exception', () async {
76+
final httpMock = MockClient((http.Request request) async {
77+
throw http.ClientException(
78+
'Connection closed before full header was received');
79+
});
80+
81+
fixture.options.automatedTestMode = false;
82+
final sut = fixture.getSut(httpMock, MockRateLimiter());
83+
84+
final sentryEvent = SentryEvent();
85+
final envelope = SentryEnvelope.fromEvent(
86+
sentryEvent,
87+
fixture.options.sdk,
88+
dsn: fixture.options.dsn,
89+
);
90+
91+
await sut.send(envelope);
92+
93+
expect(fixture.clientReportRecorder.discardedEvents.length, 1);
94+
expect(fixture.clientReportRecorder.discardedEvents.first.reason,
95+
DiscardReason.networkError);
96+
expect(fixture.clientReportRecorder.discardedEvents.first.category,
97+
DataCategory.error);
98+
});
99+
100+
test('records lost transaction and spans when client throws exception',
101+
() async {
102+
final httpMock = MockClient((http.Request request) async {
103+
throw http.ClientException(
104+
'Connection closed before full header was received');
105+
});
106+
107+
fixture.options.automatedTestMode = false;
108+
final sut = fixture.getSut(httpMock, MockRateLimiter());
109+
110+
final transaction = fixture.getTransaction();
111+
transaction.tracer.startChild('child1');
112+
transaction.tracer.startChild('child2');
113+
final envelope = SentryEnvelope.fromTransaction(
114+
transaction,
115+
fixture.options.sdk,
116+
dsn: fixture.options.dsn,
117+
);
118+
119+
await sut.send(envelope);
120+
121+
final transactionDiscardedEvent = fixture
122+
.clientReportRecorder.discardedEvents
123+
.firstWhereOrNull((element) =>
124+
element.category == DataCategory.transaction &&
125+
element.reason == DiscardReason.networkError);
126+
127+
final spanDiscardedEvent = fixture.clientReportRecorder.discardedEvents
128+
.firstWhereOrNull((element) =>
129+
element.category == DataCategory.span &&
130+
element.reason == DiscardReason.networkError);
131+
132+
expect(transactionDiscardedEvent, isNotNull);
133+
expect(spanDiscardedEvent, isNotNull);
134+
expect(spanDiscardedEvent!.quantity, 3);
135+
});
53136
});
54137

55138
group('updateRetryAfterLimits', () {
@@ -83,10 +166,6 @@ void main() {
83166
expect(mockRateLimiter.errorCode, 429);
84167
expect(mockRateLimiter.retryAfterHeader, '1');
85168
expect(mockRateLimiter.sentryRateLimitHeader, isNull);
86-
87-
expect(fixture.loggedLevel, SentryLevel.warning);
88-
expect(
89-
fixture.loggedMessage, 'Rate limit reached, failed to send envelope');
90169
});
91170

92171
test('sentryRateLimitHeader', () async {
@@ -236,10 +315,6 @@ void main() {
236315
await sut.send(envelope);
237316

238317
expect(fixture.clientReportRecorder.discardedEvents.isEmpty, isTrue);
239-
240-
expect(fixture.loggedLevel, SentryLevel.warning);
241-
expect(
242-
fixture.loggedMessage, 'Rate limit reached, failed to send envelope');
243318
});
244319

245320
test('does record lost event for error >= 500', () async {
@@ -271,7 +346,6 @@ class Fixture {
271346

272347
HttpTransport getSut(http.Client client, RateLimiter rateLimiter) {
273348
options.debug = true;
274-
options.log = mockLogger;
275349
options.httpClient = client;
276350
options.recorder = clientReportRecorder;
277351
options.clock = () {
@@ -289,18 +363,4 @@ class Fixture {
289363
final tracer = SentryTracer(context, MockHub());
290364
return SentryTransaction(tracer);
291365
}
292-
293-
SentryLevel? loggedLevel;
294-
String? loggedMessage;
295-
296-
void mockLogger(
297-
SentryLevel level,
298-
String message, {
299-
String? logger,
300-
Object? exception,
301-
StackTrace? stackTrace,
302-
}) {
303-
loggedLevel = level;
304-
loggedMessage = message;
305-
}
306366
}

0 commit comments

Comments
 (0)