From 642c4350d02062dc633a73c79a00b622b38d16e5 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 17 Jul 2023 16:28:15 +0200 Subject: [PATCH 01/13] add response body in failed request client --- .../http_client/failed_request_client.dart | 17 ++++++ .../failed_request_client_test.dart | 52 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/dart/lib/src/http_client/failed_request_client.dart b/dart/lib/src/http_client/failed_request_client.dart index 98df59194f..6f7baa0e29 100644 --- a/dart/lib/src/http_client/failed_request_client.dart +++ b/dart/lib/src/http_client/failed_request_client.dart @@ -104,6 +104,7 @@ class FailedRequestClient extends BaseClient { try { response = await _client.send(request); statusCode = response.statusCode; + return response; } catch (e, st) { exception = e; @@ -210,6 +211,9 @@ class FailedRequestClient extends BaseClient { headers: _hub.options.sendDefaultPii ? response.headers : null, bodySize: response.contentLength, statusCode: response.statusCode, + data: _hub.options.sendDefaultPii + ? await _getDataFromStreamedResponse(response) + : null, ); hint.set(TypeCheckHint.httpResponse, response); } @@ -221,6 +225,19 @@ class FailedRequestClient extends BaseClient { ); } + Future _getDataFromStreamedResponse( + StreamedResponse streamedResponse) async { + final contentLength = streamedResponse.contentLength; + if (contentLength == null) { + return null; + } + if (!_hub.options.maxResponseBodySize.shouldAddBody(contentLength)) { + return null; + } + var response = await Response.fromStream(streamedResponse); + return response.body; + } + // Types of Request can be found here: // https://pub.dev/documentation/http/latest/http/http-library.html Object? _getDataFromRequest(BaseRequest request) { diff --git a/dart/test/http_client/failed_request_client_test.dart b/dart/test/http_client/failed_request_client_test.dart index 4e6cac15b1..db78406ce3 100644 --- a/dart/test/http_client/failed_request_client_test.dart +++ b/dart/test/http_client/failed_request_client_test.dart @@ -1,3 +1,5 @@ +import 'dart:convert'; + import 'package:http/http.dart'; import 'package:http/testing.dart'; import 'package:mockito/mockito.dart'; @@ -281,6 +283,56 @@ void main() { } }); + 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), + ]; + + fixture._hub.options.captureFailedRequests = true; + fixture._hub.options.sendDefaultPii = true; + + for (final scenario in scenarios) { + fixture._hub.options.maxResponseBodySize = scenario.maxBodySize; + fixture.transport.reset(); + + final bodyBytes = List.generate(scenario.contentLength, (index) => 0); + final bodyString = utf8.decode(bodyBytes); + + final sut = fixture.getSut( + client: fixture.getClient(statusCode: 401, body: bodyString), + failedRequestStatusCodes: [SentryStatusCode(401)], + ); + + final request = Request('GET', requestUri); + await sut.send(request); + + expect(fixture.transport.calls, 1); + + final eventCall = fixture.transport.events.first; + final capturedResponse = eventCall.contexts.response; + expect(capturedResponse, isNotNull); + expect(capturedResponse?.data, + scenario.shouldBeIncluded ? isNotNull : isNull); + } + }); + test('request passed to hint', () async { fixture._hub.options.captureFailedRequests = true; From aa95370b7e7e2d174cff065adefc7029f38fb25b Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 18 Jul 2023 11:22:38 +0200 Subject: [PATCH 02/13] attach response data in dio event processor --- dio/lib/src/dio_event_processor.dart | 18 ++++ dio/test/dio_event_processor_test.dart | 141 ++++++++++++++++++++++++- 2 files changed, 156 insertions(+), 3 deletions(-) diff --git a/dio/lib/src/dio_event_processor.dart b/dio/lib/src/dio_event_processor.dart index 0e9eedd5eb..421f2ca5a4 100644 --- a/dio/lib/src/dio_event_processor.dart +++ b/dio/lib/src/dio_event_processor.dart @@ -89,6 +89,24 @@ class DioEventProcessor implements EventProcessor { headers: _options.sendDefaultPii ? headers : null, bodySize: dioError.response?.data?.length as int?, statusCode: response?.statusCode, + data: _getResponseData(dioError.response?.data), ); } + + /// Returns the response data, if possible according to the users settings. + Object? _getResponseData(dynamic data) { + if (!_options.sendDefaultPii) { + return null; + } + if (data is String) { + if (_options.maxResponseBodySize.shouldAddBody(data.codeUnits.length)) { + return data; + } + } else if (data is List) { + if (_options.maxResponseBodySize.shouldAddBody(data.length)) { + return data; + } + } + return null; + } } diff --git a/dio/test/dio_event_processor_test.dart b/dio/test/dio_event_processor_test.dart index 62520b6f44..d5431844fa 100644 --- a/dio/test/dio_event_processor_test.dart +++ b/dio/test/dio_event_processor_test.dart @@ -248,6 +248,122 @@ void main() { expect(processedEvent.exceptions?[1].value, exception.toString()); expect(processedEvent.exceptions?[1].stackTrace, isNotNull); }); + + test('request body is included according to $MaxResponseBodySize', () async { + final scenarios = [ + // never + MaxBodySizeTestConfig(MaxRequestBodySize.never, 0, false), + MaxBodySizeTestConfig(MaxRequestBodySize.never, 4001, false), + MaxBodySizeTestConfig(MaxRequestBodySize.never, 10001, false), + // always + MaxBodySizeTestConfig(MaxRequestBodySize.always, 0, true), + MaxBodySizeTestConfig(MaxRequestBodySize.always, 4001, true), + MaxBodySizeTestConfig(MaxRequestBodySize.always, 10001, true), + // small + MaxBodySizeTestConfig(MaxRequestBodySize.small, 0, true), + MaxBodySizeTestConfig(MaxRequestBodySize.small, 4000, true), + MaxBodySizeTestConfig(MaxRequestBodySize.small, 4001, false), + // medium + 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) { + final sut = fixture.getSut( + sendDefaultPii: true, + captureFailedRequests: true, + maxRequestBodySize: scenario.maxBodySize, + ); + + final data = List.generate(scenario.contentLength, (index) => 0); + final request = requestOptions.copyWith(method: 'POST', data: data); + final throwable = Exception(); + final dioError = DioError( + requestOptions: request, + response: Response( + requestOptions: request, + statusCode: 401, + data: data, + ), + ); + final event = SentryEvent( + throwable: throwable, + exceptions: [ + fixture.sentryError(throwable), + fixture.sentryError(dioError) + ], + ); + final processedEvent = sut.apply(event) as SentryEvent; + final capturedRequest = processedEvent.request; + + expect(capturedRequest, isNotNull); + expect( + capturedRequest?.data, + scenario.shouldBeIncluded ? isNotNull : isNull, + ); + } + }); + + 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), + ]; + + fixture.options.captureFailedRequests = true; + + for (final scenario in scenarios) { + final sut = fixture.getSut( + sendDefaultPii: true, + captureFailedRequests: true, + maxResponseBodySize: scenario.maxBodySize, + ); + + final data = List.generate(scenario.contentLength, (index) => 0); + final request = requestOptions.copyWith(method: 'POST', data: data); + final throwable = Exception(); + final dioError = DioError( + requestOptions: request, + response: Response( + requestOptions: request, + statusCode: 401, + data: data, + ), + ); + final event = SentryEvent( + throwable: throwable, + exceptions: [ + fixture.sentryError(throwable), + fixture.sentryError(dioError) + ], + ); + final processedEvent = sut.apply(event) as SentryEvent; + final capturedResponse = processedEvent.contexts.response; + + expect(capturedResponse, isNotNull); + expect( + capturedResponse?.data, + scenario.shouldBeIncluded ? isNotNull : isNull, + ); + } + }); } final requestOptions = RequestOptions( @@ -266,12 +382,17 @@ class Fixture { // ignore: invalid_use_of_internal_member SentryExceptionFactory get exceptionFactory => options.exceptionFactory; - DioEventProcessor getSut({bool sendDefaultPii = false}) { + DioEventProcessor getSut({ + bool sendDefaultPii = false, + bool captureFailedRequests = true, + MaxRequestBodySize maxRequestBodySize = MaxRequestBodySize.always, + MaxResponseBodySize maxResponseBodySize = MaxResponseBodySize.always, + }) { return DioEventProcessor( options ..sendDefaultPii = sendDefaultPii - ..maxRequestBodySize = MaxRequestBodySize.always - ..maxResponseBodySize = MaxResponseBodySize.always, + ..maxRequestBodySize = maxRequestBodySize + ..maxResponseBodySize = maxResponseBodySize, ); } @@ -283,3 +404,17 @@ class Fixture { ); } } + +class MaxBodySizeTestConfig { + MaxBodySizeTestConfig( + this.maxBodySize, + this.contentLength, + this.shouldBeIncluded, + ); + + final T maxBodySize; + final int contentLength; + final bool shouldBeIncluded; + + Matcher get matcher => shouldBeIncluded ? isNotNull : isNull; +} From 881a8d55a9055b055778285886ba581981e4c092 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 18 Jul 2023 11:56:06 +0200 Subject: [PATCH 03/13] add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f049c3ddd4..a1b0a3b9e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,8 @@ [Trace origin](https://develop.sentry.dev/sdk/performance/trace-origin/) indicates what created a trace or a span. Not all transactions and spans contain enough information to tell whether the user or what precisely in the SDK created it. Origin solves this problem. The SDK now sends origin for transactions and spans. +- Append http response body ([#1557](https://github.com/getsentry/sentry-dart/pull/1557)) + ## 7.8.0 ### Enhancements From d72d286aab29e847f8663da8a66c409b0c8481d0 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 18 Jul 2023 15:04:17 +0200 Subject: [PATCH 04/13] use Object ? instead of dynamic --- dio/lib/src/dio_event_processor.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/dio/lib/src/dio_event_processor.dart b/dio/lib/src/dio_event_processor.dart index 421f2ca5a4..23acacd025 100644 --- a/dio/lib/src/dio_event_processor.dart +++ b/dio/lib/src/dio_event_processor.dart @@ -62,7 +62,7 @@ class DioEventProcessor implements EventProcessor { } /// Returns the request data, if possible according to the users settings. - Object? _getRequestData(dynamic data) { + Object? _getRequestData(Object? data) { if (!_options.sendDefaultPii) { return null; } @@ -94,7 +94,7 @@ class DioEventProcessor implements EventProcessor { } /// Returns the response data, if possible according to the users settings. - Object? _getResponseData(dynamic data) { + Object? _getResponseData(Object? data) { if (!_options.sendDefaultPii) { return null; } From f9bdb8f9a5839f2776f36c5b7cb823d90cdae18e Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 18 Jul 2023 15:47:54 +0200 Subject: [PATCH 05/13] support all response body types --- dio/lib/src/dio_event_processor.dart | 74 ++++++- dio/test/dio_event_processor_test.dart | 286 +++++++++++++++---------- 2 files changed, 233 insertions(+), 127 deletions(-) diff --git a/dio/lib/src/dio_event_processor.dart b/dio/lib/src/dio_event_processor.dart index 23acacd025..b6a17b8e0a 100644 --- a/dio/lib/src/dio_event_processor.dart +++ b/dio/lib/src/dio_event_processor.dart @@ -1,5 +1,7 @@ // ignore_for_file: deprecated_member_use +import 'dart:convert'; + import 'package:dio/dio.dart'; import 'package:sentry/sentry.dart'; @@ -87,26 +89,76 @@ class DioEventProcessor implements EventProcessor { return SentryResponse( headers: _options.sendDefaultPii ? headers : null, - bodySize: dioError.response?.data?.length as int?, + bodySize: _getBodySize( + dioError.response?.data, + dioError.requestOptions.responseType, + ), statusCode: response?.statusCode, - data: _getResponseData(dioError.response?.data), + data: _getResponseData( + dioError.response?.data, + dioError.requestOptions.responseType, + ), ); } /// Returns the response data, if possible according to the users settings. - Object? _getResponseData(Object? data) { + Object? _getResponseData(Object? data, ResponseType responseType) { if (!_options.sendDefaultPii) { return null; } - if (data is String) { - if (_options.maxResponseBodySize.shouldAddBody(data.codeUnits.length)) { - return data; - } - } else if (data is List) { - if (_options.maxResponseBodySize.shouldAddBody(data.length)) { - return data; - } + if (data == null) { + return null; + } + switch (responseType) { + case ResponseType.json: + final js = json.encode(data); + if (_options.maxResponseBodySize.shouldAddBody(js.codeUnits.length)) { + return data; + } + break; + case ResponseType.stream: + break; // No support for logging stream body. + case ResponseType.plain: + if (data is String && + _options.maxResponseBodySize.shouldAddBody(data.codeUnits.length)) { + return data; + } + break; + case ResponseType.bytes: + if (data is List && + _options.maxResponseBodySize.shouldAddBody(data.length)) { + return data; + } + break; } return null; } + + int? _getBodySize(Object? data, ResponseType responseType) { + if (data == null) { + return null; + } + switch (responseType) { + case ResponseType.json: + return json.encode(data).codeUnits.length; + case ResponseType.stream: + if (data is String) { + return data.length; + } else { + return null; + } + case ResponseType.plain: + if (data is String) { + return data.codeUnits.length; + } else { + return null; + } + case ResponseType.bytes: + if (data is List) { + return data.length; + } else { + return null; + } + } + } } diff --git a/dio/test/dio_event_processor_test.dart b/dio/test/dio_event_processor_test.dart index d5431844fa..d2a5531e80 100644 --- a/dio/test/dio_event_processor_test.dart +++ b/dio/test/dio_event_processor_test.dart @@ -132,6 +132,64 @@ void main() { expect(processedEvent.request?.headers, {}); }); + + test('request body is included according to $MaxResponseBodySize', + () async { + final scenarios = [ + // never + MaxBodySizeTestConfig(MaxRequestBodySize.never, 0, false), + MaxBodySizeTestConfig(MaxRequestBodySize.never, 4001, false), + MaxBodySizeTestConfig(MaxRequestBodySize.never, 10001, false), + // always + MaxBodySizeTestConfig(MaxRequestBodySize.always, 0, true), + MaxBodySizeTestConfig(MaxRequestBodySize.always, 4001, true), + MaxBodySizeTestConfig(MaxRequestBodySize.always, 10001, true), + // small + MaxBodySizeTestConfig(MaxRequestBodySize.small, 0, true), + MaxBodySizeTestConfig(MaxRequestBodySize.small, 4000, true), + MaxBodySizeTestConfig(MaxRequestBodySize.small, 4001, false), + // medium + 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) { + final sut = fixture.getSut( + sendDefaultPii: true, + captureFailedRequests: true, + maxRequestBodySize: scenario.maxBodySize, + ); + + final data = List.generate(scenario.contentLength, (index) => 0); + final request = requestOptions.copyWith(method: 'POST', data: data); + final throwable = Exception(); + final dioError = DioError( + requestOptions: request, + response: Response( + requestOptions: request, + statusCode: 401, + data: data, + ), + ); + final event = SentryEvent( + throwable: throwable, + exceptions: [ + fixture.sentryError(throwable), + fixture.sentryError(dioError) + ], + ); + final processedEvent = sut.apply(event) as SentryEvent; + final capturedRequest = processedEvent.request; + + expect(capturedRequest, isNotNull); + expect( + capturedRequest?.data, + scenario.shouldBeIncluded ? isNotNull : isNull, + ); + } + }); }); group('response', () { @@ -211,6 +269,117 @@ void main() { expect(processedEvent.contexts.response?.statusCode, 200); expect(processedEvent.contexts.response?.headers, {}); }); + + 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), + ]; + + for (final scenario in scenarios) { + final sut = fixture.getSut( + sendDefaultPii: true, + captureFailedRequests: true, + maxResponseBodySize: scenario.maxBodySize, + ); + + final data = List.generate(scenario.contentLength, (index) => 0); + final request = requestOptions.copyWith(method: 'POST', data: data); + final throwable = Exception(); + final dioError = DioError( + requestOptions: request, + response: Response( + requestOptions: request, + statusCode: 401, + data: data, + ), + ); + final event = SentryEvent( + throwable: throwable, + exceptions: [ + fixture.sentryError(throwable), + fixture.sentryError(dioError) + ], + ); + final processedEvent = sut.apply(event) as SentryEvent; + final capturedResponse = processedEvent.contexts.response; + + expect(capturedResponse, isNotNull); + expect( + capturedResponse?.data, + scenario.shouldBeIncluded ? isNotNull : isNull, + ); + } + }); + + test('data supports all response body types', () async { + final dataByType = { + ResponseType.plain: ['plain'], + ResponseType.bytes: [ + [1337] + ], + ResponseType.json: [ + 9001, + null, + 'string', + true, + ['list'], + {'map-key': 'map-value'}, + ] + }; + + for (final entry in dataByType.entries) { + final responseType = entry.key; + + for (final data in entry.value) { + final request = requestOptions.copyWith( + method: 'POST', + data: data, + responseType: responseType, + ); + final throwable = Exception(); + final dioError = DioError( + requestOptions: request, + response: Response( + requestOptions: request, + statusCode: 401, + data: data, + ), + ); + + final sut = fixture.getSut(sendDefaultPii: true); + + final event = SentryEvent( + throwable: throwable, + exceptions: [ + fixture.sentryError(throwable), + fixture.sentryError(dioError) + ], + ); + final processedEvent = sut.apply(event) as SentryEvent; + final capturedResponse = processedEvent.contexts.response; + + expect(capturedResponse, isNotNull); + expect(capturedResponse?.data, data); + } + } + }); }); test('$DioEventProcessor adds chained stacktraces', () { @@ -248,122 +417,6 @@ void main() { expect(processedEvent.exceptions?[1].value, exception.toString()); expect(processedEvent.exceptions?[1].stackTrace, isNotNull); }); - - test('request body is included according to $MaxResponseBodySize', () async { - final scenarios = [ - // never - MaxBodySizeTestConfig(MaxRequestBodySize.never, 0, false), - MaxBodySizeTestConfig(MaxRequestBodySize.never, 4001, false), - MaxBodySizeTestConfig(MaxRequestBodySize.never, 10001, false), - // always - MaxBodySizeTestConfig(MaxRequestBodySize.always, 0, true), - MaxBodySizeTestConfig(MaxRequestBodySize.always, 4001, true), - MaxBodySizeTestConfig(MaxRequestBodySize.always, 10001, true), - // small - MaxBodySizeTestConfig(MaxRequestBodySize.small, 0, true), - MaxBodySizeTestConfig(MaxRequestBodySize.small, 4000, true), - MaxBodySizeTestConfig(MaxRequestBodySize.small, 4001, false), - // medium - 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) { - final sut = fixture.getSut( - sendDefaultPii: true, - captureFailedRequests: true, - maxRequestBodySize: scenario.maxBodySize, - ); - - final data = List.generate(scenario.contentLength, (index) => 0); - final request = requestOptions.copyWith(method: 'POST', data: data); - final throwable = Exception(); - final dioError = DioError( - requestOptions: request, - response: Response( - requestOptions: request, - statusCode: 401, - data: data, - ), - ); - final event = SentryEvent( - throwable: throwable, - exceptions: [ - fixture.sentryError(throwable), - fixture.sentryError(dioError) - ], - ); - final processedEvent = sut.apply(event) as SentryEvent; - final capturedRequest = processedEvent.request; - - expect(capturedRequest, isNotNull); - expect( - capturedRequest?.data, - scenario.shouldBeIncluded ? isNotNull : isNull, - ); - } - }); - - 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), - ]; - - fixture.options.captureFailedRequests = true; - - for (final scenario in scenarios) { - final sut = fixture.getSut( - sendDefaultPii: true, - captureFailedRequests: true, - maxResponseBodySize: scenario.maxBodySize, - ); - - final data = List.generate(scenario.contentLength, (index) => 0); - final request = requestOptions.copyWith(method: 'POST', data: data); - final throwable = Exception(); - final dioError = DioError( - requestOptions: request, - response: Response( - requestOptions: request, - statusCode: 401, - data: data, - ), - ); - final event = SentryEvent( - throwable: throwable, - exceptions: [ - fixture.sentryError(throwable), - fixture.sentryError(dioError) - ], - ); - final processedEvent = sut.apply(event) as SentryEvent; - final capturedResponse = processedEvent.contexts.response; - - expect(capturedResponse, isNotNull); - expect( - capturedResponse?.data, - scenario.shouldBeIncluded ? isNotNull : isNull, - ); - } - }); } final requestOptions = RequestOptions( @@ -391,6 +444,7 @@ class Fixture { return DioEventProcessor( options ..sendDefaultPii = sendDefaultPii + ..captureFailedRequests = captureFailedRequests ..maxRequestBodySize = maxRequestBodySize ..maxResponseBodySize = maxResponseBodySize, ); From 326bff641448a37cfcd469aebe0896cc22fc18f9 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 24 Jul 2023 13:18:15 +0200 Subject: [PATCH 06/13] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5abe5ad314..b7a7d44a87 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ [Trace origin](https://develop.sentry.dev/sdk/performance/trace-origin/) indicates what created a trace or a span. Not all transactions and spans contain enough information to tell whether the user or what precisely in the SDK created it. Origin solves this problem. The SDK now sends origin for transactions and spans. - Append http response body ([#1557](https://github.com/getsentry/sentry-dart/pull/1557)) + ### Dependencies - Bump Cocoa SDK from v8.8.0 to v8.9.1 ([#1553](https://github.com/getsentry/sentry-dart/pull/1553)) From 67cd86666be6650a39fd089495ff686c093c65d6 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 24 Jul 2023 13:24:10 +0200 Subject: [PATCH 07/13] revert logging of streamed response --- .../http_client/failed_request_client.dart | 17 ------ .../failed_request_client_test.dart | 52 ------------------- flutter/example/lib/main.dart | 1 - 3 files changed, 70 deletions(-) diff --git a/dart/lib/src/http_client/failed_request_client.dart b/dart/lib/src/http_client/failed_request_client.dart index 6f7baa0e29..98df59194f 100644 --- a/dart/lib/src/http_client/failed_request_client.dart +++ b/dart/lib/src/http_client/failed_request_client.dart @@ -104,7 +104,6 @@ class FailedRequestClient extends BaseClient { try { response = await _client.send(request); statusCode = response.statusCode; - return response; } catch (e, st) { exception = e; @@ -211,9 +210,6 @@ class FailedRequestClient extends BaseClient { headers: _hub.options.sendDefaultPii ? response.headers : null, bodySize: response.contentLength, statusCode: response.statusCode, - data: _hub.options.sendDefaultPii - ? await _getDataFromStreamedResponse(response) - : null, ); hint.set(TypeCheckHint.httpResponse, response); } @@ -225,19 +221,6 @@ class FailedRequestClient extends BaseClient { ); } - Future _getDataFromStreamedResponse( - StreamedResponse streamedResponse) async { - final contentLength = streamedResponse.contentLength; - if (contentLength == null) { - return null; - } - if (!_hub.options.maxResponseBodySize.shouldAddBody(contentLength)) { - return null; - } - var response = await Response.fromStream(streamedResponse); - return response.body; - } - // Types of Request can be found here: // https://pub.dev/documentation/http/latest/http/http-library.html Object? _getDataFromRequest(BaseRequest request) { diff --git a/dart/test/http_client/failed_request_client_test.dart b/dart/test/http_client/failed_request_client_test.dart index db78406ce3..4e6cac15b1 100644 --- a/dart/test/http_client/failed_request_client_test.dart +++ b/dart/test/http_client/failed_request_client_test.dart @@ -1,5 +1,3 @@ -import 'dart:convert'; - import 'package:http/http.dart'; import 'package:http/testing.dart'; import 'package:mockito/mockito.dart'; @@ -283,56 +281,6 @@ void main() { } }); - 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), - ]; - - fixture._hub.options.captureFailedRequests = true; - fixture._hub.options.sendDefaultPii = true; - - for (final scenario in scenarios) { - fixture._hub.options.maxResponseBodySize = scenario.maxBodySize; - fixture.transport.reset(); - - final bodyBytes = List.generate(scenario.contentLength, (index) => 0); - final bodyString = utf8.decode(bodyBytes); - - final sut = fixture.getSut( - client: fixture.getClient(statusCode: 401, body: bodyString), - failedRequestStatusCodes: [SentryStatusCode(401)], - ); - - final request = Request('GET', requestUri); - await sut.send(request); - - expect(fixture.transport.calls, 1); - - final eventCall = fixture.transport.events.first; - final capturedResponse = eventCall.contexts.response; - expect(capturedResponse, isNotNull); - expect(capturedResponse?.data, - scenario.shouldBeIncluded ? isNotNull : isNull); - } - }); - test('request passed to hint', () async { fixture._hub.options.captureFailedRequests = true; diff --git a/flutter/example/lib/main.dart b/flutter/example/lib/main.dart index 9d02d1ecba..6d657a589e 100644 --- a/flutter/example/lib/main.dart +++ b/flutter/example/lib/main.dart @@ -2,7 +2,6 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:io' show Platform; import 'package:flutter/foundation.dart'; import 'package:flutter/material.dart'; From 4b34e70241b05d03859cf2410bdf78d1d25138e2 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Mon, 24 Jul 2023 13:35:04 +0200 Subject: [PATCH 08/13] fix test expectations --- dio/test/dio_event_processor_test.dart | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/dio/test/dio_event_processor_test.dart b/dio/test/dio_event_processor_test.dart index d2a5531e80..af45d0c681 100644 --- a/dio/test/dio_event_processor_test.dart +++ b/dio/test/dio_event_processor_test.dart @@ -198,6 +198,7 @@ void main() { final request = requestOptions.copyWith( method: 'POST', + responseType: ResponseType.plain, ); final throwable = Exception(); final dioError = DioError( @@ -239,6 +240,7 @@ void main() { final request = requestOptions.copyWith( method: 'POST', + responseType: ResponseType.plain, ); final throwable = Exception(); final dioError = DioError( @@ -300,7 +302,11 @@ void main() { ); final data = List.generate(scenario.contentLength, (index) => 0); - final request = requestOptions.copyWith(method: 'POST', data: data); + final request = requestOptions.copyWith( + method: 'POST', + data: data, + responseType: ResponseType.bytes, + ); final throwable = Exception(); final dioError = DioError( requestOptions: request, From 101e319ce71e5f1e848a252e4e84f1466404f29b Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 25 Jul 2023 11:13:03 +0200 Subject: [PATCH 09/13] update changelog entry --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aa5a316521..6f09386519 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,8 @@ [Trace origin](https://develop.sentry.dev/sdk/performance/trace-origin/) indicates what created a trace or a span. Not all transactions and spans contain enough information to tell whether the user or what precisely in the SDK created it. Origin solves this problem. The SDK now sends origin for transactions and spans. -- Append http response body ([#1557](https://github.com/getsentry/sentry-dart/pull/1557)) - Add `appHangTimeoutInterval` to `SentryFlutterOptions` ([#1568](https://github.com/getsentry/sentry-dart/pull/1568)) +- DioEventProcessor: Append http response body ([#1557](https://github.com/getsentry/sentry-dart/pull/1557)) ### Dependencies From 3485b4f6a89bb72bce9e46c2239b22fe428c87af Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 25 Jul 2023 11:20:12 +0200 Subject: [PATCH 10/13] one early return --- dio/lib/src/dio_event_processor.dart | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/dio/lib/src/dio_event_processor.dart b/dio/lib/src/dio_event_processor.dart index b6a17b8e0a..12ffc66847 100644 --- a/dio/lib/src/dio_event_processor.dart +++ b/dio/lib/src/dio_event_processor.dart @@ -103,10 +103,7 @@ class DioEventProcessor implements EventProcessor { /// Returns the response data, if possible according to the users settings. Object? _getResponseData(Object? data, ResponseType responseType) { - if (!_options.sendDefaultPii) { - return null; - } - if (data == null) { + if (!_options.sendDefaultPii || data == null) { return null; } switch (responseType) { From a2b682e191142d7b47ae29fe50b5ade201bf9cf2 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 25 Jul 2023 11:43:36 +0200 Subject: [PATCH 11/13] update changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f09386519..a412afb301 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,8 @@ - Add `appHangTimeoutInterval` to `SentryFlutterOptions` ([#1568](https://github.com/getsentry/sentry-dart/pull/1568)) - DioEventProcessor: Append http response body ([#1557](https://github.com/getsentry/sentry-dart/pull/1557)) + - This is opt-in and depends on `maxResponseBodySize` + - Only for `dio` package ### Dependencies From 617aa2418833265e3d801ded2f77ca2b4feec351 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 25 Jul 2023 15:01:59 +0200 Subject: [PATCH 12/13] use utf8JsonEncoder --- dart/lib/sentry.dart | 2 ++ dart/lib/src/sentry_options.dart | 1 - dart/lib/src/utils.dart | 4 ++++ dart/test/protocol/breadcrumb_test.dart | 1 - dart/test/sentry_envelope_header_test.dart | 1 - dart/test/sentry_envelope_item_test.dart | 1 - dart/test/sentry_envelope_test.dart | 1 - dart/test/sentry_event_test.dart | 1 - dart/test/sentry_span_test.dart | 1 - dart/test/sentry_tracer_test.dart | 1 - dio/lib/src/dio_event_processor.dart | 5 +++-- 11 files changed, 9 insertions(+), 10 deletions(-) diff --git a/dart/lib/sentry.dart b/dart/lib/sentry.dart index f23525bfda..9d06bb7e2a 100644 --- a/dart/lib/sentry.dart +++ b/dart/lib/sentry.dart @@ -46,3 +46,5 @@ export 'src/utils/url_details.dart'; export 'src/utils/http_header_utils.dart'; // ignore: invalid_export_of_internal_element export 'src/sentry_trace_origins.dart'; +// ignore: invalid_export_of_internal_element +export 'src/utils.dart'; diff --git a/dart/lib/src/sentry_options.dart b/dart/lib/src/sentry_options.dart index 34544076ba..55450be9dd 100644 --- a/dart/lib/src/sentry_options.dart +++ b/dart/lib/src/sentry_options.dart @@ -13,7 +13,6 @@ import 'diagnostic_logger.dart'; import 'environment/environment_variables.dart'; import 'noop_client.dart'; import 'transport/noop_transport.dart'; -import 'utils.dart'; import 'version.dart'; // TODO: shutdownTimeout, flushTimeoutMillis diff --git a/dart/lib/src/utils.dart b/dart/lib/src/utils.dart index f349635051..792d5b2c5e 100644 --- a/dart/lib/src/utils.dart +++ b/dart/lib/src/utils.dart @@ -8,9 +8,11 @@ import 'package:meta/meta.dart'; /// Sentry does not take a timezone and instead expects the date-time to be /// submitted in UTC timezone. +@internal DateTime getUtcDateTime() => DateTime.now().toUtc(); /// Formats a Date as ISO8601 and UTC with millis precision +@internal String formatDateAsIso8601WithMillisPrecision(DateTime date) { var iso = date.toIso8601String(); final millisecondSeparatorIndex = iso.lastIndexOf('.'); @@ -22,9 +24,11 @@ String formatDateAsIso8601WithMillisPrecision(DateTime date) { return '${iso}Z'; } +@internal final utf8JsonEncoder = JsonUtf8Encoder(null, jsonSerializationFallback, null); @visibleForTesting +@internal Object? jsonSerializationFallback(Object? nonEncodable) { if (nonEncodable == null) { return null; diff --git a/dart/test/protocol/breadcrumb_test.dart b/dart/test/protocol/breadcrumb_test.dart index 4062e1b895..ddd4008d98 100644 --- a/dart/test/protocol/breadcrumb_test.dart +++ b/dart/test/protocol/breadcrumb_test.dart @@ -1,7 +1,6 @@ import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; import 'package:test/test.dart'; -import 'package:sentry/src/utils.dart'; void main() { final timestamp = DateTime.now(); diff --git a/dart/test/sentry_envelope_header_test.dart b/dart/test/sentry_envelope_header_test.dart index 5dff84aa87..cc10f97434 100644 --- a/dart/test/sentry_envelope_header_test.dart +++ b/dart/test/sentry_envelope_header_test.dart @@ -1,6 +1,5 @@ import 'package:sentry/sentry.dart'; import 'package:sentry/src/sentry_envelope_header.dart'; -import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; import 'mocks.dart'; diff --git a/dart/test/sentry_envelope_item_test.dart b/dart/test/sentry_envelope_item_test.dart index 4fe01e5a8e..673f679740 100644 --- a/dart/test/sentry_envelope_item_test.dart +++ b/dart/test/sentry_envelope_item_test.dart @@ -8,7 +8,6 @@ import 'package:sentry/src/sentry_envelope_item_header.dart'; import 'package:sentry/src/sentry_item_type.dart'; import 'package:sentry/src/sentry_tracer.dart'; import 'package:sentry/src/transport/data_category.dart'; -import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; import 'mocks/mock_hub.dart'; diff --git a/dart/test/sentry_envelope_test.dart b/dart/test/sentry_envelope_test.dart index 063be42ba9..375f62846e 100644 --- a/dart/test/sentry_envelope_test.dart +++ b/dart/test/sentry_envelope_test.dart @@ -6,7 +6,6 @@ import 'package:sentry/src/sentry_envelope_header.dart'; import 'package:sentry/src/sentry_envelope_item_header.dart'; import 'package:sentry/src/sentry_item_type.dart'; import 'package:sentry/src/sentry_tracer.dart'; -import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; import 'mocks.dart'; diff --git a/dart/test/sentry_event_test.dart b/dart/test/sentry_event_test.dart index 58fb946110..4e661638a8 100644 --- a/dart/test/sentry_event_test.dart +++ b/dart/test/sentry_event_test.dart @@ -5,7 +5,6 @@ import 'package:collection/collection.dart'; import 'package:sentry/sentry.dart'; import 'package:sentry/src/version.dart'; -import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; void main() { diff --git a/dart/test/sentry_span_test.dart b/dart/test/sentry_span_test.dart index 37a81309da..e878be5cf2 100644 --- a/dart/test/sentry_span_test.dart +++ b/dart/test/sentry_span_test.dart @@ -1,6 +1,5 @@ import 'package:sentry/sentry.dart'; import 'package:sentry/src/sentry_tracer.dart'; -import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; import 'mocks/mock_hub.dart'; diff --git a/dart/test/sentry_tracer_test.dart b/dart/test/sentry_tracer_test.dart index f2c0bcdf7c..cb9f72fdfc 100644 --- a/dart/test/sentry_tracer_test.dart +++ b/dart/test/sentry_tracer_test.dart @@ -1,6 +1,5 @@ import 'package:sentry/sentry.dart'; import 'package:sentry/src/sentry_tracer.dart'; -import 'package:sentry/src/utils.dart'; import 'package:test/test.dart'; import 'mocks.dart'; diff --git a/dio/lib/src/dio_event_processor.dart b/dio/lib/src/dio_event_processor.dart index 12ffc66847..3a9603abd1 100644 --- a/dio/lib/src/dio_event_processor.dart +++ b/dio/lib/src/dio_event_processor.dart @@ -108,8 +108,9 @@ class DioEventProcessor implements EventProcessor { } switch (responseType) { case ResponseType.json: - final js = json.encode(data); - if (_options.maxResponseBodySize.shouldAddBody(js.codeUnits.length)) { + // ignore: invalid_use_of_internal_member + final jsData = utf8JsonEncoder.convert(data); + if (_options.maxResponseBodySize.shouldAddBody(jsData.length)) { return data; } break; From 120b19390711841ff291e3cdd41cfaaa3bd4d1d6 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Wed, 26 Jul 2023 12:51:57 +0200 Subject: [PATCH 13/13] fix --- dart/lib/src/utils.dart | 1 - 1 file changed, 1 deletion(-) diff --git a/dart/lib/src/utils.dart b/dart/lib/src/utils.dart index 792d5b2c5e..db5ca614c2 100644 --- a/dart/lib/src/utils.dart +++ b/dart/lib/src/utils.dart @@ -27,7 +27,6 @@ String formatDateAsIso8601WithMillisPrecision(DateTime date) { @internal final utf8JsonEncoder = JsonUtf8Encoder(null, jsonSerializationFallback, null); -@visibleForTesting @internal Object? jsonSerializationFallback(Object? nonEncodable) { if (nonEncodable == null) {