fix: Crash when initializing SentryHub manually (#3374)

philipphofmann · armcknight · web-flow · commit 313b1d969cac · 2023-11-08T09:29:05.000+01:00
Fix a crash when serializing the trace context after initializing the
SentryHub manually.

Co-authored-by: Andrew McKnight &lt;andrew.mcknight@sentry.io&gt;
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,11 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Crash when initializing SentryHub manually (#3374)
+
 ## 8.15.0
 
 ### Features
diff --git a/Sources/Sentry/SentryClient.m b/Sources/Sentry/SentryClient.m
@@ -371,7 +371,9 @@ - (nullable SentryTraceContext *)getTraceStateWithEvent:(SentryEvent *)event
     }
 
     if (event.error || event.exceptions.count > 0) {
-        return scope.propagationContext.traceContext;
+        return [[SentryTraceContext alloc] initWithTraceId:scope.propagationContext.traceId
+                                                   options:self.options
+                                               userSegment:scope.userObject.segment];
     }
 
     return nil;
diff --git a/Sources/Sentry/SentryNetworkTracker.m b/Sources/Sentry/SentryNetworkTracker.m
@@ -23,6 +23,7 @@
 #import "SentryTraceHeader.h"
 #import "SentryTraceOrigins.h"
 #import "SentryTracer.h"
+#import "SentryUser.h"
 #import <objc/runtime.h>
 @import SentryPrivate;
 
@@ -216,7 +217,12 @@ - (void)urlSessionTaskResume:(NSURLSessionTask *)sessionTask
 - (void)addTraceWithoutTransactionToTask:(NSURLSessionTask *)sessionTask
 {
     SentryPropagationContext *propagationContext = SentrySDK.currentHub.scope.propagationContext;
-    [self addBaggageHeader:[[propagationContext traceContext] toBaggage]
+    SentryTraceContext *traceContext =
+        [[SentryTraceContext alloc] initWithTraceId:propagationContext.traceId
+                                            options:SentrySDK.currentHub.client.options
+                                        userSegment:SentrySDK.currentHub.scope.userObject.segment];
+
+    [self addBaggageHeader:[traceContext toBaggage]
                traceHeader:[propagationContext traceHeader]
                  toRequest:sessionTask];
 }
diff --git a/Sources/Sentry/SentryPropagationContext.h b/Sources/Sentry/SentryPropagationContext.h
@@ -8,8 +8,6 @@ NS_ASSUME_NONNULL_BEGIN
 
 @property (nonatomic, strong) SentryId *traceId;
 @property (nonatomic, strong) SentrySpanId *spanId;
-@property (nonatomic, readonly, nullable) SentryTraceContext *traceContext;
-
 @property (nonatomic, readonly) SentryTraceHeader *traceHeader;
 
 - (NSDictionary<NSString *, NSString *> *)traceContextForEvent;
diff --git a/Sources/Sentry/SentryPropagationContext.m b/Sources/Sentry/SentryPropagationContext.m
@@ -28,20 +28,6 @@ - (SentryTraceHeader *)traceHeader
                                               sampled:kSentrySampleDecisionNo];
 }
 
-- (SentryTraceContext *)traceContext
-{
-    SentryOptions *options = SentrySDK.options;
-    SentryScope *scope = SentrySDK.currentHub.scope;
-    return [[SentryTraceContext alloc] initWithTraceId:self.traceId
-                                             publicKey:options.parsedDsn.url.user
-                                           releaseName:options.releaseName
-                                           environment:options.environment
-                                           transaction:nil
-                                           userSegment:scope.userObject.segment
-                                            sampleRate:nil
-                                               sampled:nil];
-}
-
 - (NSDictionary<NSString *, NSString *> *)traceContextForEvent
 {
     return
diff --git a/Sources/Sentry/SentryTraceContext.m b/Sources/Sentry/SentryTraceContext.m
@@ -83,6 +83,20 @@ - (nullable instancetype)initWithTracer:(SentryTracer *)tracer
                          sampled:sampled];
 }
 
+- (instancetype)initWithTraceId:(SentryId *)traceId
+                        options:(SentryOptions *)options
+                    userSegment:(nullable NSString *)userSegment
+{
+    return [[SentryTraceContext alloc] initWithTraceId:traceId
+                                             publicKey:options.parsedDsn.url.user
+                                           releaseName:options.releaseName
+                                           environment:options.environment
+                                           transaction:nil
+                                           userSegment:userSegment
+                                            sampleRate:nil
+                                               sampled:nil];
+}
+
 - (nullable instancetype)initWithDict:(NSDictionary<NSString *, id> *)dictionary
 {
     SentryId *traceId = [[SentryId alloc] initWithUUIDString:dictionary[@"trace_id"]];
diff --git a/Sources/Sentry/include/SentryTraceContext.h b/Sources/Sentry/include/SentryTraceContext.h
@@ -86,6 +86,17 @@ NS_ASSUME_NONNULL_BEGIN
                                   scope:(nullable SentryScope *)scope
                                 options:(SentryOptions *)options;
 
+/**
+ * Initializes a SentryTraceContext with data from a traceID, options and userSegment.
+ *
+ *  @param traceId The current tracer.
+ *  @param options The current active options.
+ *  @param userSegment You can retrieve this usually from the `scope.userObject.segment`.
+ */
+- (instancetype)initWithTraceId:(SentryId *)traceId
+                        options:(SentryOptions *)options
+                    userSegment:(nullable NSString *)userSegment;
+
 /**
  * Create a SentryBaggage with the information of this SentryTraceContext.
  */
diff --git a/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/Network/SentryNetworkTrackerTests.swift
@@ -590,7 +590,8 @@ class SentryNetworkTrackerTests: XCTestCase {
         sut.urlSessionTaskResume(task)
 
         let expectedTraceHeader = SentrySDK.currentHub().scope.propagationContext.traceHeader.value()
-        let expectedBaggageHeader = SentrySDK.currentHub().scope.propagationContext.traceContext?.toBaggage().toHTTPHeader()
+        let traceContext = SentryTraceContext(trace: SentrySDK.currentHub().scope.propagationContext.traceId, options: self.fixture.options, userSegment: self.fixture.scope.userObject?.segment)
+        let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader()
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", expectedBaggageHeader)
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", expectedTraceHeader)
     }
@@ -601,7 +602,8 @@ class SentryNetworkTrackerTests: XCTestCase {
         sut.urlSessionTaskResume(task)
 
         let expectedTraceHeader = SentrySDK.currentHub().scope.propagationContext.traceHeader.value()
-        let expectedBaggageHeader = SentrySDK.currentHub().scope.propagationContext.traceContext?.toBaggage().toHTTPHeader()
+        let traceContext = SentryTraceContext(trace: SentrySDK.currentHub().scope.propagationContext.traceId, options: self.fixture.options, userSegment: self.fixture.scope.userObject?.segment)
+        let expectedBaggageHeader = traceContext.toBaggage().toHTTPHeader()
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["baggage"] ?? "", expectedBaggageHeader)
         XCTAssertEqual(task.currentRequest?.allHTTPHeaderFields?["sentry-trace"] ?? "", expectedTraceHeader)
     }
diff --git a/Tests/SentryTests/SentryClientTests.swift b/Tests/SentryTests/SentryClientTests.swift
@@ -555,8 +555,9 @@ class SentryClientTest: XCTestCase {
             try assertValidErrorEvent(eventWithSessionArguments.event, error)
             XCTAssertEqual(fixture.session, eventWithSessionArguments.session)
             
-            XCTAssertEqual(eventWithSessionArguments.traceContext?.traceId, 
-                           scope.propagationContext.traceContext?.traceId)
+            let expectedTraceContext = SentryTraceContext(trace: scope.propagationContext.traceId, options: Options(), userSegment: "segment") 
+            XCTAssertEqual(eventWithSessionArguments.traceContext?.traceId,
+                           expectedTraceContext.traceId)
         }
     }
     
diff --git a/Tests/SentryTests/SentryHubTests.swift b/Tests/SentryTests/SentryHubTests.swift
@@ -57,7 +57,7 @@ class SentryHubTests: XCTestCase {
     }
     
     private var fixture: Fixture!
-    private var sut: SentryHub!
+    private lazy var sut = fixture.getSut()
     
     override func setUp() {
         super.setUp()
@@ -67,8 +67,6 @@ class SentryHubTests: XCTestCase {
         fixture.fileManager.deleteAppState()
         fixture.fileManager.deleteTimestampLastInForeground()
         fixture.fileManager.deleteAllEnvelopes()
-        
-        sut = fixture.getSut()
     }
     
     override func tearDown() {
@@ -81,6 +79,17 @@ class SentryHubTests: XCTestCase {
         clearTestState()
     }
     
+    func testCaptureErrorWithRealDSN() {
+        let sentryOption = Options()
+        sentryOption.dsn = "https://6cc9bae94def43cab8444a99e0031c28@o447951.ingest.sentry.io/5428557"
+                
+        let scope = Scope()
+        let sentryHub = SentryHub(client: SentryClient(options: sentryOption), andScope: scope)
+
+        let error = NSError(domain: "Test.CaptureErrorWithRealDSN", code: 12)
+        sentryHub.capture(error: error)
+    }
+    
     func testBeforeBreadcrumbWithoutCallbackStoresBreadcrumb() {
         let hub = fixture.getSut()
         
diff --git a/Tests/SentryTests/Transaction/SentryTraceStateTests.swift b/Tests/SentryTests/Transaction/SentryTraceStateTests.swift
@@ -88,6 +88,40 @@ class SentryTraceContextTests: XCTestCase {
         XCTAssertNil(traceContext)
     }
     
+    func testInitTraceIdOptionsSegment_WithOptionsAndSegment() throws {
+        let options = Options()
+        options.dsn = TestConstants.realDSN
+    
+        let traceId = SentryId()
+        let traceContext = SentryTraceContext(trace: traceId, options: options, userSegment: "segment")
+        
+        XCTAssertEqual(options.parsedDsn?.url.user, traceContext.publicKey)
+        XCTAssertEqual(traceId, traceContext.traceId)
+        XCTAssertEqual(options.releaseName, traceContext.releaseName)
+        XCTAssertEqual(options.environment, traceContext.environment)
+        XCTAssertNil(traceContext.transaction)
+        XCTAssertEqual("segment", traceContext.userSegment)
+        XCTAssertNil(traceContext.sampleRate)
+        XCTAssertNil(traceContext.sampled)
+    }
+    
+    func testInitTraceIdOptionsSegment_WithOptionsOnly() throws {
+        let options = Options()
+        options.dsn = TestConstants.realDSN
+    
+        let traceId = SentryId()
+        let traceContext = SentryTraceContext(trace: traceId, options: options, userSegment: nil)
+        
+        XCTAssertEqual(options.parsedDsn?.url.user, traceContext.publicKey)
+        XCTAssertEqual(traceId, traceContext.traceId)
+        XCTAssertEqual(options.releaseName, traceContext.releaseName)
+        XCTAssertEqual(options.environment, traceContext.environment)
+        XCTAssertNil(traceContext.transaction)
+        XCTAssertNil(traceContext.userSegment)
+        XCTAssertNil(traceContext.sampleRate)
+        XCTAssertNil(traceContext.sampled)
+    }
+    
     func test_toBaggage() {
         let traceContext = SentryTraceContext(
             trace: fixture.traceId,
diff --git a/scripts/no-changes-in-high-risk-files.sh b/scripts/no-changes-in-high-risk-files.sh
@@ -5,7 +5,7 @@ set -euo pipefail
 
 ACTUAL=$(shasum -a 256 ./Sources/Sentry/SentryNSURLSessionTaskSearch.m ./Sources/Sentry/SentryNetworkTracker.m ./Sources/Sentry/SentryUIViewControllerSwizzling.m ./Sources/Sentry/SentryNSDataSwizzling.m ./Sources/Sentry/SentrySubClassFinder.m ./Sources/Sentry/SentryCoreDataSwizzling.m ./Sources/Sentry/SentrySwizzleWrapper.m ./Sources/Sentry/include/SentrySwizzle.h ./Sources/Sentry/SentrySwizzle.m)
 EXPECTED="819d5ca5e3db2ac23c859b14c149b7f0754d3ae88bea1dba92c18f49a81da0e1  ./Sources/Sentry/SentryNSURLSessionTaskSearch.m
-526c44c5f06a1d03fcb7ed813cee8aac520ffe539b911a44d5d8bd1c85e91a0e  ./Sources/Sentry/SentryNetworkTracker.m
+2f9ea6984ff32b53fc03c386b648f4f1bdf8737ce69050d25ce6d1307f8d1672  ./Sources/Sentry/SentryNetworkTracker.m
 132a491c706bdb68b47c2e0a14aeaa5c611664f34156565dbfc874b360d6a742  ./Sources/Sentry/SentryUIViewControllerSwizzling.m
 e95e62ec7363984f20c78643bb7d992a41a740f97e1befb71525ac34caf88b37  ./Sources/Sentry/SentryNSDataSwizzling.m
 cc3849725bd1733515c71742872bed94ca47d2c115ef9d8c98383eae2e171925  ./Sources/Sentry/SentrySubClassFinder.m

Original file line number	Diff line number	Diff line change
`@@ -371,7 +371,9 @@ - (nullable SentryTraceContext )getTraceStateWithEvent:(SentryEvent )event`
`371`	`371`	`}`
`372`	`372`
`373`	`373`	`if (event.error \|\| event.exceptions.count > 0) {`
`374`		`- return scope.propagationContext.traceContext;`
	`374`	`+ return [[SentryTraceContext alloc] initWithTraceId:scope.propagationContext.traceId`
	`375`	`+ options:self.options`
	`376`	`+ userSegment:scope.userObject.segment];`
`375`	`377`	`}`
`376`	`378`
`377`	`379`	`return nil;`
Original file line number	Diff line number	Diff line change
`@@ -555,8 +555,9 @@ class SentryClientTest: XCTestCase {`
`555`	`555`	`try assertValidErrorEvent(eventWithSessionArguments.event, error)`
`556`	`556`	`XCTAssertEqual(fixture.session, eventWithSessionArguments.session)`
`557`	`557`
`558`		`- XCTAssertEqual(eventWithSessionArguments.traceContext?.traceId,`
`559`		`- scope.propagationContext.traceContext?.traceId)`
	`558`	`+ let expectedTraceContext = SentryTraceContext(trace: scope.propagationContext.traceId, options: Options(), userSegment: "segment")`
	`559`	`+ XCTAssertEqual(eventWithSessionArguments.traceContext?.traceId,`
	`560`	`+ expectedTraceContext.traceId)`
`560`	`561`	`}`
`561`	`562`	`}`
`562`	`563`