From 2532dbf4afe9fb31563d900ca8b8920cec6fad70 Mon Sep 17 00:00:00 2001 From: Denis Andrasec Date: Tue, 19 Nov 2024 13:44:01 +0100 Subject: [PATCH] use yield for items in envelope, remove length closure from header, skip throwing envelope data closure --- dart/lib/src/sentry_envelope.dart | 31 +++--- dart/lib/src/sentry_envelope_item.dart | 102 ++++-------------- dart/lib/src/sentry_envelope_item_header.dart | 16 +-- dart/test/mocks/mock_transport.dart | 6 +- dart/test/sentry_attachment_test.dart | 2 +- dart/test/sentry_client_test.dart | 16 ++- .../sentry_envelope_item_header_test.dart | 7 +- dart/test/sentry_envelope_item_test.dart | 48 --------- dart/test/sentry_envelope_test.dart | 100 +++++++++++------ dart/test/sentry_envelope_vm_test.dart | 6 +- 10 files changed, 127 insertions(+), 207 deletions(-) diff --git a/dart/lib/src/sentry_envelope.dart b/dart/lib/src/sentry_envelope.dart index 9c02bfa8f1..45aa837e68 100644 --- a/dart/lib/src/sentry_envelope.dart +++ b/dart/lib/src/sentry_envelope.dart @@ -122,23 +122,26 @@ class SentryEnvelope { final newLineData = utf8.encode('\n'); for (final item in items) { - final length = await item.header.length(); - // A length smaller than 0 indicates an invalid envelope, which should not - // be send to Sentry.io - if (length < 0) { - continue; - } - // Only attachments should be filtered according to - // SentryOptions.maxAttachmentSize - if (item.header.type == SentryItemType.attachment) { - if (await item.header.length() > options.maxAttachmentSize) { + try { + final data = await item.dataFactory(); + + // Only attachments should be filtered according to + // SentryOptions.maxAttachmentSize + if (item.header.type == SentryItemType.attachment && + data.length > options.maxAttachmentSize) { continue; } - } - final itemStream = await item.envelopeItemStream(); - if (itemStream.isNotEmpty) { + + yield newLineData; + yield utf8.encode(jsonEncode(await item.header.toJson(data.length))); yield newLineData; - yield itemStream; + yield data; + } catch (_) { + if (options.automatedTestMode) { + rethrow; + } + // Skip throwing envelope item data closure. + continue; } } } diff --git a/dart/lib/src/sentry_envelope_item.dart b/dart/lib/src/sentry_envelope_item.dart index b11231691e..1e6602eec5 100644 --- a/dart/lib/src/sentry_envelope_item.dart +++ b/dart/lib/src/sentry_envelope_item.dart @@ -1,6 +1,5 @@ import 'dart:async'; import 'dart:convert'; -import 'dart:typed_data'; import 'client_reports/client_report.dart'; import 'metrics/metric.dart'; @@ -20,82 +19,72 @@ class SentryEnvelopeItem { /// Creates a [SentryEnvelopeItem] which sends [SentryTransaction]. factory SentryEnvelopeItem.fromTransaction(SentryTransaction transaction) { - final cachedItem = - _CachedItem(() async => utf8JsonEncoder.convert(transaction.toJson())); - final header = SentryEnvelopeItemHeader( SentryItemType.transaction, - cachedItem.getDataLength, contentType: 'application/json', ); - return SentryEnvelopeItem(header, cachedItem.getData, + return SentryEnvelopeItem( + header, () async => utf8JsonEncoder.convert(transaction.toJson()), originalObject: transaction); } factory SentryEnvelopeItem.fromAttachment(SentryAttachment attachment) { - final cachedItem = _CachedItem(() async => attachment.bytes); - final header = SentryEnvelopeItemHeader( SentryItemType.attachment, - cachedItem.getDataLength, contentType: attachment.contentType, fileName: attachment.filename, attachmentType: attachment.attachmentType, ); - return SentryEnvelopeItem(header, cachedItem.getData, - originalObject: attachment); + return SentryEnvelopeItem( + header, + () async => attachment.bytes, + originalObject: attachment, + ); } /// Create a [SentryEnvelopeItem] which sends [SentryUserFeedback]. @Deprecated('Will be removed in a future version.') factory SentryEnvelopeItem.fromUserFeedback(SentryUserFeedback feedback) { - final cachedItem = - _CachedItem(() async => utf8JsonEncoder.convert(feedback.toJson())); + final dataFactory = () async => utf8JsonEncoder.convert(feedback.toJson()); final header = SentryEnvelopeItemHeader( SentryItemType.userFeedback, - cachedItem.getDataLength, contentType: 'application/json', ); - return SentryEnvelopeItem(header, cachedItem.getData, - originalObject: feedback); + return SentryEnvelopeItem( + header, + dataFactory, + originalObject: feedback, + ); } /// Create a [SentryEnvelopeItem] which holds the [SentryEvent] data. factory SentryEnvelopeItem.fromEvent(SentryEvent event) { - final cachedItem = - _CachedItem(() async => utf8JsonEncoder.convert(event.toJson())); - return SentryEnvelopeItem( SentryEnvelopeItemHeader( event.type == 'feedback' ? 'feedback' : SentryItemType.event, - cachedItem.getDataLength, contentType: 'application/json', ), - cachedItem.getData, + () async => utf8JsonEncoder.convert(event.toJson()), originalObject: event, ); } /// Create a [SentryEnvelopeItem] which holds the [ClientReport] data. factory SentryEnvelopeItem.fromClientReport(ClientReport clientReport) { - final cachedItem = - _CachedItem(() async => utf8JsonEncoder.convert(clientReport.toJson())); - return SentryEnvelopeItem( SentryEnvelopeItemHeader( SentryItemType.clientReport, - cachedItem.getDataLength, contentType: 'application/json', ), - cachedItem.getData, + () async => utf8JsonEncoder.convert(clientReport.toJson()), originalObject: clientReport, ); } /// Creates a [SentryEnvelopeItem] which holds several [Metric] data. factory SentryEnvelopeItem.fromMetrics(Map> buckets) { - final cachedItem = _CachedItem(() async { + final dataFactory = () async { final statsd = StringBuffer(); // Encode all metrics of a bucket in statsd format, using the bucket key, // which is the timestamp of the bucket. @@ -105,15 +94,16 @@ class SentryEnvelopeItem { statsd.write(encodedMetrics.join('\n')); } return utf8.encode(statsd.toString()); - }); - + }; final header = SentryEnvelopeItemHeader( SentryItemType.statsd, - cachedItem.getDataLength, contentType: 'application/octet-stream', ); - return SentryEnvelopeItem(header, cachedItem.getData, - originalObject: buckets); + return SentryEnvelopeItem( + header, + dataFactory, + originalObject: buckets, + ); } /// Header with info about type and length of data in bytes. @@ -121,52 +111,4 @@ class SentryEnvelopeItem { /// Create binary data representation of item data. final Future> Function() dataFactory; - - /// Stream binary data of `Envelope` item. - Future> envelopeItemStream() async { - // Each item needs to be encoded as one unit. - // Otherwise the header already got yielded if the content throws - // an exception. - try { - final itemHeader = utf8JsonEncoder.convert(await header.toJson()); - final newLine = utf8.encode('\n'); - final data = await dataFactory(); - - final totalLength = itemHeader.length + newLine.length + data.length; - - final result = Uint8List(totalLength); - result.setRange(0, itemHeader.length, itemHeader); - result.setRange( - itemHeader.length, - itemHeader.length + newLine.length, - newLine, - ); - result.setRange(itemHeader.length + newLine.length, totalLength, data); - - return result; - } catch (e) { - // TODO rethrow in options.automatedTestMode (currently not available here to check) - return []; - } - } -} - -class _CachedItem { - _CachedItem(this._dataFactory); - - final Future> Function() _dataFactory; - List? _data; - - Future> getData() async { - _data ??= await _dataFactory(); - return _data!; - } - - Future getDataLength() async { - try { - return (await getData()).length; - } catch (_) { - return -1; - } - } } diff --git a/dart/lib/src/sentry_envelope_item_header.dart b/dart/lib/src/sentry_envelope_item_header.dart index 99ad4607fe..c1e742cfd1 100644 --- a/dart/lib/src/sentry_envelope_item_header.dart +++ b/dart/lib/src/sentry_envelope_item_header.dart @@ -1,8 +1,7 @@ /// Header with item info about type and length of data in bytes. class SentryEnvelopeItemHeader { SentryEnvelopeItemHeader( - this.type, - this.length, { + this.type, { this.contentType, this.fileName, this.attachmentType, @@ -11,11 +10,6 @@ class SentryEnvelopeItemHeader { /// Type of encoded data. final String type; - /// The number of bytes of the encoded item JSON. - /// A negative number indicates an invalid envelope which should not be send - /// to Sentry.io. - final Future Function() length; - final String? contentType; final String? fileName; @@ -23,15 +17,13 @@ class SentryEnvelopeItemHeader { final String? attachmentType; /// Item header encoded as JSON - Future> toJson() async { - final json = { + Future> toJson(int length) async { + return { if (contentType != null) 'content_type': contentType, if (fileName != null) 'filename': fileName, if (attachmentType != null) 'attachment_type': attachmentType, 'type': type, - 'length': await length(), + 'length': length, }; - - return json; } } diff --git a/dart/test/mocks/mock_transport.dart b/dart/test/mocks/mock_transport.dart index 024fd70843..4f4c117dde 100644 --- a/dart/test/mocks/mock_transport.dart +++ b/dart/test/mocks/mock_transport.dart @@ -42,11 +42,11 @@ class MockTransport implements Transport { } Future _eventFromEnvelope(SentryEnvelope envelope) async { - final envelopeItemData = []; final RegExp statSdRegex = RegExp('^(?!{).+@.+:.+\\|.+', multiLine: true); - envelopeItemData.addAll(await envelope.items.first.envelopeItemStream()); - final envelopeItem = utf8.decode(envelopeItemData).split('\n').last; + final envelopeItemData = await envelope.items.first.dataFactory(); + final envelopeItem = utf8.decode(envelopeItemData); + if (statSdRegex.hasMatch(envelopeItem)) { statsdItems.add(envelopeItem); } else { diff --git a/dart/test/sentry_attachment_test.dart b/dart/test/sentry_attachment_test.dart index 267ca62601..060b8578f0 100644 --- a/dart/test/sentry_attachment_test.dart +++ b/dart/test/sentry_attachment_test.dart @@ -80,7 +80,7 @@ void main() { 'test.txt', ); await expectLater( - await attachmentEnvelope.header.length(), + (await attachmentEnvelope.dataFactory()).length, 4, ); }); diff --git a/dart/test/sentry_client_test.dart b/dart/test/sentry_client_test.dart index 17b209ec83..889e7fa88c 100644 --- a/dart/test/sentry_client_test.dart +++ b/dart/test/sentry_client_test.dart @@ -2197,21 +2197,17 @@ void main() { } Future eventFromEnvelope(SentryEnvelope envelope) async { - final envelopeItemData = []; - envelopeItemData.addAll(await envelope.items.first.envelopeItemStream()); - - final envelopeItem = utf8.decode(envelopeItemData); - final envelopeItemJson = jsonDecode(envelopeItem.split('\n').last); + final data = await envelope.items.first.dataFactory(); + final utf8Data = utf8.decode(data); + final envelopeItemJson = jsonDecode(utf8Data); return SentryEvent.fromJson(envelopeItemJson as Map); } Future> transactionFromEnvelope( SentryEnvelope envelope) async { - final envelopeItemData = []; - envelopeItemData.addAll(await envelope.items.first.envelopeItemStream()); - - final envelopeItem = utf8.decode(envelopeItemData); - final envelopeItemJson = jsonDecode(envelopeItem.split('\n').last); + final data = await envelope.items.first.dataFactory(); + final utf8Data = utf8.decode(data); + final envelopeItemJson = jsonDecode(utf8Data); return envelopeItemJson as Map; } diff --git a/dart/test/sentry_envelope_item_header_test.dart b/dart/test/sentry_envelope_item_header_test.dart index 8aca9316a3..3ffccaf0b4 100644 --- a/dart/test/sentry_envelope_item_header_test.dart +++ b/dart/test/sentry_envelope_item_header_test.dart @@ -5,15 +5,14 @@ import 'package:test/test.dart'; void main() { group('SentryEnvelopeItemHeader', () { test('serialize', () async { - final sut = SentryEnvelopeItemHeader(SentryItemType.event, () async { - return 3; - }, contentType: 'application/json'); + final sut = SentryEnvelopeItemHeader(SentryItemType.event, + contentType: 'application/json'); final expected = { 'content_type': 'application/json', 'type': 'event', 'length': 3 }; - expect(await sut.toJson(), expected); + expect(await sut.toJson(3), expected); }); }); } diff --git a/dart/test/sentry_envelope_item_test.dart b/dart/test/sentry_envelope_item_test.dart index 9074ee712c..1af2756f9a 100644 --- a/dart/test/sentry_envelope_item_test.dart +++ b/dart/test/sentry_envelope_item_test.dart @@ -5,7 +5,6 @@ import 'package:sentry/src/client_reports/client_report.dart'; import 'package:sentry/src/client_reports/discard_reason.dart'; import 'package:sentry/src/client_reports/discarded_event.dart'; import 'package:sentry/src/metrics/metric.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/transport/data_category.dart'; @@ -16,29 +15,6 @@ import 'mocks/mock_hub.dart'; void main() { group('SentryEnvelopeItem', () { - test('serialize', () async { - final header = SentryEnvelopeItemHeader(SentryItemType.event, () async { - return 9; - }, contentType: 'application/json'); - - final dataFactory = () async { - return utf8.encode('{fixture}'); - }; - - final sut = SentryEnvelopeItem(header, dataFactory); - - final headerJson = await header.toJson(); - final headerJsonEncoded = jsonEncode( - headerJson, - toEncodable: jsonSerializationFallback, - ); - final expected = utf8.encode('$headerJsonEncoded\n{fixture}'); - - final actualItem = await sut.envelopeItemStream(); - - expect(actualItem, expected); - }); - test('fromEvent', () async { final eventId = SentryId.newId(); final sentryEvent = SentryEvent(eventId: eventId); @@ -50,12 +26,8 @@ void main() { )); final actualData = await sut.dataFactory(); - final expectedLength = expectedData.length; - final actualLength = await sut.header.length(); - expect(sut.header.contentType, 'application/json'); expect(sut.header.type, SentryItemType.event); - expect(actualLength, expectedLength); expect(actualData, expectedData); }); @@ -76,12 +48,8 @@ void main() { )); final actualData = await sut.dataFactory(); - final expectedLength = expectedData.length; - final actualLength = await sut.header.length(); - expect(sut.header.contentType, 'application/json'); expect(sut.header.type, 'feedback'); - expect(actualLength, expectedLength); expect(actualData, expectedData); }); @@ -104,12 +72,8 @@ void main() { )); final actualData = await sut.dataFactory(); - final expectedLength = expectedData.length; - final actualLength = await sut.header.length(); - expect(sut.header.contentType, 'application/json'); expect(sut.header.type, SentryItemType.transaction); - expect(actualLength, expectedLength); expect(actualData, expectedData); }); @@ -129,12 +93,8 @@ void main() { )); final actualData = await sut.dataFactory(); - final expectedLength = expectedData.length; - final actualLength = await sut.header.length(); - expect(sut.header.contentType, 'application/json'); expect(sut.header.type, SentryItemType.clientReport); - expect(actualLength, expectedLength); expect(actualData, expectedData); }); @@ -154,12 +114,8 @@ void main() { )); final actualData = await sut.dataFactory(); - final expectedLength = expectedData.length; - final actualLength = await sut.header.length(); - expect(sut.header.contentType, 'application/json'); expect(sut.header.type, SentryItemType.userFeedback); - expect(actualLength, expectedLength); expect(actualData, expectedData); }); @@ -175,12 +131,8 @@ void main() { final expectedData = utf8.encode(statsd.toString()); final actualData = await sut.dataFactory(); - final expectedLength = expectedData.length; - final actualLength = await sut.header.length(); - expect(sut.header.contentType, 'application/octet-stream'); expect(sut.header.type, SentryItemType.statsd); - expect(actualLength, expectedLength); expect(actualData, expectedData); }); }); diff --git a/dart/test/sentry_envelope_test.dart b/dart/test/sentry_envelope_test.dart index bf30b853c4..973980d767 100644 --- a/dart/test/sentry_envelope_test.dart +++ b/dart/test/sentry_envelope_test.dart @@ -14,13 +14,24 @@ import 'test_utils.dart'; void main() { group('SentryEnvelope', () { + Future serializedItem(SentryEnvelopeItem item) async { + final expectedItemData = await item.dataFactory(); + final expectedItemHeader = utf8JsonEncoder + .convert(await item.header.toJson(expectedItemData.length)); + final newLine = utf8.encode('\n'); + final expectedItem = [ + ...expectedItemHeader, + ...newLine, + ...expectedItemData + ]; + return utf8.decode(expectedItem); + } + test('serialize', () async { final eventId = SentryId.newId(); - final itemHeader = - SentryEnvelopeItemHeader(SentryItemType.event, () async { - return 9; - }, contentType: 'application/json'); + final itemHeader = SentryEnvelopeItemHeader(SentryItemType.event, + contentType: 'application/json'); final dataFactory = () async { return utf8.encode('{fixture}'); @@ -45,8 +56,7 @@ void main() { toEncodable: jsonSerializationFallback, ); - final expectedItem = await item.envelopeItemStream(); - final expectedItemSerialized = utf8.decode(expectedItem); + final expectedItemSerialized = await serializedItem(item); final expected = utf8.encode( '$expectedHeaderJsonSerialized\n$expectedItemSerialized\n$expectedItemSerialized'); @@ -83,13 +93,9 @@ void main() { expect(sut.items[0].header.contentType, expectedEnvelopeItem.header.contentType); expect(sut.items[0].header.type, expectedEnvelopeItem.header.type); - expect(await sut.items[0].header.length(), - await expectedEnvelopeItem.header.length()); - - final actualItem = await sut.items[0].envelopeItemStream(); - - final expectedItem = await expectedEnvelopeItem.envelopeItemStream(); + final actualItem = await sut.items[0].dataFactory(); + final expectedItem = await expectedEnvelopeItem.dataFactory(); expect(actualItem, expectedItem); }); @@ -123,13 +129,9 @@ void main() { expect(sut.items[0].header.contentType, expectedEnvelopeItem.header.contentType); expect(sut.items[0].header.type, expectedEnvelopeItem.header.type); - expect(await sut.items[0].header.length(), - await expectedEnvelopeItem.header.length()); - - final actualItem = await sut.items[0].envelopeItemStream(); - - final expectedItem = await expectedEnvelopeItem.envelopeItemStream(); + final actualItem = await sut.items[0].dataFactory(); + final expectedItem = await expectedEnvelopeItem.dataFactory(); expect(actualItem, expectedItem); }); @@ -157,13 +159,9 @@ void main() { expect(sut.items[0].header.contentType, expectedEnvelopeItem.header.contentType); expect(sut.items[0].header.type, expectedEnvelopeItem.header.type); - expect(await sut.items[0].header.length(), - await expectedEnvelopeItem.header.length()); - - final actualItem = await sut.items[0].envelopeItemStream(); - - final expectedItem = await expectedEnvelopeItem.envelopeItemStream(); + final actualItem = await sut.items[0].dataFactory(); + final expectedItem = await expectedEnvelopeItem.dataFactory(); expect(actualItem, expectedItem); }); @@ -183,13 +181,9 @@ void main() { expect(sut.items[0].header.contentType, expectedEnvelopeItem.header.contentType); expect(sut.items[0].header.type, expectedEnvelopeItem.header.type); - expect(await sut.items[0].header.length(), - await expectedEnvelopeItem.header.length()); - - final actualItem = await sut.items[0].envelopeItemStream(); - - final expectedItem = await expectedEnvelopeItem.envelopeItemStream(); + final actualItem = await sut.items[0].dataFactory(); + final expectedItem = await expectedEnvelopeItem.dataFactory(); expect(actualItem, expectedItem); }); @@ -230,11 +224,55 @@ void main() { expect(sutEnvelopeData, envelopeData); }); + test('ignore throwing envelope items', () async { + final eventId = SentryId.newId(); + + final itemHeader = SentryEnvelopeItemHeader(SentryItemType.event, + contentType: 'application/json'); + final dataFactory = () async { + return utf8.encode('{fixture}'); + }; + final dataFactoryThrowing = () async { + throw Exception('Exception in data factory.'); + }; + + final item = SentryEnvelopeItem(itemHeader, dataFactory); + final throwingItem = SentryEnvelopeItem(itemHeader, dataFactoryThrowing); + + final context = SentryTraceContextHeader.fromJson({ + 'trace_id': '${SentryId.newId()}', + 'public_key': '123', + }); + final header = SentryEnvelopeHeader( + eventId, + null, + traceContext: context, + ); + final sut = SentryEnvelope(header, [item, throwingItem]); + + final expectedHeaderJson = header.toJson(); + final expectedHeaderJsonSerialized = jsonEncode( + expectedHeaderJson, + toEncodable: jsonSerializationFallback, + ); + + final expectedItemSerialized = await serializedItem(item); + + final expected = + utf8.encode('$expectedHeaderJsonSerialized\n$expectedItemSerialized'); + + final envelopeData = []; + await sut + .envelopeStream(defaultTestOptions()) + .forEach(envelopeData.addAll); + expect(envelopeData, expected); + }); + // This test passes if no exceptions are thrown, thus no asserts. // This is a test for https://github.com/getsentry/sentry-dart/issues/523 test('serialize with non-serializable class', () async { // ignore: deprecated_member_use_from_same_package - final event = SentryEvent(extra: {'non-ecodable': NonEncodable()}); + final event = SentryEvent(extra: {'non-encodable': NonEncodable()}); final sut = SentryEnvelope.fromEvent( event, SdkVersion( diff --git a/dart/test/sentry_envelope_vm_test.dart b/dart/test/sentry_envelope_vm_test.dart index e5f0276704..0afafd48f6 100644 --- a/dart/test/sentry_envelope_vm_test.dart +++ b/dart/test/sentry_envelope_vm_test.dart @@ -17,15 +17,13 @@ void main() { test('item with binary payload', () async { // Attachment - final length = () async { - return 3535; - }; + // length == 3535 final dataFactory = () async { final file = File('test_resources/sentry.png'); final bytes = await file.readAsBytes(); return bytes; }; - final attachmentHeader = SentryEnvelopeItemHeader('attachment', length, + final attachmentHeader = SentryEnvelopeItemHeader('attachment', contentType: 'image/png', fileName: 'sentry.png'); final attachmentItem = SentryEnvelopeItem(attachmentHeader, dataFactory);