Skip to content

Commit

Permalink
feat: capture Response context in HTTP Client (#1095)
Browse files Browse the repository at this point in the history
  • Loading branch information
vaind authored Nov 4, 2022
1 parent 0ceb89c commit 5603ab2
Show file tree
Hide file tree
Showing 14 changed files with 230 additions and 266 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,15 @@

## Unreleased

### Features

- Capture response information in `SentryHttpClient` ([#1095](https://github.com/getsentry/sentry-dart/pull/1095))

### Changes

- Remove experimental `SentryResponse` fields: `url`, `body`, `redirected`, `status` ([#1095](https://github.com/getsentry/sentry-dart/pull/1095))
- `SentryHttpClient` request body capture checks default PII capture setting, same as the DIO integration ([#1095](https://github.com/getsentry/sentry-dart/pull/1095))

### Dependencies

- Bump Android SDK from v6.5.0 to v6.6.0 ([#1090](https://github.com/getsentry/sentry-dart/pull/1090))
Expand Down
46 changes: 28 additions & 18 deletions dart/lib/src/http_client/failed_request_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -77,15 +77,15 @@ 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;

/// Describes which HTTP status codes should be considered as a failed
Expand All @@ -101,12 +101,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 +119,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 +153,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 @@ -164,8 +166,7 @@ class FailedRequestClient extends BaseClient {
headers: sendDefaultPii ? request.headers : null,
url: urlWithoutQuery,
queryString: query,
cookies: sendDefaultPii ? request.headers['Cookie'] : null,
data: _getDataFromRequest(request),
data: sendDefaultPii ? _getDataFromRequest(request) : null,
// ignore: deprecated_member_use_from_same_package
other: {
'content_length': request.contentLength.toString(),
Expand All @@ -183,6 +184,15 @@ class FailedRequestClient extends BaseClient {
throwable: throwableMechanism,
request: sentryRequest,
);

if (response != null) {
event.contexts.response = SentryResponse(
headers: sendDefaultPii ? response.headers : null,
bodySize: response.contentLength,
statusCode: response.statusCode,
);
}

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

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 a 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 a new enum value is added.
}
return false;
}
Expand Down
15 changes: 11 additions & 4 deletions dart/lib/src/protocol/sentry_request.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import 'package:meta/meta.dart';

import '../utils/iterable_extension.dart';

/// The Request interface contains information on a HTTP request related to the event.
/// In client SDKs, this can be an outgoing request, or the request that rendered the current web page.
/// On server SDKs, this could be the incoming web request that is being handled.
Expand Down Expand Up @@ -63,14 +65,19 @@ class SentryRequest {
this.url,
this.method,
this.queryString,
this.cookies,
String? cookies,
this.fragment,
dynamic data,
Map<String, String>? headers,
Map<String, String>? env,
@Deprecated('Will be removed in v7.') Map<String, String>? other,
}) : _data = data,
_headers = headers != null ? Map.from(headers) : null,
// Look for a 'Set-Cookie' header (case insensitive) if not given.
cookies = cookies ??
headers?.entries
.firstWhereOrNull((e) => e.key.toLowerCase() == 'cookie')
?.value,
_env = env != null ? Map.from(env) : null,
_other = other != null ? Map.from(other) : null;

Expand All @@ -82,10 +89,10 @@ class SentryRequest {
queryString: json['query_string'],
cookies: json['cookies'],
data: json['data'],
headers: json['headers'],
env: json['env'],
headers: json.containsKey('headers') ? Map.from(json['headers']) : null,
env: json.containsKey('env') ? Map.from(json['env']) : null,
// ignore: deprecated_member_use_from_same_package
other: json['other'],
other: json.containsKey('other') ? Map.from(json['other']) : null,
fragment: json['fragment'],
);
}
Expand Down
82 changes: 28 additions & 54 deletions dart/lib/src/protocol/sentry_response.dart
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import 'package:meta/meta.dart';
import 'contexts.dart';
import '../utils/iterable_extension.dart';

/// The response interface contains information on a HTTP request related to the event.
/// This is an experimental feature. It might be removed at any time.
Expand All @@ -9,24 +10,13 @@ class SentryResponse {
/// The tpye of this class in the [Contexts] field
static const String type = 'response';

/// The URL of the response if available.
/// This might be the redirected URL
final String? url;

/// Indicates whether or not the response is the result of a redirect
/// (that is, its URL list has more than one entry).
final bool? redirected;

/// The body of the response
final Object? body;
/// The size of the response body.
final int? bodySize;

/// The HTTP status code of the response.
/// See https://developer.mozilla.org/en-US/docs/Web/HTTP/Status
final int? statusCode;

/// The status message for the corresponding [statusCode]
final String? status;

/// An immutable dictionary of submitted headers.
/// If a header appears multiple times it,
/// needs to be merged according to the HTTP standard for header merging.
Expand All @@ -35,73 +25,57 @@ class SentryResponse {

final Map<String, String>? _headers;

Map<String, String> get other => Map.unmodifiable(_other ?? const {});
/// Cookie key-value pairs as string.
final String? cookies;

final Map<String, String>? _other;

SentryResponse({
this.url,
this.body,
this.redirected,
this.statusCode,
this.status,
Map<String, String>? headers,
Map<String, String>? other,
}) : _headers = headers != null ? Map.from(headers) : null,
_other = other != null ? Map.from(other) : null;
SentryResponse(
{this.bodySize,
this.statusCode,
Map<String, String>? headers,
String? cookies})
: _headers = headers != null ? Map.from(headers) : null,
// Look for a 'Set-Cookie' header (case insensitive) if not given.
cookies = cookies ??
headers?.entries
.firstWhereOrNull((e) => e.key.toLowerCase() == 'set-cookie')
?.value;

/// Deserializes a [SentryResponse] from JSON [Map].
factory SentryResponse.fromJson(Map<String, dynamic> json) {
return SentryResponse(
url: json['url'],
headers: json['headers'],
other: json['other'],
body: json['body'],
statusCode: json['status_code'],
status: json['status'],
redirected: json['redirected'],
);
headers: json.containsKey('headers') ? Map.from(json['headers']) : null,
cookies: json['cookies'],
bodySize: json['body_size'],
statusCode: json['status_code']);
}

/// Produces a [Map] that can be serialized to JSON.
Map<String, dynamic> toJson() {
return <String, dynamic>{
if (url != null) 'url': url,
if (headers.isNotEmpty) 'headers': headers,
if (other.isNotEmpty) 'other': other,
if (redirected != null) 'redirected': redirected,
if (body != null) 'body': body,
if (status != null) 'status': status,
if (cookies != null) 'cookies': cookies,
if (bodySize != null) 'body_size': bodySize,
if (statusCode != null) 'status_code': statusCode,
};
}

SentryResponse copyWith({
String? url,
bool? redirected,
int? statusCode,
String? status,
Object? body,
int? bodySize,
Map<String, String>? headers,
Map<String, String>? other,
String? cookies,
}) =>
SentryResponse(
url: url ?? this.url,
headers: headers ?? _headers,
redirected: redirected ?? this.redirected,
other: other ?? _other,
body: body ?? this.body,
status: status ?? this.status,
cookies: cookies ?? this.cookies,
bodySize: bodySize ?? this.bodySize,
statusCode: statusCode ?? this.statusCode,
);

SentryResponse clone() => SentryResponse(
body: body,
bodySize: bodySize,
headers: headers,
other: other,
redirected: redirected,
status: status,
cookies: cookies,
statusCode: statusCode,
url: url,
);
}
8 changes: 8 additions & 0 deletions dart/lib/src/utils/iterable_extension.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
extension IterableExtension<T> on Iterable<T> {
T? firstWhereOrNull(bool Function(T item) predicate) {
for (var item in this) {
if (predicate(item)) return item;
}
return null;
}
}
Loading

0 comments on commit 5603ab2

Please sign in to comment.