Skip to content

Commit 073562b

Browse files
philprimephilprime
authored
refactor: Add nullability handling for data serialization (#5742)
1 parent 01faa71 commit 073562b

File tree

7 files changed

+190
-35
lines changed

7 files changed

+190
-35
lines changed

‎Sources/Sentry/SentryClient.m‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -541,8 +541,16 @@ - (void)captureReplayEvent:(SentryReplayEvent *)replayEvent
541541
withScope:scope
542542
alwaysAttachStacktrace:NO];
543543

544+
if (replayEvent == nil) {
545+
SENTRY_LOG_DEBUG(@"The replay event was filtered out in prepare event. "
546+
@"The replay was discarded.");
547+
return;
548+
}
549+
550+
// Only check the type of the returned event, as the instance could be changed in the event
551+
// preprocessor and before-send handlers.
544552
if (![replayEvent isKindOfClass:SentryReplayEvent.class]) {
545-
SENTRY_LOG_DEBUG(@"The event preprocessor didn't update the replay event in place. The "
553+
SENTRY_LOG_ERROR(@"The event preprocessor didn't update the replay event in place. The "
546554
@"replay was discarded.");
547555
return;
548556
}
@@ -553,8 +561,13 @@ - (void)captureReplayEvent:(SentryReplayEvent *)replayEvent
553561
video:videoURL];
554562

555563
if (videoEnvelopeItem == nil) {
556-
SENTRY_LOG_DEBUG(@"The Session Replay segment will not be sent to Sentry because an "
564+
SENTRY_LOG_ERROR(@"The Session Replay segment will not be sent to Sentry because an "
557565
@"Envelope Item could not be created.");
566+
// Record a counted lost event in case preparing the event (e.g. encoding the event) failed.
567+
// This is used to determine if replay events are missing due to an error in the SDK.
568+
[self recordLostEvent:kSentryDataCategoryReplay
569+
reason:kSentryDiscardReasonInsufficientData
570+
quantity:1];
558571
return;
559572
}
560573

‎Sources/Sentry/SentryEnvelope.m‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
#import "SentryEnvelopeItemType.h"
88
#import "SentryEvent+Serialize.h"
99
#import "SentryEvent.h"
10+
#import "SentryInternalDefines.h"
1011
#import "SentryLogC.h"
1112
#import "SentryMessage.h"
1213
#import "SentryMsgPackSerializer.h"
@@ -176,11 +177,12 @@ - (_Nullable instancetype)initWithAttachment:(SentryAttachment *)attachment
176177
data = attachment.data;
177178
#endif // DEBUG || SENTRY_TEST || SENTRY_TEST_CI
178179
} else if (nil != attachment.path) {
180+
NSString *_Nonnull attachmentPath = SENTRY_UNWRAP_NULLABLE(NSString, attachment.path);
179181

180182
NSError *error = nil;
181183
NSFileManager *fileManager = [NSFileManager defaultManager];
182184
NSDictionary<NSFileAttributeKey, id> *attr =
183-
[fileManager attributesOfItemAtPath:attachment.path error:&error];
185+
[fileManager attributesOfItemAtPath:attachmentPath error:&error];
184186

185187
if (nil != error) {
186188
SENTRY_LOG_ERROR(@"Couldn't check file size of attachment with path: %@. Error: %@",
@@ -202,13 +204,13 @@ - (_Nullable instancetype)initWithAttachment:(SentryAttachment *)attachment
202204
#if DEBUG || SENTRY_TEST || SENTRY_TEST_CI
203205
if ([NSProcessInfo.processInfo.arguments
204206
containsObject:@"--io.sentry.other.base64-attachment-data"]) {
205-
data = [[[[NSFileManager defaultManager] contentsAtPath:attachment.path]
207+
data = [[[[NSFileManager defaultManager] contentsAtPath:attachmentPath]
206208
base64EncodedStringWithOptions:0] dataUsingEncoding:NSUTF8StringEncoding];
207209
} else {
208-
data = [[NSFileManager defaultManager] contentsAtPath:attachment.path];
210+
data = [[NSFileManager defaultManager] contentsAtPath:attachmentPath];
209211
}
210212
#else
211-
data = [[NSFileManager defaultManager] contentsAtPath:attachment.path];
213+
data = [[NSFileManager defaultManager] contentsAtPath:attachmentPath];
212214
#endif // DEBUG || SENTRY_TEST || SENTRY_TEST_CI
213215
}
214216

@@ -231,8 +233,24 @@ - (nullable instancetype)initWithReplayEvent:(SentryReplayEvent *)replayEvent
231233
replayRecording:(SentryReplayRecording *)replayRecording
232234
video:(NSURL *)videoURL
233235
{
234-
NSData *replayEventData = [SentrySerialization dataWithJSONObject:[replayEvent serialize]];
235-
NSData *recording = [SentrySerialization dataWithReplayRecording:replayRecording];
236+
NSData *_Nullable nullableReplayEventData =
237+
[SentrySerialization dataWithJSONObject:[replayEvent serialize]];
238+
if (nil == nullableReplayEventData) {
239+
SENTRY_LOG_ERROR(
240+
@"Could not serialize replay event data for envelope item. Event will be nil.");
241+
return nil;
242+
}
243+
NSData *_Nonnull replayEventData = SENTRY_UNWRAP_NULLABLE(NSData, nullableReplayEventData);
244+
245+
NSData *_Nullable nullableRecording =
246+
[SentrySerialization dataWithReplayRecording:replayRecording];
247+
if (nil == nullableRecording) {
248+
SENTRY_LOG_ERROR(
249+
@"Could not serialize replay recording data for envelope item. Recording will be nil.");
250+
return nil;
251+
}
252+
NSData *_Nonnull recording = SENTRY_UNWRAP_NULLABLE(NSData, nullableRecording);
253+
236254
NSURL *envelopeContentUrl =
237255
[[videoURL URLByDeletingPathExtension] URLByAppendingPathExtension:@"dat"];
238256

‎Sources/Sentry/SentrySerialization.m‎

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
#import "SentryEnvelopeAttachmentHeader.h"
55
#import "SentryEnvelopeItemType.h"
66
#import "SentryError.h"
7+
#import "SentryInternalDefines.h"
78
#import "SentryLevelMapper.h"
89
#import "SentryLogC.h"
910
#import "SentryModels+Serializable.h"
@@ -61,8 +62,9 @@ + (NSData *_Nullable)dataWithEnvelope:(SentryEnvelope *)envelope
6162
}
6263
[envelopeData appendData:header];
6364

65+
NSData *_Nonnull const newLineData = [NSData dataWithBytes:"\n" length:1];
6466
for (int i = 0; i < envelope.items.count; ++i) {
65-
[envelopeData appendData:[@"\n" dataUsingEncoding:NSUTF8StringEncoding]];
67+
[envelopeData appendData:newLineData];
6668
NSDictionary *serializedItemHeaderData = [envelope.items[i].header serialize];
6769

6870
NSData *itemHeader = [SentrySerialization dataWithJSONObject:serializedItemHeaderData];
@@ -71,7 +73,7 @@ + (NSData *_Nullable)dataWithEnvelope:(SentryEnvelope *)envelope
7173
return nil;
7274
}
7375
[envelopeData appendData:itemHeader];
74-
[envelopeData appendData:[@"\n" dataUsingEncoding:NSUTF8StringEncoding]];
76+
[envelopeData appendData:newLineData];
7577
[envelopeData appendData:envelope.items[i].data];
7678
}
7779

@@ -110,21 +112,27 @@ + (SentryEnvelope *_Nullable)envelopeWithData:(NSData *)data
110112
}
111113

112114
SentrySdkInfo *sdkInfo = nil;
113-
if (nil != headerDictionary[@"sdk"]) {
114-
sdkInfo = [[SentrySdkInfo alloc] initWithDict:headerDictionary[@"sdk"]];
115+
if (nil != headerDictionary[@"sdk"] &&
116+
[headerDictionary[@"sdk"] isKindOfClass:[NSDictionary class]]) {
117+
sdkInfo = [[SentrySdkInfo alloc]
118+
initWithDict:SENTRY_UNWRAP_NULLABLE(NSDictionary, headerDictionary[@"sdk"])];
115119
}
116120

117121
SentryTraceContext *traceContext = nil;
118-
if (nil != headerDictionary[@"trace"]) {
119-
traceContext = [[SentryTraceContext alloc] initWithDict:headerDictionary[@"trace"]];
122+
if (nil != headerDictionary[@"trace"] &&
123+
[headerDictionary[@"trace"] isKindOfClass:[NSDictionary class]]) {
124+
traceContext = [[SentryTraceContext alloc]
125+
initWithDict:SENTRY_UNWRAP_NULLABLE(NSDictionary, headerDictionary[@"trace"])];
120126
}
121127

122128
envelopeHeader = [[SentryEnvelopeHeader alloc] initWithId:eventId
123129
sdkInfo:sdkInfo
124130
traceContext:traceContext];
125131

126-
if (headerDictionary[@"sent_at"] != nil) {
127-
envelopeHeader.sentAt = sentry_fromIso8601String(headerDictionary[@"sent_at"]);
132+
if (headerDictionary[@"sent_at"] != nil &&
133+
[headerDictionary[@"sent_at"] isKindOfClass:[NSString class]]) {
134+
envelopeHeader.sentAt = sentry_fromIso8601String(
135+
SENTRY_UNWRAP_NULLABLE(NSString, headerDictionary[@"sent_at"]));
128136
}
129137

130138
break;
@@ -165,11 +173,13 @@ + (SentryEnvelope *_Nullable)envelopeWithData:(NSData *)data
165173
SENTRY_LOG_ERROR(@"Failed to parse envelope item header %@", error);
166174
return nil;
167175
}
168-
NSString *_Nullable type = [headerDictionary valueForKey:@"type"];
169-
if (nil == type) {
176+
NSString *_Nullable nullableType = [headerDictionary valueForKey:@"type"];
177+
if (nil == nullableType) {
170178
SENTRY_LOG_ERROR(@"Envelope item type is required.");
171179
break;
172180
}
181+
NSString *_Nonnull type = SENTRY_UNWRAP_NULLABLE(NSString, nullableType);
182+
173183
NSNumber *bodyLengthNumber = [headerDictionary valueForKey:@"length"];
174184
NSUInteger bodyLength = [bodyLengthNumber unsignedIntegerValue];
175185
if (endOfEnvelope == i && bodyLength != 0) {
@@ -259,13 +269,28 @@ + (SentrySession *_Nullable)sessionWithData:(NSData *)sessionData
259269
return session;
260270
}
261271

262-
+ (NSData *)dataWithReplayRecording:(SentryReplayRecording *)replayRecording
272+
+ (NSData *_Nullable)dataWithReplayRecording:(SentryReplayRecording *)replayRecording
263273
{
264274
NSMutableData *recording = [NSMutableData data];
265-
[recording appendData:[SentrySerialization
266-
dataWithJSONObject:[replayRecording headerForReplayRecording]]];
267-
[recording appendData:[@"\n" dataUsingEncoding:NSUTF8StringEncoding]];
268-
[recording appendData:[SentrySerialization dataWithJSONObject:[replayRecording serialize]]];
275+
276+
NSData *_Nullable headerData =
277+
[SentrySerialization dataWithJSONObject:[replayRecording headerForReplayRecording]];
278+
if (headerData == nil) {
279+
SENTRY_LOG_ERROR(@"Failed to serialize replay recording header.");
280+
return nil;
281+
}
282+
[recording appendData:SENTRY_UNWRAP_NULLABLE(NSData, headerData)];
283+
284+
NSData *_Nonnull const newLineData = [NSData dataWithBytes:"\n" length:1];
285+
[recording appendData:newLineData];
286+
287+
NSData *_Nullable replayData =
288+
[SentrySerialization dataWithJSONObject:[replayRecording serialize]];
289+
if (replayData == nil) {
290+
SENTRY_LOG_ERROR(@"Failed to serialize replay recording data.");
291+
return nil;
292+
}
293+
[recording appendData:SENTRY_UNWRAP_NULLABLE(NSData, replayData)];
269294
return recording;
270295
}
271296

‎Sources/Sentry/include/SentrySerialization.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ NS_ASSUME_NONNULL_BEGIN
1717

1818
+ (NSData *_Nullable)dataWithEnvelope:(SentryEnvelope *)envelope;
1919

20-
+ (NSData *)dataWithReplayRecording:(SentryReplayRecording *)replayRecording;
20+
+ (NSData *_Nullable)dataWithReplayRecording:(SentryReplayRecording *)replayRecording;
2121

2222
+ (SentryEnvelope *_Nullable)envelopeWithData:(NSData *)data;
2323

‎Tests/SentryTests/Helper/SentrySerializationTests.swift‎

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ class SentrySerializationTests: XCTestCase {
539539
XCTAssertNil(SentrySerialization.session(with: data))
540540
}
541541

542-
func testSerializeReplayRecording() {
542+
func testSerializeReplayRecording() throws {
543543
class MockReplayRecording: SentryReplayRecording {
544544
override func serialize() -> [[String: Any]] {
545545
return [["KEY": "VALUE"]]
@@ -548,13 +548,49 @@ class SentrySerializationTests: XCTestCase {
548548

549549
let date = Date(timeIntervalSince1970: 2)
550550
let recording = MockReplayRecording(segmentId: 5, size: 5_000, start: date, duration: 5_000, frameCount: 5, frameRate: 1, height: 320, width: 950, extraEvents: [])
551-
let data = SentrySerialization.data(with: recording)
552-
551+
let data = try XCTUnwrap(SentrySerialization.data(with: recording))
552+
553553
let serialized = String(data: data, encoding: .utf8)
554554

555555
XCTAssertEqual(serialized, "{\"segment_id\":5}\n[{\"KEY\":\"VALUE\"}]")
556556
}
557-
557+
558+
func testDataWithReplayRecording_whenHeaderCanNotBeSerialized_shouldReturnNil() throws {
559+
// -- Arrange --
560+
class MockReplayRecording: SentryReplayRecording {
561+
override func headerForReplayRecording() -> [String: Any] {
562+
// This will cause serialization to fail, because NSObject cannot be serialized to JSON
563+
return ["KEY": NSObject()]
564+
}
565+
}
566+
567+
let recording = MockReplayRecording(segmentId: 5, size: 5_000, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 320, width: 950, extraEvents: [])
568+
569+
// -- Act --
570+
let result = SentrySerialization.data(with: recording)
571+
572+
// -- Assert --
573+
XCTAssertNil(result, "Data serialization should return nil when the header cannot be serialized.")
574+
}
575+
576+
func testDataWithReplayRecording_whenRecordingCanNotBeSerialized_shouldReturnNil() throws {
577+
// -- Arrange --
578+
class MockReplayRecording: SentryReplayRecording {
579+
override func serialize() -> [[String: Any]] {
580+
// This will cause serialization to fail, because NSObject cannot be serialized to JSON
581+
return [["KEY": NSObject()]]
582+
}
583+
}
584+
585+
let recording = MockReplayRecording(segmentId: 5, size: 5_000, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 320, width: 950, extraEvents: [])
586+
587+
// -- Act --
588+
let result = SentrySerialization.data(with: recording)
589+
590+
// -- Assert --
591+
XCTAssertNil(result, "Data serialization should return nil when the header cannot be serialized.")
592+
}
593+
558594
func testLevelFromEventData() {
559595
let envelopeItem = SentryEnvelopeItem(event: TestData.event)
560596

‎Tests/SentryTests/Protocol/SentryEnvelopeTests.swift‎

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,32 @@ class SentryEnvelopeTests: XCTestCase {
325325
XCTAssertNotNil(
326326
SentryEnvelopeItem(attachment: attachment, maxAttachmentSize: fixture.maxAttachmentSize))
327327
}
328-
328+
329+
func testInitWithReplayEvent_replayRecordingFailsToSerialize_shouldReturnNil() {
330+
// -- Arrange --
331+
class MockReplayRecording: SentryReplayRecording {
332+
override func serialize() -> [[String: Any]] {
333+
// This will cause serialization to fail, because NSObject cannot be serialized to JSON
334+
return [["KEY": NSObject()]]
335+
}
336+
}
337+
338+
let event = SentryReplayEvent(
339+
eventId: SentryId(),
340+
replayStartTimestamp: Date(timeIntervalSince1970: 1_000),
341+
replayType: SentryReplayType.buffer,
342+
segmentId: 5
343+
)
344+
let recording = MockReplayRecording(segmentId: 5, size: 5_000, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 320, width: 950, extraEvents: [])
345+
let videoUrl = URL(fileURLWithPath: fixture.path)
346+
347+
// -- Act --
348+
let result = SentryEnvelopeItem(replayEvent: event, replayRecording: recording, video: videoUrl)
349+
350+
// -- Assert --
351+
XCTAssertNil(result, "Expected nil result when replay recording serialization fails.")
352+
}
353+
329354
private func writeDataToFile(data: Data) {
330355
do {
331356
try data.write(to: URL(fileURLWithPath: fixture.path))

‎Tests/SentryTests/SentryClientTests.swift‎

Lines changed: 44 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2081,21 +2081,59 @@ class SentryClientTest: XCTestCase {
20812081
let replayEvent = SentryReplayEvent(eventId: SentryId(), replayStartTimestamp: Date(), replayType: .session, segmentId: 2)
20822082
replayEvent.sdk = ["name": "Test SDK", "version": "1.0.0"]
20832083
let replayRecording = SentryReplayRecording(segmentId: 2, size: 200, start: Date(timeIntervalSince1970: 2), duration: 5_000, frameCount: 5, frameRate: 1, height: 930, width: 390, extraEvents: [])
2084-
2084+
20852085
//Not a video url, but its ok for test the envelope
20862086
let movieUrl = try XCTUnwrap(Bundle(for: self.classForCoder).url(forResource: "Resources/raw", withExtension: "json"))
2087-
2087+
20882088
let scope = Scope()
20892089
scope.addBreadcrumb(Breadcrumb(level: .debug, category: "Test Breadcrumb"))
2090-
2090+
20912091
sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl, with: scope)
2092-
2092+
20932093
let header = try XCTUnwrap(self.fixture.transport.sentEnvelopes.first?.header)
2094-
2094+
20952095
XCTAssertEqual(header.sdkInfo?.name, "Test SDK")
20962096
XCTAssertEqual(header.sdkInfo?.version, "1.0.0")
20972097
}
2098-
2098+
2099+
func testCaptureReplayEvent_preparingEventFails_shouldNotCaptureAndRecordLostEvent() throws {
2100+
// -- Arrange --
2101+
let sut = fixture.getSut()
2102+
let replayEvent = SentryReplayEvent(eventId: SentryId(), replayStartTimestamp: Date(), replayType: .session, segmentId: 2)
2103+
replayEvent.sdk = ["name": "Test SDK", "version": "1.0.0"]
2104+
let replayRecording = SentryReplayRecording(
2105+
segmentId: 2,
2106+
size: 200,
2107+
start: Date(timeIntervalSince1970: 2),
2108+
duration: 5_000,
2109+
frameCount: 5,
2110+
frameRate: 1,
2111+
height: 930,
2112+
width: 390,
2113+
extraEvents: [
2114+
SentryRRWebEvent(
2115+
type: .custom,
2116+
timestamp: Date(timeIntervalSince1970: 0),
2117+
data: ["KEY": NSObject()] // There is a non-serializable object in the extra events, which will cause the event preparation to fail
2118+
)
2119+
]
2120+
)
2121+
let movieUrl = try XCTUnwrap(URL(string: "https://example.com/movie.mp4"))
2122+
let scope = Scope()
2123+
2124+
// -- Act --
2125+
sut.capture(replayEvent, replayRecording: replayRecording, video: movieUrl, with: scope)
2126+
2127+
// -- Assert --
2128+
XCTAssertEqual(fixture.transport.sentEnvelopes.count, 0)
2129+
2130+
XCTAssertEqual(fixture.transport.recordLostEventsWithCount.count, 1)
2131+
let lostEvent = try XCTUnwrap(fixture.transport.recordLostEventsWithCount.first)
2132+
XCTAssertEqual(lostEvent.category, .replay)
2133+
XCTAssertEqual(lostEvent.reason, SentryDiscardReason.insufficientData)
2134+
XCTAssertEqual(lostEvent.quantity, 1)
2135+
}
2136+
20992137
func testCaptureFatalEventSetReplayInScope() {
21002138
let sut = fixture.getSut()
21012139
let event = Event()

0 commit comments

Comments
 (0)