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

capture response body for failed requests and if tracing is enabled in SentryHttpClient #2293

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
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
142 changes: 14 additions & 128 deletions dart/lib/src/http_client/failed_request_client.dart
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
import 'package:http/http.dart';
import '../hint.dart';

Check warning on line 2 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

Unused import: '../hint.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unused_import to learn more about this problem.
import '../type_check_hint.dart';

Check warning on line 3 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

Unused import: '../type_check_hint.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unused_import to learn more about this problem.
import '../utils/http_deep_copy_streamed_response.dart';
import '../utils/tracing_utils.dart';

Check warning on line 5 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

Unused import: '../utils/tracing_utils.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unused_import to learn more about this problem.
import 'sentry_http_client_error.dart';

Check warning on line 6 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

Unused import: 'sentry_http_client_error.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unused_import to learn more about this problem.
import '../protocol.dart';

Check warning on line 7 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

Unused import: '../protocol.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unused_import to learn more about this problem.
import '../hub.dart';
import '../hub_adapter.dart';
import '../throwable_mechanism.dart';

Check warning on line 10 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

Unused import: '../throwable_mechanism.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unused_import to learn more about this problem.
import 'sentry_http_client.dart';

/// A [http](https://pub.dev/packages/http)-package compatible HTTP client
Expand Down Expand Up @@ -77,14 +78,14 @@
}) : _hub = hub ?? HubAdapter(),
_client = client ?? Client(),
_captureFailedRequests = captureFailedRequests {
if (captureFailedRequests ?? _hub.options.captureFailedRequests) {

Check warning on line 81 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

'captureFailedRequests' is deprecated and shouldn't be used. All Request and Responses are now logged, if `sendDefaultPii` is `true` and `maxRequestBodySize` and `maxResponseBodySize` conditions are met.

Try replacing the use of the deprecated member with the replacement. See https://dart.dev/diagnostics/deprecated_member_use_from_same_package to learn more about this problem.
_hub.options.sdk.addIntegration('HTTPClientError');
}
}

final Client _client;
final Hub _hub;
final bool? _captureFailedRequests;

Check warning on line 88 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The value of the field '_captureFailedRequests' isn't used.

Try removing the field, or using it. See https://dart.dev/diagnostics/unused_field to learn more about this problem.

/// Describes which HTTP status codes should be considered as a failed
/// requests.
Expand All @@ -100,156 +101,41 @@
Object? exception;
StackTrace? stackTrace;
StreamedResponse? response;
List<StreamedResponse> copiedResponses = [];

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

try {
response = await _client.send(request);
statusCode = response.statusCode;
return response;
copiedResponses = await deepCopyStreamedResponse(response, 2);
statusCode = copiedResponses[0].statusCode;
return copiedResponses[0];
} catch (e, st) {
exception = e;
stackTrace = st;
rethrow;
} finally {
stopwatch.stop();
await _captureEventIfNeeded(
request,
statusCode,
exception,
stackTrace,
response,
stopwatch.elapsed,
);
}
}

Future<void> _captureEventIfNeeded(
BaseRequest request,
int? statusCode,
Object? exception,
StackTrace? stackTrace,
StreamedResponse? response,
Duration duration) async {
if (!(_captureFailedRequests ?? _hub.options.captureFailedRequests)) {
return;
}

// Only check `failedRequestStatusCodes` & `failedRequestTargets` if no exception was thrown.
if (exception == null) {
if (!failedRequestStatusCodes._containsStatusCode(statusCode)) {
return;
}
if (!containsTargetOrMatchesRegExp(
failedRequestTargets, request.url.toString())) {
return;
}
await captureEvent(
_hub,
exception: exception,
stackTrace: stackTrace,
request: request,
requestDuration: stopwatch.elapsed,
response: copiedResponses.isNotEmpty ? copiedResponses[1] : null,
reason: 'HTTP Client Event with status code: $statusCode',
);
}

final reason = 'HTTP Client Error with status code: $statusCode';
exception ??= SentryHttpClientError(reason);

await _captureEvent(
exception: exception,
stackTrace: stackTrace,
request: request,
requestDuration: duration,
response: response,
reason: reason,
);
}

@override
void close() => _client.close();

// See https://develop.sentry.dev/sdk/event-payloads/request/
Future<void> _captureEvent({
required Object? exception,
StackTrace? stackTrace,
String? reason,
required Duration requestDuration,
required BaseRequest request,
required StreamedResponse? response,
}) async {
final sentryRequest = SentryRequest.fromUri(
method: request.method,
headers: _hub.options.sendDefaultPii ? request.headers : null,
uri: request.url,
data: _hub.options.sendDefaultPii ? _getDataFromRequest(request) : null,
// ignore: deprecated_member_use_from_same_package
other: {
'content_length': request.contentLength.toString(),
'duration': requestDuration.toString(),
},
);

final mechanism = Mechanism(
type: 'SentryHttpClient',
description: reason,
);

bool? snapshot;
if (exception is SentryHttpClientError) {
snapshot = true;
}

final throwableMechanism = ThrowableMechanism(
mechanism,
exception,
snapshot: snapshot,
);

final event = SentryEvent(
throwable: throwableMechanism,
request: sentryRequest,
timestamp: _hub.options.clock(),
);

final hint = Hint.withMap({TypeCheckHint.httpRequest: request});

if (response != null) {
event.contexts.response = SentryResponse(
headers: _hub.options.sendDefaultPii ? response.headers : null,
bodySize: response.contentLength,
statusCode: response.statusCode,
);
hint.set(TypeCheckHint.httpResponse, response);
}

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

// Types of Request can be found here:
// https://pub.dev/documentation/http/latest/http/http-library.html
Object? _getDataFromRequest(BaseRequest request) {
final contentLength = request.contentLength;
if (contentLength == null) {
return null;
}
if (!_hub.options.maxRequestBodySize.shouldAddBody(contentLength)) {
return null;
}
if (request is MultipartRequest) {
final data = <String, String>{...request.fields};
return data;
}

if (request is Request) {
return request.body;
}

// There's nothing we can do for a StreamedRequest
return null;
}
}

extension _ListX on List<SentryStatusCode> {
bool _containsStatusCode(int? statusCode) {

Check notice on line 138 in dart/lib/src/http_client/failed_request_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The declaration '_containsStatusCode' isn't referenced.

Try removing the declaration of '_containsStatusCode'. See https://dart.dev/diagnostics/unused_element to learn more about this problem.
if (statusCode == null) {
return false;
}
Expand Down
94 changes: 94 additions & 0 deletions dart/lib/src/http_client/sentry_http_client.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import 'package:http/http.dart';
import 'package:meta/meta.dart';
import '../../sentry.dart';
import 'tracing_client.dart';
import '../hub.dart';

Check notice on line 5 in dart/lib/src/http_client/sentry_http_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../hub.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.
import '../hub_adapter.dart';

Check notice on line 6 in dart/lib/src/http_client/sentry_http_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../hub_adapter.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.
import 'breadcrumb_client.dart';
import 'failed_request_client.dart';

Expand Down Expand Up @@ -160,3 +162,95 @@
return '$_min..$_max';
}
}

@internal
// See https://develop.sentry.dev/sdk/event-payloads/request/
Future<void> captureEvent(
Hub hub, {
Object? exception,
StackTrace? stackTrace,
String? reason,
required Duration requestDuration,
required BaseRequest request,
required StreamedResponse? response,
}) async {
final sentryRequest = SentryRequest.fromUri(
method: request.method,
headers: hub.options.sendDefaultPii ? request.headers : null,
uri: request.url,
data: hub.options.sendDefaultPii ? _getDataFromRequest(hub, request) : null,
// ignore: deprecated_member_use_from_same_package
other: {
'content_length': request.contentLength.toString(),
'duration': requestDuration.toString(),
},
);

final mechanism = Mechanism(
type: 'SentryHttpClient',
description: reason,
);

bool? snapshot;
ThrowableMechanism? throwableMechanism;
if (exception is SentryHttpClientError) {
snapshot = true;
throwableMechanism = ThrowableMechanism(
mechanism,
exception,
snapshot: snapshot,
);
}

final event = SentryEvent(
throwable: throwableMechanism,
request: sentryRequest,
timestamp: hub.options.clock(),
);

final hint = Hint.withMap({TypeCheckHint.httpRequest: request});

if (response != null) {
final responseBody = await response.stream.bytesToString();
event.contexts.response = SentryResponse(
headers: hub.options.sendDefaultPii ? response.headers : null,
bodySize: response.contentLength,
statusCode: response.statusCode,
data: hub.options.sendDefaultPii &&
hub.options.maxResponseBodySize
.shouldAddBody(response.contentLength!)
? responseBody
: null,
);
hint.set(TypeCheckHint.httpResponse, response);
}

await hub.captureEvent(
event,
stackTrace: stackTrace,
hint: hint,
);
}

// Types of Request can be found here:
// https://pub.dev/documentation/http/latest/http/http-library.html
Object? _getDataFromRequest(Hub hub, BaseRequest request) {
final contentLength = request.contentLength;
if (contentLength == null) {
return null;
}
if (!hub.options.maxRequestBodySize.shouldAddBody(contentLength)) {
return null;
}
if (request is MultipartRequest) {
final data = <String, String>{...request.fields};
return data;
}

if (request is Request) {
return request.body;
}

// There's nothing we can do for a StreamedRequest
return null;
}
32 changes: 28 additions & 4 deletions dart/lib/src/http_client/tracing_client.dart
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import 'package:http/http.dart';
import '../../sentry.dart';
import '../hub.dart';

Check notice on line 3 in dart/lib/src/http_client/tracing_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../hub.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.
import '../hub_adapter.dart';

Check notice on line 4 in dart/lib/src/http_client/tracing_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../hub_adapter.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.
import '../protocol.dart';

Check notice on line 5 in dart/lib/src/http_client/tracing_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../protocol.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.
import '../sentry_trace_origins.dart';

Check notice on line 6 in dart/lib/src/http_client/tracing_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../sentry_trace_origins.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.
import '../tracing.dart';

Check notice on line 7 in dart/lib/src/http_client/tracing_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../tracing.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.
import '../utils/http_deep_copy_streamed_response.dart';
import '../utils/tracing_utils.dart';

Check notice on line 9 in dart/lib/src/http_client/tracing_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../utils/tracing_utils.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.
import '../utils/http_sanitizer.dart';

Check notice on line 10 in dart/lib/src/http_client/tracing_client.dart

View workflow job for this annotation

GitHub Actions / analyze / analyze

The import of '../utils/http_sanitizer.dart' is unnecessary because all of the used elements are also provided by the import of '../../sentry.dart'.

Try removing the import directive. See https://dart.dev/diagnostics/unnecessary_import to learn more about this problem.

/// A [http](https://pub.dev/packages/http)-package compatible HTTP client
/// which adds support to Sentry Performance feature.
Expand All @@ -21,6 +23,9 @@
@override
Future<StreamedResponse> send(BaseRequest request) async {
// see https://develop.sentry.dev/sdk/performance/#header-sentry-trace
int? statusCode;
final stopwatch = Stopwatch();
stopwatch.start();

final urlDetails = HttpSanitizer.sanitizeUrl(request.url.toString());

Expand All @@ -45,6 +50,7 @@
urlDetails?.applyToSpan(span);

StreamedResponse? response;
List<StreamedResponse> copiedResponses = [];
try {
if (containsTargetOrMatchesRegExp(
_hub.options.tracePropagationTargets, request.url.toString())) {
Expand Down Expand Up @@ -72,18 +78,36 @@
}

response = await _client.send(request);
span?.setData('http.response.status_code', response.statusCode);
span?.setData('http.response_content_length', response.contentLength);
span?.status = SpanStatus.fromHttpStatusCode(response.statusCode);
copiedResponses = await deepCopyStreamedResponse(response, 2);
statusCode = copiedResponses[0].statusCode;
span?.setData('http.response.status_code', copiedResponses[1].statusCode);
span?.setData(
'http.response_content_length', copiedResponses[1].contentLength);
if (_hub.options.sendDefaultPii &&
_hub.options.maxResponseBodySize
.shouldAddBody(response.contentLength!)) {
final responseBody = await copiedResponses[1].stream.bytesToString();
span?.setData('http.response_content', responseBody);
}
span?.status =
SpanStatus.fromHttpStatusCode(copiedResponses[1].statusCode);
} catch (exception) {
span?.throwable = exception;
span?.status = SpanStatus.internalError();

rethrow;
} finally {
await span?.finish();
stopwatch.stop();
await captureEvent(
_hub,
request: request,
requestDuration: stopwatch.elapsed,
response: copiedResponses.isNotEmpty ? copiedResponses[1] : null,
reason: 'HTTP Client Event with status code: $statusCode',
);
}
return response;
return copiedResponses[0];
}

@override
Expand Down
2 changes: 2 additions & 0 deletions dart/lib/src/sentry_options.dart
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,8 @@ class SentryOptions {
/// because the connection was interrupted.
/// Use with [SentryHttpClient] or `sentry_dio` integration for this to work,
/// or iOS native where it sets the value to `enableCaptureFailedRequests`.
@Deprecated(
"All Request and Responses are now logged, if `sendDefaultPii` is `true` and `maxRequestBodySize` and `maxResponseBodySize` conditions are met.")
bool captureFailedRequests = true;
Comment on lines +282 to 283
Copy link
Contributor

Choose a reason for hiding this comment

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

let's rethink this, I don't think deprecating this flag is the right way to go


/// Whether to records requests as breadcrumbs. This is on by default.
Expand Down
28 changes: 28 additions & 0 deletions dart/lib/src/utils/http_deep_copy_streamed_response.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import 'package:http/http.dart';
import 'package:meta/meta.dart';

/// Helper to deep copy the StreamedResponse of a web request
@internal
Future<List<StreamedResponse>> deepCopyStreamedResponse(
StreamedResponse originalResponse, int copies) async {
final List<int> bufferedData = [];

await for (final List<int> chunk in originalResponse.stream) {
bufferedData.addAll(chunk);
}

List<StreamedResponse> copiedElements = [];
for (int i = 1; i <= copies; i++) {
copiedElements.add(StreamedResponse(
Stream.fromIterable([bufferedData]),
originalResponse.statusCode,
contentLength: originalResponse.contentLength,
request: originalResponse.request,
headers: originalResponse.headers,
reasonPhrase: originalResponse.reasonPhrase,
isRedirect: originalResponse.isRedirect,
persistentConnection: originalResponse.persistentConnection,
));
}
return copiedElements;
}
Loading
Loading