refactor: Replace calls useSpan with callback to direct span accessor (#4896)

philprime · web-flow · commit 5ea7b5aa0414 · 2025-02-25T15:32:38.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@
 
 - Log message when setting user before starting the SDK (#4882)
 - Add experimental flag to disable swizzling of `NSData` individually (#4859)
+- Replace calls of `SentryScope.useSpan` with callback to direct span accessor (#4896)
 
 ### Fixes
 
diff --git a/Sources/Sentry/Public/SentryDefines.h b/Sources/Sentry/Public/SentryDefines.h
@@ -157,7 +157,8 @@ typedef NSNumber *_Nullable (^SentryTracesSamplerCallback)(
  * Function pointer for span manipulation.
  * @param span The span to be used.
  */
-typedef void (^SentrySpanCallback)(id<SentrySpan> _Nullable span);
+typedef void (^SentrySpanCallback)(id<SentrySpan> _Nullable span DEPRECATED_MSG_ATTRIBUTE(
+    "See `SentryScope.useSpan` for reasoning of deprecation."));
 
 /**
  * Log level.
diff --git a/Sources/Sentry/Public/SentryScope.h b/Sources/Sentry/Public/SentryScope.h
@@ -158,7 +158,16 @@ NS_SWIFT_NAME(Scope)
  * Mutates the current transaction atomically.
  * @param callback the SentrySpanCallback.
  */
-- (void)useSpan:(SentrySpanCallback)callback;
+- (void)useSpan:(SentrySpanCallback)callback
+    DEPRECATED_MSG_ATTRIBUTE(
+        "This method was used to create an atomic block that could be used to mutate the current "
+        "span. It is not atomic anymore and due to issues with memory safety in `NSBlock` it is "
+        "now considered unsafe and deprecated. Use `span` instead.");
+
+/**
+ * Returns the current span.
+ */
+- (id<SentrySpan> _Nullable)span;
 
 @end
 
diff --git a/Sources/Sentry/SentryCoreDataTracker.m b/Sources/Sentry/SentryCoreDataTracker.m
@@ -36,17 +36,20 @@ - (NSArray *)managedObjectContext:(NSManagedObjectContext *)context
                             error:(NSError **)error
                       originalImp:(NSArray *(NS_NOESCAPE ^)(NSFetchRequest *, NSError **))original
 {
-    __block id<SentrySpan> fetchSpan;
-    [SentrySDK.currentHub.scope useSpan:^(id<SentrySpan> _Nullable span) {
-        fetchSpan = [span startChildWithOperation:SentrySpanOperation.coredataFetchOperation
-                                      description:[self descriptionFromRequest:request]];
-        fetchSpan.origin = SentryTraceOrigin.autoDBCoreData;
-    }];
+    id<SentrySpan> _Nullable currentSpan = [SentrySDK.currentHub.scope span];
+    id<SentrySpan> _Nullable fetchSpan;
+    if (currentSpan) {
+        NSString *spanDescription = [self descriptionFromRequest:request];
+        fetchSpan = [currentSpan startChildWithOperation:SentrySpanOperation.coredataFetchOperation
+                                             description:spanDescription];
+    }
 
     if (fetchSpan) {
+        fetchSpan.origin = SentryTraceOrigin.autoDBCoreData;
+
         SENTRY_LOG_DEBUG(@"SentryCoreDataTracker automatically started a new span with "
-                         @"description: %@, operation: %@",
-            fetchSpan.description, fetchSpan.operation);
+                         @"description: %@, operation: %@, origin: %@",
+            fetchSpan.description, fetchSpan.operation, fetchSpan.origin);
     }
 
     NSArray *result = original(request, error);
@@ -70,22 +73,26 @@ - (BOOL)managedObjectContext:(NSManagedObjectContext *)context
                  originalImp:(BOOL(NS_NOESCAPE ^)(NSError **))original
 {
 
-    __block id<SentrySpan> saveSpan = nil;
+    __block id<SentrySpan> _Nullable saveSpan = nil;
     if (context.hasChanges) {
         __block NSDictionary<NSString *, NSDictionary *> *operations =
             [self groupEntitiesOperations:context];
 
-        [SentrySDK.currentHub.scope useSpan:^(id<SentrySpan> _Nullable span) {
-            saveSpan = [span startChildWithOperation:SentrySpanOperation.coredataSaveOperation
-                                         description:[self descriptionForOperations:operations
-                                                                          inContext:context]];
-            saveSpan.origin = SentryTraceOrigin.autoDBCoreData;
-        }];
+        id<SentrySpan> _Nullable currentSpan = [SentrySDK.currentHub.scope span];
+        if (currentSpan) {
+            NSString *spanDescription = [self descriptionForOperations:operations
+                                                             inContext:context];
+            saveSpan =
+                [currentSpan startChildWithOperation:SentrySpanOperation.coredataSaveOperation
+                                         description:spanDescription];
+        }
 
         if (saveSpan) {
+            saveSpan.origin = SentryTraceOrigin.autoDBCoreData;
+
             SENTRY_LOG_DEBUG(@"SentryCoreDataTracker automatically started a new span with "
-                             @"description: %@, operation: %@",
-                saveSpan.description, saveSpan.operation);
+                             @"description: %@, operation: %@, origin: %@",
+                saveSpan.description, saveSpan.operation, saveSpan.origin);
 
             [saveSpan setDataValue:operations forKey:@"operations"];
         } else {
diff --git a/Sources/Sentry/SentryFileIOTracker.m b/Sources/Sentry/SentryFileIOTracker.m
@@ -193,25 +193,27 @@ - (BOOL)measureNSFileManagerCreateFileAtPath:(NSString *)path
         return nil;
     }
 
-    __block id<SentrySpan> ioSpan;
     NSString *spanDescription = [self transactionDescriptionForFile:path fileSize:size];
-    [SentrySDK.currentHub.scope useSpan:^(id<SentrySpan> _Nullable span) {
-        // Keep the logic inside the `useSpan` block to a minimum, as we have noticed memory issues
-        // See: https://github.com/getsentry/sentry-cocoa/issues/4887
-        ioSpan = [span startChildWithOperation:operation description:spanDescription];
-    }];
+    id<SentrySpan> _Nullable currentSpan = [SentrySDK.currentHub.scope span];
+    if (currentSpan == NULL) {
+        SENTRY_LOG_DEBUG(@"No transaction bound to scope. Won't track file IO operation.");
+        return nil;
+    }
 
+    id<SentrySpan> _Nullable ioSpan = [currentSpan startChildWithOperation:operation
+                                                               description:spanDescription];
     if (ioSpan == nil) {
         SENTRY_LOG_DEBUG(@"No transaction bound to scope. Won't track file IO operation.");
         return nil;
     }
 
-    SENTRY_LOG_DEBUG(@"Automatically started a new span with description: %@, operation: %@",
-        ioSpan.description, operation);
-
     ioSpan.origin = origin;
     [ioSpan setDataValue:path forKey:SentrySpanDataKey.filePath];
 
+    SENTRY_LOG_DEBUG(
+        @"Automatically started a new span with description: %@, operation: %@, origin: %@",
+        ioSpan.description, operation, origin);
+
     [self mainThreadExtraInfo:ioSpan];
 
     return ioSpan;
diff --git a/Sources/Sentry/SentryNetworkTracker.m b/Sources/Sentry/SentryNetworkTracker.m
@@ -174,37 +174,36 @@ - (void)urlSessionTaskResume:(NSURLSessionTask *)sessionTask
 
     UrlSanitized *safeUrl = [[UrlSanitized alloc] initWithURL:url];
     @synchronized(sessionTask) {
-        __block id<SentrySpan> span;
-        __block id<SentrySpan> netSpan;
+        __block id<SentrySpan> _Nullable span;
+        __block id<SentrySpan> _Nullable netSpan;
         netSpan = objc_getAssociatedObject(sessionTask, &SENTRY_NETWORK_REQUEST_TRACKER_SPAN);
 
         // The task already has a span. Nothing to do.
         if (netSpan != nil) {
             return;
         }
 
-        [SentrySDK.currentHub.scope useSpan:^(id<SentrySpan> _Nullable innerSpan) {
-            if (innerSpan != nil) {
-                span = innerSpan;
-                netSpan = [span startChildWithOperation:SentrySpanOperation.networkRequestOperation
-                                            description:[NSString stringWithFormat:@"%@ %@",
-                                                            sessionTask.currentRequest.HTTPMethod,
-                                                            safeUrl.sanitizedUrl]];
-                netSpan.origin = SentryTraceOrigin.autoHttpNSURLSession;
-
-                [netSpan setDataValue:sessionTask.currentRequest.HTTPMethod
-                               forKey:@"http.request.method"];
-                [netSpan setDataValue:safeUrl.sanitizedUrl forKey:@"url"];
-                [netSpan setDataValue:@"fetch" forKey:@"type"];
-
-                if (safeUrl.queryItems && safeUrl.queryItems.count > 0) {
-                    [netSpan setDataValue:safeUrl.query forKey:@"http.query"];
-                }
-                if (safeUrl.fragment != nil) {
-                    [netSpan setDataValue:safeUrl.fragment forKey:@"http.fragment"];
-                }
+        id<SentrySpan> _Nullable currentSpan = [SentrySDK.currentHub.scope span];
+        if (currentSpan != nil) {
+            span = currentSpan;
+            netSpan = [span startChildWithOperation:SentrySpanOperation.networkRequestOperation
+                                        description:[NSString stringWithFormat:@"%@ %@",
+                                                        sessionTask.currentRequest.HTTPMethod,
+                                                        safeUrl.sanitizedUrl]];
+            netSpan.origin = SentryTraceOrigin.autoHttpNSURLSession;
+
+            [netSpan setDataValue:sessionTask.currentRequest.HTTPMethod
+                           forKey:@"http.request.method"];
+            [netSpan setDataValue:safeUrl.sanitizedUrl forKey:@"url"];
+            [netSpan setDataValue:@"fetch" forKey:@"type"];
+
+            if (safeUrl.queryItems && safeUrl.queryItems.count > 0) {
+                [netSpan setDataValue:safeUrl.query forKey:@"http.query"];
             }
-        }];
+            if (safeUrl.fragment != nil) {
+                [netSpan setDataValue:safeUrl.fragment forKey:@"http.fragment"];
+            }
+        }
 
         // We only create a span if there is a transaction in the scope,
         // otherwise we have nothing else to do here.
diff --git a/Sources/Sentry/SentryTracer.m b/Sources/Sentry/SentryTracer.m
@@ -635,11 +635,10 @@ - (BOOL)finishTracer:(SentrySpanStatus)unfinishedSpansFinishStatus shouldCleanUp
     }
 
     if (shouldCleanUp) {
-        [_hub.scope useSpan:^(id<SentrySpan> _Nullable span) {
-            if (span == self) {
-                [self->_hub.scope setSpan:nil];
-            }
-        }];
+        id<SentrySpan> _Nullable currentSpan = [_hub.scope span];
+        if (currentSpan == self) {
+            [_hub.scope setSpan:nil];
+        }
     }
 
     if (self.configuration.finishMustBeCalled && !self.wasFinishCalled) {
diff --git a/Sources/Sentry/SentryUIEventTrackerTransactionMode.m b/Sources/Sentry/SentryUIEventTrackerTransactionMode.m
@@ -67,30 +67,31 @@ - (void)handleUIEvent:(NSString *)action
                                              operation:operation
                                                 origin:SentryTraceOrigin.autoUiEventTracker];
 
-    __block SentryTracer *transaction;
-    [SentrySDK.currentHub.scope useSpan:^(id<SentrySpan> _Nullable span) {
-        BOOL ongoingScreenLoadTransaction
-            = span != nil && [span.operation isEqualToString:SentrySpanOperation.uiLoad];
-        BOOL ongoingManualTransaction = span != nil
-            && ![span.operation isEqualToString:SentrySpanOperation.uiLoad]
-            && ![span.operation containsString:SentrySpanOperation.uiAction];
-
-        BOOL bindToScope = !ongoingScreenLoadTransaction && !ongoingManualTransaction;
-
-        transaction = [SentrySDK.currentHub
-            startTransactionWithContext:context
-                            bindToScope:bindToScope
-                  customSamplingContext:@{}
-                          configuration:[SentryTracerConfiguration configurationWithBlock:^(
-                                            SentryTracerConfiguration *config) {
-                              config.idleTimeout = self.idleTimeout;
-                              config.waitForChildren = YES;
-                          }]];
-
-        SENTRY_LOG_DEBUG(@"Automatically started a new transaction with name: "
-                         @"%@, bindToScope: %@",
-            action, bindToScope ? @"YES" : @"NO");
-    }];
+    id<SentrySpan> _Nullable currentSpan = [SentrySDK.currentHub.scope span];
+    BOOL ongoingScreenLoadTransaction = false;
+    BOOL ongoingManualTransaction = false;
+    if (currentSpan != nil) {
+        ongoingScreenLoadTransaction =
+            [currentSpan.operation isEqualToString:SentrySpanOperation.uiLoad];
+        ongoingManualTransaction
+            = ![currentSpan.operation isEqualToString:SentrySpanOperation.uiLoad]
+            && ![currentSpan.operation containsString:SentrySpanOperation.uiAction];
+    }
+    BOOL bindToScope = !ongoingScreenLoadTransaction && !ongoingManualTransaction;
+
+    __block SentryTracer *transaction = [SentrySDK.currentHub
+        startTransactionWithContext:context
+                        bindToScope:bindToScope
+              customSamplingContext:@{}
+                      configuration:[SentryTracerConfiguration configurationWithBlock:^(
+                                        SentryTracerConfiguration *config) {
+                          config.idleTimeout = self.idleTimeout;
+                          config.waitForChildren = YES;
+                      }]];
+
+    SENTRY_LOG_DEBUG(@"Automatically started a new transaction with name: "
+                     @"%@, bindToScope: %@",
+        action, bindToScope ? @"YES" : @"NO");
 
     if (accessibilityIdentifier) {
         [transaction setTagValue:accessibilityIdentifier forKey:@"accessibilityIdentifier"];
diff --git a/Tests/SentryTests/SentryScopeSwiftTests.swift b/Tests/SentryTests/SentryScopeSwiftTests.swift
@@ -271,14 +271,16 @@ class SentryScopeSwiftTests: XCTestCase {
         
         XCTAssertEqual(event.environment, actual?.environment)
     }
-    
+
+    @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")
     func testUseSpan() {
         fixture.scope.span = fixture.transaction
         fixture.scope.useSpan { (span) in
             XCTAssert(span === self.fixture.transaction)
         }
     }
     
+    @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")
     func testUseSpanLock_DoesNotBlock_WithBlockingCallback() {
         let scope = fixture.scope
         scope.span = fixture.transaction
@@ -311,6 +313,7 @@ class SentryScopeSwiftTests: XCTestCase {
         wait(for: [expect], timeout: 0.1)
     }
     
+    @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")
     func testUseSpanLock_IsReentrant() {
         let expect = expectation(description: "finish on time")
         let scope = fixture.scope
@@ -324,6 +327,7 @@ class SentryScopeSwiftTests: XCTestCase {
         wait(for: [expect], timeout: 0.1)
     }
     
+    @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")
     func testSpan_FromMultipleThreads() {
         let scope = fixture.scope
         
@@ -358,6 +362,7 @@ class SentryScopeSwiftTests: XCTestCase {
         XCTAssertNil(serialized["breadcrumbs"])
     }
     
+    @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")
     func testUseSpanForClear() {
         fixture.scope.span = fixture.transaction
         fixture.scope.useSpan { (_) in

Original file line number	Diff line number	Diff line change
`@@ -635,11 +635,10 @@ - (BOOL)finishTracer:(SentrySpanStatus)unfinishedSpansFinishStatus shouldCleanUp`
`635`	`635`	`}`
`636`	`636`
`637`	`637`	`if (shouldCleanUp) {`
`638`		`- [_hub.scope useSpan:^(id<SentrySpan> _Nullable span) {`
`639`		`- if (span == self) {`
`640`		`- [self->_hub.scope setSpan:nil];`
`641`		`- }`
`642`		`- }];`
	`638`	`+ id<SentrySpan> _Nullable currentSpan = [_hub.scope span];`
	`639`	`+ if (currentSpan == self) {`
	`640`	`+ [_hub.scope setSpan:nil];`
	`641`	`+ }`
`643`	`642`	`}`
`644`	`643`
`645`	`644`	`if (self.configuration.finishMustBeCalled && !self.wasFinishCalled) {`
Original file line number	Diff line number	Diff line change
`@@ -271,14 +271,16 @@ class SentryScopeSwiftTests: XCTestCase {`
`271`	`271`
`272`	`272`	`XCTAssertEqual(event.environment, actual?.environment)`
`273`	`273`	`}`
`274`		`-`
	`274`	`+`
	`275`	`+ @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")`
`275`	`276`	`func testUseSpan() {`
`276`	`277`	`fixture.scope.span = fixture.transaction`
`277`	`278`	`fixture.scope.useSpan { (span) in`
`278`	`279`	`XCTAssert(span === self.fixture.transaction)`
`279`	`280`	`}`
`280`	`281`	`}`
`281`	`282`
	`283`	`+ @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")`
`282`	`284`	`func testUseSpanLock_DoesNotBlock_WithBlockingCallback() {`
`283`	`285`	`let scope = fixture.scope`
`284`	`286`	`scope.span = fixture.transaction`
`@@ -311,6 +313,7 @@ class SentryScopeSwiftTests: XCTestCase {`
`311`	`313`	`wait(for: [expect], timeout: 0.1)`
`312`	`314`	`}`
`313`	`315`
	`316`	`+ @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")`
`314`	`317`	`func testUseSpanLock_IsReentrant() {`
`315`	`318`	`let expect = expectation(description: "finish on time")`
`316`	`319`	`let scope = fixture.scope`
`@@ -324,6 +327,7 @@ class SentryScopeSwiftTests: XCTestCase {`
`324`	`327`	`wait(for: [expect], timeout: 0.1)`
`325`	`328`	`}`
`326`	`329`
	`330`	`+ @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")`
`327`	`331`	`func testSpan_FromMultipleThreads() {`
`328`	`332`	`let scope = fixture.scope`
`329`	`333`
`@@ -358,6 +362,7 @@ class SentryScopeSwiftTests: XCTestCase {`
`358`	`362`	`XCTAssertNil(serialized["breadcrumbs"])`
`359`	`363`	`}`
`360`	`364`
	`365`	`+ @available(*, deprecated, message: "The test is marked as deprecated to silence the deprecation warning of useSpan")`
`361`	`366`	`func testUseSpanForClear() {`
`362`	`367`	`fixture.scope.span = fixture.transaction`
`363`	`368`	`fixture.scope.useSpan { (_) in`