Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: HttpClient response capture #1093

Closed
wants to merge 3 commits into from
Closed
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
51 changes: 35 additions & 16 deletions dart/lib/src/http_client/failed_request_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ import 'sentry_http_client.dart';
class FailedRequestClient extends BaseClient {
FailedRequestClient({
this.maxRequestBodySize = MaxRequestBodySize.never,
this.maxResponseBodySize = MaxResponseBodySize.never,
this.failedRequestStatusCodes = const [],
this.captureFailedRequests = true,
this.sendDefaultPii = false,
Expand All @@ -77,17 +78,21 @@ class FailedRequestClient extends BaseClient {
final Client _client;
final Hub _hub;

/// Configures wether to record exceptions for failed requests.
/// Configures whether to record exceptions for failed requests.
/// Examples for captures exceptions are:
/// - In an browser environment this can be requests which fail because of CORS.
/// - In an mobile or desktop application this can be requests which failed
/// because the connection was interrupted.
final bool captureFailedRequests;

/// Configures up to which size request bodies should be included in events.
/// This does not change wether an event is captured.
/// This does not change whether an event is captured.
final MaxRequestBodySize maxRequestBodySize;

/// Configures up to which size response bodies should be included in events.
/// This does not change whether an event is captured.
final MaxResponseBodySize maxResponseBodySize;

/// Describes which HTTP status codes should be considered as a failed
/// requests.
///
Expand All @@ -101,12 +106,13 @@ class FailedRequestClient extends BaseClient {
int? statusCode;
Object? exception;
StackTrace? stackTrace;
StreamedResponse? response;

final stopwatch = Stopwatch();
stopwatch.start();

try {
final response = await _client.send(request);
response = await _client.send(request);
statusCode = response.statusCode;
return response;
} catch (e, st) {
Expand All @@ -118,25 +124,25 @@ class FailedRequestClient extends BaseClient {

// If captureFailedRequests is true, there statusCode is null.
// So just one of these blocks can be called.

var capture = false;
String? reason;
if (captureFailedRequests && exception != null) {
await _captureEvent(
exception: exception,
stackTrace: stackTrace,
request: request,
requestDuration: stopwatch.elapsed,
);
capture = true;
} else if (failedRequestStatusCodes.containsStatusCode(statusCode)) {
final message =
'Event was captured because the request status code was $statusCode';
final httpException = SentryHttpClientError(message);

// Capture an exception if the status code is considered bad
capture = true;
reason =
'Event was captured because the request status code was $statusCode';
exception ??= SentryHttpClientError(reason);
}
if (capture) {
await _captureEvent(
exception: exception ?? httpException,
exception: exception,
stackTrace: stackTrace,
request: request,
reason: message,
requestDuration: stopwatch.elapsed,
response: response,
reason: reason,
);
}
}
Expand All @@ -152,6 +158,7 @@ class FailedRequestClient extends BaseClient {
String? reason,
required Duration requestDuration,
required BaseRequest request,
required StreamedResponse? response,
}) async {
// As far as I can tell there's no way to get the uri without the query part
// so we replace it with an empty string.
Expand All @@ -165,6 +172,7 @@ class FailedRequestClient extends BaseClient {
url: urlWithoutQuery,
queryString: query,
cookies: sendDefaultPii ? request.headers['Cookie'] : null,
// TODO FIXME - this should check [sendDefaultPii], same as DIO integration.
data: _getDataFromRequest(request),
// ignore: deprecated_member_use_from_same_package
other: {
Expand All @@ -183,6 +191,17 @@ class FailedRequestClient extends BaseClient {
throwable: throwableMechanism,
request: sentryRequest,
);

if (response != null) {
event.contexts.response = SentryResponse(
headers: sendDefaultPii ? response.headers : null,
redirected: response.isRedirect,
body: sendDefaultPii ? null : null, // TODO response.stream
statusCode: response.statusCode,
status: response.reasonPhrase,
);
}

await _hub.captureEvent(event, stackTrace: stackTrace);
}

Expand Down
2 changes: 2 additions & 0 deletions dart/lib/src/http_client/sentry_http_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class SentryHttpClient extends BaseClient {
Hub? hub,
bool recordBreadcrumbs = true,
MaxRequestBodySize maxRequestBodySize = MaxRequestBodySize.never,
MaxResponseBodySize maxResponseBodySize = MaxResponseBodySize.never,
List<SentryStatusCode> failedRequestStatusCodes = const [],
bool captureFailedRequests = false,
bool sendDefaultPii = false,
Expand All @@ -94,6 +95,7 @@ class SentryHttpClient extends BaseClient {
failedRequestStatusCodes: failedRequestStatusCodes,
captureFailedRequests: captureFailedRequests,
maxRequestBodySize: maxRequestBodySize,
maxResponseBodySize: maxResponseBodySize,
sendDefaultPii: sendDefaultPii,
hub: _hub,
client: innerClient,
Expand Down
2 changes: 1 addition & 1 deletion dart/lib/src/platform_checker.dart
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class PlatformChecker {
: 'profile';
}

/// Indicates wether a native integration is available.
/// Indicates whether a native integration is available.
bool get hasNativeIntegration {
if (isWeb) {
return false;
Expand Down
44 changes: 20 additions & 24 deletions dart/lib/src/protocol/max_body_size.dart
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,16 @@ enum MaxRequestBodySize {

extension MaxRequestBodySizeX on MaxRequestBodySize {
bool shouldAddBody(int contentLength) {
if (this == MaxRequestBodySize.never) {
return false;
}
if (this == MaxRequestBodySize.always) {
return true;
}
if (this == MaxRequestBodySize.medium && contentLength <= _mediumSize) {
return true;
}

if (this == MaxRequestBodySize.small && contentLength <= _smallSize) {
return true;
switch (this) {
case MaxRequestBodySize.never:
break;
case MaxRequestBodySize.small:
return contentLength <= _smallSize;
case MaxRequestBodySize.medium:
return contentLength <= _mediumSize;
case MaxRequestBodySize.always:
return true;
// No default here to get a warning when new enum value is added.
}
return false;
}
Expand All @@ -61,18 +59,16 @@ enum MaxResponseBodySize {

extension MaxResponseBodySizeX on MaxResponseBodySize {
bool shouldAddBody(int contentLength) {
if (this == MaxResponseBodySize.never) {
return false;
}
if (this == MaxResponseBodySize.always) {
return true;
}
if (this == MaxResponseBodySize.medium && contentLength <= _mediumSize) {
return true;
}

if (this == MaxResponseBodySize.small && contentLength <= _smallSize) {
return true;
switch (this) {
case MaxResponseBodySize.never:
break;
case MaxResponseBodySize.small:
return contentLength <= _smallSize;
case MaxResponseBodySize.medium:
return contentLength <= _mediumSize;
case MaxResponseBodySize.always:
return true;
// No default here to get a warning when new enum value is added.
}
return false;
}
Expand Down
95 changes: 74 additions & 21 deletions dart/test/http_client/failed_request_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,7 @@ void main() {
expect(fixture.transport.calls, 1);
expect(event.request?.headers.isEmpty, true);
expect(event.request?.cookies, isNull);
expect(event.request?.data, isNull);
});

test('pii is not send on invalid status code', () async {
Expand All @@ -180,22 +181,22 @@ void main() {
test('request body is included according to $MaxRequestBodySize', () async {
final scenarios = [
// never
MaxRequestBodySizeTestConfig(MaxRequestBodySize.never, 0, false),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.never, 4001, false),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.never, 10001, false),
MaxBodySizeTestConfig(MaxRequestBodySize.never, 0, false),
MaxBodySizeTestConfig(MaxRequestBodySize.never, 4001, false),
MaxBodySizeTestConfig(MaxRequestBodySize.never, 10001, false),
// always
MaxRequestBodySizeTestConfig(MaxRequestBodySize.always, 0, true),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.always, 4001, true),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.always, 10001, true),
MaxBodySizeTestConfig(MaxRequestBodySize.always, 0, true),
MaxBodySizeTestConfig(MaxRequestBodySize.always, 4001, true),
MaxBodySizeTestConfig(MaxRequestBodySize.always, 10001, true),
// small
MaxRequestBodySizeTestConfig(MaxRequestBodySize.small, 0, true),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.small, 4000, true),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.small, 4001, false),
MaxBodySizeTestConfig(MaxRequestBodySize.small, 0, true),
MaxBodySizeTestConfig(MaxRequestBodySize.small, 4000, true),
MaxBodySizeTestConfig(MaxRequestBodySize.small, 4001, false),
// medium
MaxRequestBodySizeTestConfig(MaxRequestBodySize.medium, 0, true),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.medium, 4001, true),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.medium, 10000, true),
MaxRequestBodySizeTestConfig(MaxRequestBodySize.medium, 10001, false),
MaxBodySizeTestConfig(MaxRequestBodySize.medium, 0, true),
MaxBodySizeTestConfig(MaxRequestBodySize.medium, 4001, true),
MaxBodySizeTestConfig(MaxRequestBodySize.medium, 10000, true),
MaxBodySizeTestConfig(MaxRequestBodySize.medium, 10001, false),
];

for (final scenario in scenarios) {
Expand All @@ -204,7 +205,7 @@ void main() {
final sut = fixture.getSut(
client: createThrowingClient(),
captureFailedRequests: true,
maxRequestBodySize: scenario.maxRequestBodySize,
maxRequestBodySize: scenario.maxBodySize,
);

final request = Request('GET', requestUri)
Expand All @@ -220,10 +221,58 @@ void main() {

final eventCall = fixture.transport.events.first;
final capturedRequest = eventCall.request;
expect(
capturedRequest?.data,
scenario.shouldBeIncluded ? isNotNull : isNull,
expect(capturedRequest, isNotNull);
expect(capturedRequest?.data, scenario.matcher);
}
});

test('response body is included according to $MaxResponseBodySize',
() async {
final scenarios = [
// never
MaxBodySizeTestConfig(MaxResponseBodySize.never, 0, false),
MaxBodySizeTestConfig(MaxResponseBodySize.never, 4001, false),
MaxBodySizeTestConfig(MaxResponseBodySize.never, 10001, false),
// always
MaxBodySizeTestConfig(MaxResponseBodySize.always, 0, true),
MaxBodySizeTestConfig(MaxResponseBodySize.always, 4001, true),
MaxBodySizeTestConfig(MaxResponseBodySize.always, 10001, true),
// small
MaxBodySizeTestConfig(MaxResponseBodySize.small, 0, true),
MaxBodySizeTestConfig(MaxResponseBodySize.small, 4000, true),
MaxBodySizeTestConfig(MaxResponseBodySize.small, 4001, false),
// medium
MaxBodySizeTestConfig(MaxResponseBodySize.medium, 0, true),
MaxBodySizeTestConfig(MaxResponseBodySize.medium, 4001, true),
MaxBodySizeTestConfig(MaxResponseBodySize.medium, 10000, true),
MaxBodySizeTestConfig(MaxResponseBodySize.medium, 10001, false),
];

const errCode = 500;
const errReason = 'Internal Server Error';

for (final scenario in scenarios) {
fixture.transport.reset();

final sut = fixture.getSut(
client: MockClient((request) => Future.value(Response(
' ' * scenario.contentLength, errCode,
reasonPhrase: errReason))),
badStatusCodes: [SentryStatusCode(errCode)],
captureFailedRequests: true,
maxResponseBodySize: scenario.maxBodySize,
);

final response = await sut.send(Request('GET', requestUri));
expect(response.statusCode, errCode);
expect(response.reasonPhrase, errReason);

expect(fixture.transport.calls, 1);

final eventCall = fixture.transport.events.first;
final capturedResponse = eventCall.contexts.response;
expect(capturedResponse, isNotNull);
expect(capturedResponse?.body, scenario.matcher);
}
});
});
Expand Down Expand Up @@ -253,6 +302,7 @@ class Fixture {
MockClient? client,
bool captureFailedRequests = false,
MaxRequestBodySize maxRequestBodySize = MaxRequestBodySize.small,
MaxResponseBodySize maxResponseBodySize = MaxResponseBodySize.small,
List<SentryStatusCode> badStatusCodes = const [],
bool sendDefaultPii = true,
}) {
Expand All @@ -263,6 +313,7 @@ class Fixture {
captureFailedRequests: captureFailedRequests,
failedRequestStatusCodes: badStatusCodes,
maxRequestBodySize: maxRequestBodySize,
maxResponseBodySize: maxResponseBodySize,
sendDefaultPii: sendDefaultPii,
);
}
Expand All @@ -277,14 +328,16 @@ class Fixture {

class TestException implements Exception {}

class MaxRequestBodySizeTestConfig {
MaxRequestBodySizeTestConfig(
this.maxRequestBodySize,
class MaxBodySizeTestConfig<T> {
MaxBodySizeTestConfig(
this.maxBodySize,
this.contentLength,
this.shouldBeIncluded,
);

final MaxRequestBodySize maxRequestBodySize;
final T maxBodySize;
final int contentLength;
final bool shouldBeIncluded;

Matcher get matcher => shouldBeIncluded ? isNotNull : isNull;
}
6 changes: 2 additions & 4 deletions dio/lib/src/dio_event_processor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,9 @@ class DioEventProcessor implements EventProcessor {
return event;
}

final response = _responseFrom(dioError);

Contexts contexts = event.contexts;
if (event.contexts.response == null) {
contexts = contexts.copyWith(response: response);
contexts = contexts.copyWith(response: _responseFrom(dioError));
}
// Don't override just parts of the original request.
// Keep the original one or if there's none create one.
Expand Down Expand Up @@ -155,7 +153,7 @@ class DioEventProcessor implements EventProcessor {
return null;
}

SentryResponse? _responseFrom(DioError dioError) {
SentryResponse _responseFrom(DioError dioError) {
final response = dioError.response;

final headers = response?.headers.map.map(
Expand Down