Skip to content

Commit

Permalink
Add override captureFailedRequests option (#1931)
Browse files Browse the repository at this point in the history
  • Loading branch information
denrase authored Mar 12, 2024
1 parent fb06db4 commit e8603bb
Show file tree
Hide file tree
Showing 9 changed files with 195 additions and 19 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,12 @@

## Unreleased

## Features

- Add override `captureFailedRequests` option ([#1931](https://github.com/getsentry/sentry-dart/pull/1931))
- The `dio` integration and `SentryHttpClient` now take an additional `captureFailedRequests` option.
- This is useful if you want to disable this option on native and only enable it on `dio` for example.

### Dependencies

- Bump Android SDK from v7.5.0 to v7.6.0 ([#1927](https://github.com/getsentry/sentry-dart/pull/1927))
Expand Down
9 changes: 6 additions & 3 deletions dart/lib/src/http_client/failed_request_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,15 +73,18 @@ class FailedRequestClient extends BaseClient {
this.failedRequestTargets = SentryHttpClient.defaultFailedRequestTargets,
Client? client,
Hub? hub,
bool? captureFailedRequests,
}) : _hub = hub ?? HubAdapter(),
_client = client ?? Client() {
if (_hub.options.captureFailedRequests) {
_client = client ?? Client(),
_captureFailedRequests = captureFailedRequests {
if (captureFailedRequests ?? _hub.options.captureFailedRequests) {
_hub.options.sdk.addIntegration('HTTPClientError');
}
}

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

/// Describes which HTTP status codes should be considered as a failed
/// requests.
Expand Down Expand Up @@ -129,7 +132,7 @@ class FailedRequestClient extends BaseClient {
StackTrace? stackTrace,
StreamedResponse? response,
Duration duration) async {
if (!_hub.options.captureFailedRequests) {
if (!(_captureFailedRequests ?? _hub.options.captureFailedRequests)) {
return;
}

Expand Down
5 changes: 5 additions & 0 deletions dart/lib/src/http_client/sentry_http_client.dart
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,9 @@ import 'failed_request_client.dart';
/// Remarks:
/// HTTP traffic can contain PII (personal identifiable information).
/// Read more on data scrubbing [here](https://docs.sentry.io/product/data-management-settings/advanced-datascrubbing/).
///
/// The constructor parameter `captureFailedRequests` will override what you
/// have configured in options.
/// ```
class SentryHttpClient extends BaseClient {
static const defaultFailedRequestStatusCodes = [
Expand All @@ -86,6 +89,7 @@ class SentryHttpClient extends BaseClient {
List<SentryStatusCode> failedRequestStatusCodes =
defaultFailedRequestStatusCodes,
List<String> failedRequestTargets = defaultFailedRequestTargets,
bool? captureFailedRequests,
}) {
_hub = hub ?? HubAdapter();

Expand All @@ -96,6 +100,7 @@ class SentryHttpClient extends BaseClient {
failedRequestTargets: failedRequestTargets,
hub: _hub,
client: innerClient,
captureFailedRequests: captureFailedRequests,
);

if (_hub.options.isTracingEnabled()) {
Expand Down
54 changes: 45 additions & 9 deletions dart/test/http_client/failed_request_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,12 @@ void main() {
expect(eventCall.contexts.response, isNull);
});

test('event not reported if disabled', () async {
test(
'exception does not gets reported if client throws but override disables capture',
() async {
fixture._hub.options.captureFailedRequests = true;
fixture._hub.options.sendDefaultPii = true;

final sut = fixture.getSut(
client: createThrowingClient(),
captureFailedRequests: false,
Expand All @@ -84,10 +89,42 @@ void main() {
expect(fixture.transport.calls, 0);
});

test('event not reported if disabled', () async {
fixture._hub.options.captureFailedRequests = false;

final sut = fixture.getSut(
client: createThrowingClient(),
);

await expectLater(
() async => await sut.get(requestUri, headers: {'Cookie': 'foo=bar'}),
throwsException,
);

expect(fixture.transport.calls, 0);
});

test('event reported if disabled but overridden', () async {
fixture._hub.options.captureFailedRequests = false;

final sut = fixture.getSut(
client: createThrowingClient(),
captureFailedRequests: true,
);

await expectLater(
() async => await sut.get(requestUri, headers: {'Cookie': 'foo=bar'}),
throwsException,
);

expect(fixture.transport.calls, 1);
});

test('event not reported if not within the targets', () async {
fixture._hub.options.captureFailedRequests = true;

final sut = fixture.getSut(
client: fixture.getClient(statusCode: 500),
captureFailedRequests: true,
failedRequestTargets: const ["myapi.com"]);

final response = await sut.get(requestUri);
Expand Down Expand Up @@ -335,17 +372,16 @@ class Fixture {
List<SentryStatusCode> failedRequestStatusCodes = const [
SentryStatusCode.defaultRange()
],
bool captureFailedRequests = true,
List<String> failedRequestTargets = const [".*"],
bool? captureFailedRequests,
}) {
final mc = client ?? getClient();
_hub.options.captureFailedRequests = captureFailedRequests;
return FailedRequestClient(
client: mc,
hub: _hub,
failedRequestStatusCodes: failedRequestStatusCodes,
failedRequestTargets: failedRequestTargets,
);
client: mc,
hub: _hub,
failedRequestStatusCodes: failedRequestStatusCodes,
failedRequestTargets: failedRequestTargets,
captureFailedRequests: captureFailedRequests);
}

MockClient getClient(
Expand Down
34 changes: 31 additions & 3 deletions dart/test/http_client/sentry_http_client_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,10 @@ void main() {
});

test('no captured event with default config', () async {
fixture.hub.options.captureFailedRequests = false;

final sut = fixture.getSut(
client: createThrowingClient(),
captureFailedRequests: false,
);

await expectLater(() async => await sut.get(requestUri), throwsException);
Expand All @@ -43,6 +44,19 @@ void main() {
expect(fixture.hub.addBreadcrumbCalls.length, 1);
});

test('captured event with override', () async {
fixture.hub.options.captureFailedRequests = false;

final sut = fixture.getSut(
client: createThrowingClient(),
captureFailedRequests: true,
);

await expectLater(() async => await sut.get(requestUri), throwsException);

expect(fixture.hub.captureEventCalls.length, 1);
});

test('one captured event with when enabling $FailedRequestClient',
() async {
fixture.hub.options.captureFailedRequests = true;
Expand All @@ -61,6 +75,20 @@ void main() {
expect(fixture.hub.addBreadcrumbCalls.length, 1);
});

test(
'no captured event with when enabling $FailedRequestClient with override',
() async {
fixture.hub.options.captureFailedRequests = true;
final sut = fixture.getSut(
client: createThrowingClient(),
captureFailedRequests: false,
);

await expectLater(() async => await sut.get(requestUri), throwsException);

expect(fixture.hub.captureEventCalls.length, 0);
});

test('close does get called for user defined client', () async {
final mockHub = MockHub();

Expand Down Expand Up @@ -116,14 +144,14 @@ class Fixture {
SentryHttpClient getSut({
MockClient? client,
List<SentryStatusCode> badStatusCodes = const [],
bool captureFailedRequests = true,
bool? captureFailedRequests,
}) {
final mc = client ?? getClient();
hub.options.captureFailedRequests = captureFailedRequests;
return SentryHttpClient(
client: mc,
hub: hub,
failedRequestStatusCodes: badStatusCodes,
captureFailedRequests: captureFailedRequests,
);
}

Expand Down
9 changes: 6 additions & 3 deletions dio/lib/src/failed_request_interceptor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,24 @@ class FailedRequestInterceptor extends Interceptor {
SentryHttpClient.defaultFailedRequestStatusCodes,
List<String> failedRequestTargets =
SentryHttpClient.defaultFailedRequestTargets,
bool? captureFailedRequests,
}) : _hub = hub ?? HubAdapter(),
_failedRequestStatusCodes = failedRequestStatusCodes,
_failedRequestTargets = failedRequestTargets;
_failedRequestTargets = failedRequestTargets,
_captureFailedRequests = captureFailedRequests;

final Hub _hub;
final List<SentryStatusCode> _failedRequestStatusCodes;
final List<String> _failedRequestTargets;
final bool? _captureFailedRequests;

@override
Future<void> onError(
DioError err,
ErrorInterceptorHandler handler,
) async {
// ignore: invalid_use_of_internal_member
final captureFailedRequests = _hub.options.captureFailedRequests;
final cfr = _captureFailedRequests ?? _hub.options.captureFailedRequests;

final containsStatusCode =
_failedRequestStatusCodes.containsStatusCode(err.response?.statusCode);
Expand All @@ -33,7 +36,7 @@ class FailedRequestInterceptor extends Interceptor {
err.requestOptions.path,
);

if (captureFailedRequests && containsStatusCode && containsRequestTarget) {
if (cfr && containsStatusCode && containsRequestTarget) {
final mechanism = Mechanism(type: 'SentryDioClientAdapter');
final throwableMechanism = ThrowableMechanism(mechanism, err);

Expand Down
5 changes: 4 additions & 1 deletion dio/lib/src/sentry_dio_extension.dart
Original file line number Diff line number Diff line change
Expand Up @@ -42,12 +42,15 @@ extension SentryDioExtension on Dio {
/// failedRequestTargets: ['my-api.com'],
/// );
/// ```
///
/// The captureFailedRequests argument will take precedent over options.
void addSentry({
Hub? hub,
List<SentryStatusCode> failedRequestStatusCodes =
SentryHttpClient.defaultFailedRequestStatusCodes,
List<String> failedRequestTargets =
SentryHttpClient.defaultFailedRequestTargets,
bool? captureFailedRequests,
}) {
hub = hub ?? HubAdapter();

Expand All @@ -71,7 +74,7 @@ extension SentryDioExtension on Dio {
}
options.sdk.addPackage(packageName, sdkVersion);

if (options.captureFailedRequests) {
if (captureFailedRequests ?? options.captureFailedRequests) {
// Add FailedRequestInterceptor at index 0, so it's the first interceptor.
// This ensures that it is called and not skipped by any previous interceptor.
interceptors.insert(
Expand Down
34 changes: 34 additions & 0 deletions dio/test/failed_request_interceptor_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,38 @@ void main() {
expect(fixture.hub.captureExceptionCalls.length, 0);
});

test('do capture if captureFailedRequests override is true', () async {
final requestOptions = RequestOptions(path: 'https://example.com');
final error = DioError(
requestOptions: requestOptions,
response: Response(statusCode: 500, requestOptions: requestOptions),
);

fixture.hub.options.captureFailedRequests = false;

final sut = fixture.getSut(captureFailedRequests: true);
await sut.onError(error, fixture.errorInterceptorHandler);

expect(fixture.errorInterceptorHandler.nextWasCalled, true);
expect(fixture.hub.captureExceptionCalls.length, 1);
});

test('do not capture if captureFailedRequests override false', () async {
final requestOptions = RequestOptions(path: 'https://example.com');
final error = DioError(
requestOptions: requestOptions,
response: Response(statusCode: 500, requestOptions: requestOptions),
);

fixture.hub.options.captureFailedRequests = true;

final sut = fixture.getSut(captureFailedRequests: false);
await sut.onError(error, fixture.errorInterceptorHandler);

expect(fixture.errorInterceptorHandler.nextWasCalled, true);
expect(fixture.hub.captureExceptionCalls.length, 0);
});

test('capture in range failedRequestStatusCodes', () async {
final requestOptions = RequestOptions(path: 'https://example.com');
final error = DioError(
Expand Down Expand Up @@ -116,11 +148,13 @@ class Fixture {
SentryStatusCode.defaultRange(),
],
List<String> failedRequestTargets = const ['.*'],
bool? captureFailedRequests,
}) {
return FailedRequestInterceptor(
hub: hub,
failedRequestStatusCodes: failedRequestStatusCodes,
failedRequestTargets: failedRequestTargets,
captureFailedRequests: captureFailedRequests,
);
}
}
Expand Down
Loading

0 comments on commit e8603bb

Please sign in to comment.