From 676b27647506a7d209b8bf146fad94ef680fcf4b Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Tue, 25 Mar 2025 15:22:35 +0100 Subject: [PATCH 01/26] fix(session-replay): change multi-threading of session replay processing --- .../iOS-Swift/SentrySDKWrapper.swift | 2 +- Sentry.xcodeproj/project.pbxproj | 8 + Sources/Sentry/SentryDispatchQueueWrapper.m | 8 + .../Sentry/SentrySessionReplayIntegration.m | 103 +++-- .../include/SentryDispatchQueueWrapper.h | 2 + .../SessionReplay/SentryOnDemandReplay.swift | 372 ++++++++++++------ .../SentryOnDemandReplayError.swift | 6 + .../SessionReplay/SentryReplayFrame.swift | 7 + .../SentryReplayVideoMaker.swift | 2 +- .../SessionReplay/SentrySessionReplay.swift | 112 +++--- 10 files changed, 404 insertions(+), 218 deletions(-) create mode 100644 Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplayError.swift create mode 100644 Sources/Swift/Integrations/SessionReplay/SentryReplayFrame.swift diff --git a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift index 40453c79ad5..26dd4425c9f 100644 --- a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift +++ b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift @@ -19,7 +19,7 @@ struct SentrySDKWrapper { options.debug = true if #available(iOS 16.0, *), enableSessionReplay { - options.sessionReplay = SentryReplayOptions(sessionSampleRate: 0, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true) + options.sessionReplay = SentryReplayOptions(sessionSampleRate: 1.0, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true) options.sessionReplay.quality = .high } diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index 37fc1ee7a0c..813153ffbb5 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -819,6 +819,8 @@ D43B26D62D70964C007747FD /* SentrySpanOperation.m in Sources */ = {isa = PBXBuildFile; fileRef = D43B26D52D709648007747FD /* SentrySpanOperation.m */; }; D43B26D82D70A550007747FD /* SentryTraceOrigin.m in Sources */ = {isa = PBXBuildFile; fileRef = D43B26D72D70A54A007747FD /* SentryTraceOrigin.m */; }; D43B26DA2D70A612007747FD /* SentrySpanDataKey.m in Sources */ = {isa = PBXBuildFile; fileRef = D43B26D92D70A60E007747FD /* SentrySpanDataKey.m */; }; + D451ED5D2D92ECD200C9BEA8 /* SentryOnDemandReplayError.swift in Sources */ = {isa = PBXBuildFile; fileRef = D451ED5C2D92ECD200C9BEA8 /* SentryOnDemandReplayError.swift */; }; + D451ED5F2D92ECDE00C9BEA8 /* SentryReplayFrame.swift in Sources */ = {isa = PBXBuildFile; fileRef = D451ED5E2D92ECDE00C9BEA8 /* SentryReplayFrame.swift */; }; D456B4322D706BDF007068CB /* SentrySpanOperation.h in Headers */ = {isa = PBXBuildFile; fileRef = D456B4312D706BDD007068CB /* SentrySpanOperation.h */; }; D456B4362D706BF2007068CB /* SentryTraceOrigin.h in Headers */ = {isa = PBXBuildFile; fileRef = D456B4352D706BEE007068CB /* SentryTraceOrigin.h */; }; D456B4382D706BFE007068CB /* SentrySpanDataKey.h in Headers */ = {isa = PBXBuildFile; fileRef = D456B4372D706BFB007068CB /* SentrySpanDataKey.h */; }; @@ -1977,6 +1979,8 @@ D43B26D52D709648007747FD /* SentrySpanOperation.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentrySpanOperation.m; sourceTree = ""; }; D43B26D72D70A54A007747FD /* SentryTraceOrigin.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryTraceOrigin.m; sourceTree = ""; }; D43B26D92D70A60E007747FD /* SentrySpanDataKey.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentrySpanDataKey.m; sourceTree = ""; }; + D451ED5C2D92ECD200C9BEA8 /* SentryOnDemandReplayError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryOnDemandReplayError.swift; sourceTree = ""; }; + D451ED5E2D92ECDE00C9BEA8 /* SentryReplayFrame.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryReplayFrame.swift; sourceTree = ""; }; D456B4312D706BDD007068CB /* SentrySpanOperation.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentrySpanOperation.h; path = include/SentrySpanOperation.h; sourceTree = ""; }; D456B4352D706BEE007068CB /* SentryTraceOrigin.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryTraceOrigin.h; path = include/SentryTraceOrigin.h; sourceTree = ""; }; D456B4372D706BFB007068CB /* SentrySpanDataKey.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentrySpanDataKey.h; path = include/SentrySpanDataKey.h; sourceTree = ""; }; @@ -4259,6 +4263,8 @@ D8CAC02A2BA0663E00E38F34 /* SentryReplayOptions.swift */, D8CAC02B2BA0663E00E38F34 /* SentryVideoInfo.swift */, D802994D2BA836EF000F0081 /* SentryOnDemandReplay.swift */, + D451ED5E2D92ECDE00C9BEA8 /* SentryReplayFrame.swift */, + D451ED5C2D92ECD200C9BEA8 /* SentryOnDemandReplayError.swift */, D802994F2BA83A88000F0081 /* SentryPixelBuffer.swift */, D8AFC03C2BDA79BF00118BE1 /* SentryReplayVideoMaker.swift */, D8F67B1A2BE9728600C9197B /* SentrySRDefaultBreadcrumbConverter.swift */, @@ -5028,6 +5034,7 @@ 7B30B67E26527894006B2752 /* SentryDisplayLinkWrapper.m in Sources */, 63FE711D20DA4C1000CDBAE8 /* SentryCrashCPU_arm64.c in Sources */, 844EDC77294144DB00C86F34 /* SentrySystemWrapper.mm in Sources */, + D451ED5F2D92ECDE00C9BEA8 /* SentryReplayFrame.swift in Sources */, D8739CF92BECFFB5007D2F66 /* SentryTransactionNameSource.swift in Sources */, 630435FF1EBCA9D900C4D3FA /* SentryNSURLRequest.m in Sources */, 6281C5722D3E4F12009D0978 /* DecodeArbitraryData.swift in Sources */, @@ -5120,6 +5127,7 @@ D83D079C2B7F9D1C00CC9674 /* SentryMsgPackSerializer.m in Sources */, 7B6D1261265F784000C9BE4B /* PrivateSentrySDKOnly.mm in Sources */, 63BE85711ECEC6DE00DC44F5 /* SentryDateUtils.m in Sources */, + D451ED5D2D92ECD200C9BEA8 /* SentryOnDemandReplayError.swift in Sources */, D4E829D82D75E57900D375AD /* SentryMaskRenderer.swift in Sources */, 7BD4BD4927EB2A5D0071F4FF /* SentryDiscardedEvent.m in Sources */, 628308612D50ADAC00EAEF77 /* SentryRequestCodable.swift in Sources */, diff --git a/Sources/Sentry/SentryDispatchQueueWrapper.m b/Sources/Sentry/SentryDispatchQueueWrapper.m index 2663c12f49e..aa208253a37 100644 --- a/Sources/Sentry/SentryDispatchQueueWrapper.m +++ b/Sources/Sentry/SentryDispatchQueueWrapper.m @@ -105,6 +105,14 @@ - (nullable dispatch_block_t)createDispatchBlock:(void (^)(void))block return dispatch_block_create(0, block); } ++ (SentryDispatchQueueWrapper *)createBackgroundDispatchQueueWithName:(const char *)name + relativePriority:(int)relativePriority + +{ + dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( + DISPATCH_QUEUE_SERIAL, QOS_CLASS_BACKGROUND, relativePriority); + return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; +} @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/SentrySessionReplayIntegration.m b/Sources/Sentry/SentrySessionReplayIntegration.m index 0eb4de59798..9cc198a4226 100644 --- a/Sources/Sentry/SentrySessionReplayIntegration.m +++ b/Sources/Sentry/SentrySessionReplayIntegration.m @@ -57,6 +57,9 @@ @implementation SentrySessionReplayIntegration { // This is the easiest way to ensure segment 0 will always reach the server, because session // replay absolutely needs segment 0 to make replay work. BOOL _rateLimited; + id _dateProvider; + SentryDispatchQueueWrapper *_replayProcessingQueue; + SentryDispatchQueueWrapper *_replayAssetWorkerQueue; } - (instancetype)init @@ -120,6 +123,19 @@ - (void)setupWith:(SentryReplayOptions *)replayOptions } _notificationCenter = SentryDependencyContainer.sharedInstance.notificationCenterWrapper; + _dateProvider = SentryDependencyContainer.sharedInstance.dateProvider; + + // The asset worker queue is used to work on video and frames data. + // Use a relative priority of -1 to make it lower than the default background priority. + _replayAssetWorkerQueue = [SentryDispatchQueueWrapper + createBackgroundDispatchQueueWithName:"io.sentry.session-replay.asset-worker" + relativePriority:-1]; + // The dispatch queue is used to asynchronously wait for the asset worker queue to finish its + // work. To avoid a deadlock, the priority of the processing queue must be lower than the asset + // worker queue. Use a relative priority of -2 to make it lower than the asset worker queue. + _replayProcessingQueue = [SentryDispatchQueueWrapper + createBackgroundDispatchQueueWithName:"io.sentry.session-replay.processing" + relativePriority:-2]; [self moveCurrentReplay]; [self cleanUp]; @@ -189,30 +205,54 @@ - (void)resumePreviousSessionReplay:(SentryEvent *)event } SentryOnDemandReplay *resumeReplayMaker = - [[SentryOnDemandReplay alloc] initWithContentFrom:lastReplayURL.path]; + [[SentryOnDemandReplay alloc] initWithContentFrom:lastReplayURL.path + processingQueue:_replayProcessingQueue + assetWorkerQueue:_replayAssetWorkerQueue + dateProvider:_dateProvider]; resumeReplayMaker.bitRate = _replayOptions.replayBitRate; resumeReplayMaker.videoScale = _replayOptions.sizeScale; NSDate *beginning = hasCrashInfo ? [NSDate dateWithTimeIntervalSinceReferenceDate:crashInfo.lastSegmentEnd] : [resumeReplayMaker oldestFrameDate]; - if (beginning == nil) { return; // no frames to send } - - SentryReplayType _type = type; - int _segmentId = segmentId; - - NSError *error; - NSArray *videos = - [resumeReplayMaker createVideoWithBeginning:beginning - end:[beginning dateByAddingTimeInterval:duration] - error:&error]; + NSDate *end = [beginning dateByAddingTimeInterval:duration]; + + // This method is called from a background thread, so we can synchronize the creation of the + // video with a dispatch group. + __block NSArray *videos; + __block NSError *_Nullable error; + + dispatch_group_t group = dispatch_group_create(); + dispatch_group_enter(group); + [resumeReplayMaker + createVideoAsyncWithBeginning:beginning + end:end + completion:^(NSArray *_Nullable resultVideos, + NSError *_Nullable resultError) { + videos = resultVideos; + error = resultError; + dispatch_group_leave(group); + }]; + // Wait for the video creation to finish without a timeout, because the video creation is + // expected to finish in a reasonable time frame. + dispatch_group_wait(group, DISPATCH_TIME_FOREVER); + + // Either error or videos should be set. + if (error != nil) { + SENTRY_LOG_ERROR(@"Could not create replay video, reason: %@", error); + return; + } if (videos == nil) { - SENTRY_LOG_ERROR(@"Could not create replay video: %@", error); + SENTRY_LOG_ERROR(@"Could not create replay video, reason: no videos available"); return; } + + // For each segment we need to create a new event with the video. + int _segmentId = segmentId; + SentryReplayType _type = type; for (SentryVideoInfo *video in videos) { [self captureVideo:video replayId:replayId segmentId:_segmentId++ type:_type]; // type buffer is only for the first segment @@ -224,8 +264,10 @@ - (void)resumePreviousSessionReplay:(SentryEvent *)event [NSDictionary dictionaryWithObjectsAndKeys:replayId.sentryIdString, @"replay_id", nil]; event.context = eventContext; - if ([NSFileManager.defaultManager removeItemAtURL:lastReplayURL error:&error] == NO) { - SENTRY_LOG_ERROR(@"Can`t delete '%@': %@", SENTRY_LAST_REPLAY, error); + NSError *_Nullable removeError; + BOOL result = [NSFileManager.defaultManager removeItemAtURL:lastReplayURL error:&removeError]; + if (result == NO) { + SENTRY_LOG_ERROR(@"Can`t delete '%@': %@", SENTRY_LAST_REPLAY, removeError); } } @@ -316,30 +358,27 @@ - (void)startWithOptions:(SentryReplayOptions *)replayOptions error:nil]; } - SentryOnDemandReplay *replayMaker = [[SentryOnDemandReplay alloc] initWithOutputPath:docs.path]; + SentryOnDemandReplay *replayMaker = + [[SentryOnDemandReplay alloc] initWithOutputPath:docs.path + processingQueue:_replayProcessingQueue + assetWorkerQueue:_replayAssetWorkerQueue + dateProvider:_dateProvider]; replayMaker.bitRate = replayOptions.replayBitRate; replayMaker.videoScale = replayOptions.sizeScale; replayMaker.cacheMaxSize = (NSInteger)(shouldReplayFullSession ? replayOptions.sessionSegmentDuration + 1 : replayOptions.errorReplayDuration + 1); - dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( - DISPATCH_QUEUE_SERIAL, DISPATCH_QUEUE_PRIORITY_LOW, 0); - SentryDispatchQueueWrapper *dispatchQueue = - [[SentryDispatchQueueWrapper alloc] initWithName:"io.sentry.session-replay" - attributes:attributes]; - - self.sessionReplay = [[SentrySessionReplay alloc] - initWithReplayOptions:replayOptions - replayFolderPath:docs - screenshotProvider:screenshotProvider - replayMaker:replayMaker - breadcrumbConverter:breadcrumbConverter - touchTracker:_touchTracker - dateProvider:SentryDependencyContainer.sharedInstance.dateProvider - delegate:self - dispatchQueue:dispatchQueue - displayLinkWrapper:[[SentryDisplayLinkWrapper alloc] init]]; + SentryDisplayLinkWrapper *displayLinkWrapper = [[SentryDisplayLinkWrapper alloc] init]; + self.sessionReplay = [[SentrySessionReplay alloc] initWithReplayOptions:replayOptions + replayFolderPath:docs + screenshotProvider:screenshotProvider + replayMaker:replayMaker + breadcrumbConverter:breadcrumbConverter + touchTracker:_touchTracker + dateProvider:_dateProvider + delegate:self + displayLinkWrapper:displayLinkWrapper]; [self.sessionReplay startWithRootView:SentryDependencyContainer.sharedInstance.application.windows.firstObject diff --git a/Sources/Sentry/include/SentryDispatchQueueWrapper.h b/Sources/Sentry/include/SentryDispatchQueueWrapper.h index 7859c74ac55..56748389cde 100644 --- a/Sources/Sentry/include/SentryDispatchQueueWrapper.h +++ b/Sources/Sentry/include/SentryDispatchQueueWrapper.h @@ -32,6 +32,8 @@ NS_ASSUME_NONNULL_BEGIN - (nullable dispatch_block_t)createDispatchBlock:(void (^)(void))block; ++ (SentryDispatchQueueWrapper *)createBackgroundDispatchQueueWithName:(const char *)name + relativePriority:(int)relativePriority; @end NS_ASSUME_NONNULL_END diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 6017a98a5f9..5f37dd62343 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -7,27 +7,17 @@ import CoreGraphics import Foundation import UIKit -struct SentryReplayFrame { - let imagePath: String - let time: Date - let screenName: String? -} - -enum SentryOnDemandReplayError: Error { - case cantReadVideoSize - case cantCreatePixelBuffer - case errorRenderingVideo -} - +// swiftlint:disable type_body_length @objcMembers class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { private let _outputPath: String private var _totalFrames = 0 private let dateProvider: SentryCurrentDateProvider - private let workingQueue: SentryDispatchQueueWrapper + private let processingQueue: SentryDispatchQueueWrapper + private let assetWorkerQueue: SentryDispatchQueueWrapper private var _frames = [SentryReplayFrame]() - + #if SENTRY_TEST || SENTRY_TEST_CI || DEBUG //This is exposed only for tests, no need to make it thread safe. var frames: [SentryReplayFrame] { @@ -40,44 +30,57 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { var frameRate = 1 var cacheMaxSize = UInt.max - init(outputPath: String, workingQueue: SentryDispatchQueueWrapper, dateProvider: SentryCurrentDateProvider) { + init( + outputPath: String, + processingQueue: SentryDispatchQueueWrapper, + assetWorkerQueue: SentryDispatchQueueWrapper, + dateProvider: SentryCurrentDateProvider + ) { + assert(processingQueue != assetWorkerQueue, "Processing and asset worker queue must not be the same.") self._outputPath = outputPath self.dateProvider = dateProvider - self.workingQueue = workingQueue + self.processingQueue = processingQueue + self.assetWorkerQueue = assetWorkerQueue } - convenience init(withContentFrom outputPath: String, workingQueue: SentryDispatchQueueWrapper, dateProvider: SentryCurrentDateProvider) { - self.init(outputPath: outputPath, workingQueue: workingQueue, dateProvider: dateProvider) - + convenience init( + withContentFrom outputPath: String, + processingQueue: SentryDispatchQueueWrapper, + assetWorkerQueue: SentryDispatchQueueWrapper, + dateProvider: SentryCurrentDateProvider + ) { + self.init( + outputPath: outputPath, + processingQueue: processingQueue, + assetWorkerQueue: assetWorkerQueue, + dateProvider: dateProvider + ) + loadFrames(fromPath: outputPath) + } + + /// Loads the frames from the given path. + /// + /// - Parameter path: The path to the directory containing the frames. + private func loadFrames(fromPath path: String) { + SentryLog.debug("[Session Replay] Loading frames from path: \(path)") do { - let content = try FileManager.default.contentsOfDirectory(atPath: outputPath) - _frames = content.compactMap { - guard $0.hasSuffix(".png") else { return SentryReplayFrame?.none } - guard let time = Double($0.dropLast(4)) else { return nil } - return SentryReplayFrame(imagePath: "\(outputPath)/\($0)", time: Date(timeIntervalSinceReferenceDate: time), screenName: nil) + let content = try FileManager.default.contentsOfDirectory(atPath: path) + _frames = content.compactMap { frameFilePath in + guard frameFilePath.hasSuffix(".png") else { return SentryReplayFrame?.none } + guard let time = Double(frameFilePath.dropLast(4)) else { return nil } + let timestamp = Date(timeIntervalSinceReferenceDate: time) + return SentryReplayFrame(imagePath: "\(path)/\(frameFilePath)", time: timestamp, screenName: nil) }.sorted { $0.time < $1.time } + SentryLog.debug("[Session Replay] Loaded \(content.count) files into \(_frames.count) frames from path: \(path)") } catch { - SentryLog.debug("Could not list frames from replay: \(error.localizedDescription)") - return + SentryLog.debug("[Session Replay] Could not list frames from replay: \(error.localizedDescription)") } } - - convenience init(outputPath: String) { - self.init(outputPath: outputPath, - workingQueue: SentryDispatchQueueWrapper(name: "io.sentry.onDemandReplay", attributes: nil), - dateProvider: SentryDefaultCurrentDateProvider()) - } - - convenience init(withContentFrom outputPath: String) { - self.init(withContentFrom: outputPath, - workingQueue: SentryDispatchQueueWrapper(name: "io.sentry.onDemandReplay", attributes: nil), - dateProvider: SentryDefaultCurrentDateProvider()) - } - + func addFrameAsync(image: UIImage, forScreen: String?) { - workingQueue.dispatchAsync({ + processingQueue.dispatchAsync { self.addFrame(image: image, forScreen: forScreen) - }) + } } private func addFrame(image: UIImage, forScreen: String?) { @@ -88,7 +91,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { do { try data.write(to: URL(fileURLWithPath: imagePath)) } catch { - SentryLog.debug("Could not save replay frame. Error: \(error)") + SentryLog.debug("[Session Replay] Could not save replay frame. Error: \(error)") return } _frames.append(SentryReplayFrame(imagePath: imagePath, time: date, screenName: forScreen)) @@ -111,135 +114,247 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } func releaseFramesUntil(_ date: Date) { - workingQueue.dispatchAsync ({ + processingQueue.dispatchAsync { + SentryLog.debug("[Session Replay] Releasing frames until date: \(date), current queue: \(self.processingQueue.queue.label)") while let first = self._frames.first, first.time < date { self._frames.removeFirst() - try? FileManager.default.removeItem(at: URL(fileURLWithPath: first.imagePath)) + let fileUrl = URL(fileURLWithPath: first.imagePath) + do { + SentryLog.debug("[Session Replay] Removing frame at url: \(fileUrl.path)") + try FileManager.default.removeItem(at: fileUrl) + SentryLog.debug("[Session Replay] Removed frame at url: \(fileUrl.path)") + } catch { + SentryLog.warning("[Session Replay] Failed to remove frame at: \(fileUrl.path), reason: \(error.localizedDescription), ignoring error") + } } - }) + } } var oldestFrameDate: Date? { return _frames.first?.time } - - func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] { - let videoFrames = filterFrames(beginning: beginning, end: end) - var frameCount = 0 - - var videos = [SentryVideoInfo]() - - while frameCount < videoFrames.count { - let outputFileURL = URL(fileURLWithPath: _outputPath.appending("/\(videoFrames[frameCount].time.timeIntervalSinceReferenceDate).mp4")) - if let videoInfo = try renderVideo(with: videoFrames, from: &frameCount, at: outputFileURL) { - videos.append(videoInfo) - } else { - frameCount++ - } + + func createVideoAsyncWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) { + // Note: In Swift it is best practice to use `Result` instead of `(Value?, Error?)` + // Due to interoperability with Objective-C and @objc, we can not use Result here. + SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") + let videoFrames = getFilteredFrames(beginning: beginning, end: end) + + // Dispatch the video creation to a background queue to avoid blocking the calling queue. + let outputPath = self._outputPath + processingQueue.dispatchAsync { + SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end), current queue: \(self.processingQueue.queue.label)") + + do { + // Use a semaphore to wait for each video segment to finish. + let semaphore = DispatchSemaphore(value: 0) + var currentError: Error? + var frameCount = 0 + var videos = [SentryVideoInfo]() + while frameCount < videoFrames.count { + let outputFileURL = URL(fileURLWithPath: outputPath.appending("/\(videoFrames[frameCount].time.timeIntervalSinceReferenceDate).mp4")) + + self.renderVideo(with: videoFrames, from: &frameCount, at: outputFileURL) { result in + // Do not use `processingQueue` here, since it will be blocked by the semaphore. + switch result { + case .success(let videoInfo): + if let videoInfo = videoInfo { + videos.append(videoInfo) + } + case .failure(let error): + currentError = error + } + semaphore.signal() + } + + // Calling semaphore.wait will block the `processingQueue` until the video rendering completes or a timeout occurs. + // It is imporant that the renderVideo completion block signals the semaphore. + // The queue used by render video must have a higher priority than the processing queue to reduce thread inversion. + // Otherwise, it could lead to queue starvation and a deadlock. + if semaphore.wait(timeout: .now() + 2) == .timedOut { + currentError = SentryOnDemandReplayError.errorRenderingVideo + break + } + + // If there was an error, throw it to exit the loop. + if let error = currentError { + throw error + } + } + completion(videos, nil) + } catch { + SentryLog.debug("[Session Replay] Failed to create video with error: \(error)") + completion(nil, error) + } } - return videos } - - private func renderVideo(with videoFrames: [SentryReplayFrame], from: inout Int, at outputFileURL: URL) throws -> SentryVideoInfo? { - guard from < videoFrames.count, let image = UIImage(contentsOfFile: videoFrames[from].imagePath) else { return nil } + + private func getFilteredFrames(beginning: Date, end: Date) -> [SentryReplayFrame] { + var frames = [SentryReplayFrame]() + // Using dispatch queue as sync mechanism since we need a queue already to generate the video. + processingQueue.dispatchSync { + frames = self._frames.filter { $0.time >= beginning && $0.time <= end } + } + return frames + } + + // swiftlint:disable function_body_length + private func renderVideo(with videoFrames: [SentryReplayFrame], from: inout Int, at outputFileURL: URL, completion: @escaping (Result) -> Void) { + SentryLog.debug("[Session Replay] Rendering video with \(videoFrames.count) frames, from index: \(from), to output url: \(outputFileURL)") + guard from < videoFrames.count, let image = UIImage(contentsOfFile: videoFrames[from].imagePath) else { + SentryLog.debug("[Session Replay] Failed to render video, reason: index out of bounds or can't read image at path: \(videoFrames[from].imagePath)") + return completion(.success(nil)) + } + let videoWidth = image.size.width * CGFloat(videoScale) let videoHeight = image.size.height * CGFloat(videoScale) - - let videoWriter = try AVAssetWriter(url: outputFileURL, fileType: .mp4) + let pixelSize = CGSize(width: videoWidth, height: videoHeight) + + let videoWriter: AVAssetWriter + do { + videoWriter = try AVAssetWriter(url: outputFileURL, fileType: .mp4) + } catch { + SentryLog.debug("[Session Replay] Failed to create video writer, reason: \(error)") + return completion(.failure(error)) + } + + SentryLog.debug("[Session Replay] Creating pixel buffer based video writer input") let videoWriterInput = AVAssetWriterInput(mediaType: .video, outputSettings: createVideoSettings(width: videoWidth, height: videoHeight)) - - guard let currentPixelBuffer = SentryPixelBuffer(size: CGSize(width: videoWidth, height: videoHeight), videoWriterInput: videoWriterInput) - else { throw SentryOnDemandReplayError.cantCreatePixelBuffer } - + guard let currentPixelBuffer = SentryPixelBuffer(size: pixelSize, videoWriterInput: videoWriterInput) else { + SentryLog.debug("[Session Replay] Failed to create pixel buffer, reason: \(SentryOnDemandReplayError.cantCreatePixelBuffer)") + return completion(.failure(SentryOnDemandReplayError.cantCreatePixelBuffer)) + } videoWriter.add(videoWriterInput) + videoWriter.startWriting() videoWriter.startSession(atSourceTime: .zero) - + + // Append frames to the video writer input in a pull-style manner when the input is ready to receive more media data. + // + // Inside the callback: + // 1. We append media data until `isReadyForMoreMediaData` becomes false + // 2. Or until there's no more media data to process (then we mark input as finished) + // 3. If we don't mark the input as finished, the callback will be invoked again + // when the input is ready for more data + // + // By setting the queue to the asset worker queue, we ensure that the callback is invoked on the asset worker queue. + // This is important to avoid a deadlock, as this method is called on the processing queue. var lastImageSize: CGSize = image.size var usedFrames = [SentryReplayFrame]() - let group = DispatchGroup() - - var result: Result? var frameCount = from - - group.enter() - videoWriterInput.requestMediaDataWhenReady(on: workingQueue.queue) { + videoWriterInput.requestMediaDataWhenReady(on: assetWorkerQueue.queue) { [weak self] in + guard let self = self else { + SentryLog.warning("[Session Replay] On-demand replay is deallocated, completing writing session without output video info") + return completion(.success(nil)) + } guard videoWriter.status == .writing else { + SentryLog.warning("[Session Replay] Video writer is not writing anymore, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") videoWriter.cancelWriting() - result = .failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo ) - group.leave() - return + return completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) } - if frameCount >= videoFrames.count { - result = self.finishVideo(outputFileURL: outputFileURL, usedFrames: usedFrames, videoHeight: Int(videoHeight), videoWidth: Int(videoWidth), videoWriter: videoWriter) - group.leave() - return + guard frameCount < videoFrames.count else { + SentryLog.debug("[Session Replay] No more frames available to process, finishing the video") + return self.finishVideo(outputFileURL: outputFileURL, usedFrames: usedFrames, videoHeight: Int(videoHeight), videoWidth: Int(videoWidth), videoWriter: videoWriter, onCompletion: completion) } + let frame = videoFrames[frameCount] if let image = UIImage(contentsOfFile: frame.imagePath) { - if lastImageSize != image.size { - result = self.finishVideo(outputFileURL: outputFileURL, usedFrames: usedFrames, videoHeight: Int(videoHeight), videoWidth: Int(videoWidth), videoWriter: videoWriter) - group.leave() - return + guard lastImageSize == image.size else { + SentryLog.debug("[Session Replay] Image size changed, finishing the video") + return self.finishVideo(outputFileURL: outputFileURL, usedFrames: usedFrames, videoHeight: Int(videoHeight), videoWidth: Int(videoWidth), videoWriter: videoWriter, onCompletion: completion) } lastImageSize = image.size - + let presentTime = CMTime(seconds: Double(frameCount), preferredTimescale: CMTimeScale(1 / self.frameRate)) - if currentPixelBuffer.append(image: image, presentationTime: presentTime) != true { + guard currentPixelBuffer.append(image: image, presentationTime: presentTime) == true else { + SentryLog.debug("[Session Replay] Failed to append image to pixel buffer, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") videoWriter.cancelWriting() - result = .failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo ) - group.leave() - return + return completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) } usedFrames.append(frame) } frameCount += 1 } - guard group.wait(timeout: .now() + 2) == .success else { throw SentryOnDemandReplayError.errorRenderingVideo } - from = frameCount - - return try result?.get() } - - private func finishVideo(outputFileURL: URL, usedFrames: [SentryReplayFrame], videoHeight: Int, videoWidth: Int, videoWriter: AVAssetWriter) -> Result { - let group = DispatchGroup() - var finishError: Error? - var result: SentryVideoInfo? - - group.enter() + // swiftlint:enable function_body_length + + private func finishVideo( + outputFileURL: URL, + usedFrames: [SentryReplayFrame], + videoHeight: Int, + videoWidth: Int, + videoWriter: AVAssetWriter, + onCompletion completion: @escaping (Result) -> Void + ) { + // Note: This method is expected to be called from the asset worker queue and *not* the processing queue. + SentryLog.debug("[Session Replay] Finishing video with output file URL: \(outputFileURL.path)") videoWriter.inputs.forEach { $0.markAsFinished() } - videoWriter.finishWriting { - defer { group.leave() } - if videoWriter.status == .completed { + videoWriter.finishWriting { [weak self] in + SentryLog.debug("[Session Replay] Finished video writing, status: \(videoWriter.status)") + guard let strongSelf = self else { + SentryLog.warning("[Session Replay] On-demand replay is deallocated, completing writing session without output video info") + return completion(.success(nil)) + } + + switch videoWriter.status { + case .writing: + // noop + break + case .cancelled: + SentryLog.debug("[Session Replay] Finish writing video was cancelled, completing with no video info") + completion(.success(nil)) + case .completed: + SentryLog.debug("[Session Replay] Finish writing video was completed, creating video info from file attributes") do { - let fileAttributes = try FileManager.default.attributesOfItem(atPath: outputFileURL.path) - guard let fileSize = fileAttributes[FileAttributeKey.size] as? Int else { - finishError = SentryOnDemandReplayError.cantReadVideoSize - return - } - guard let start = usedFrames.min(by: { $0.time < $1.time })?.time else { return } - let duration = TimeInterval(usedFrames.count / self.frameRate) - result = SentryVideoInfo(path: outputFileURL, height: Int(videoHeight), width: Int(videoWidth), duration: duration, frameCount: usedFrames.count, frameRate: self.frameRate, start: start, end: start.addingTimeInterval(duration), fileSize: fileSize, screens: usedFrames.compactMap({ $0.screenName })) + let result = try strongSelf.getVideoInfo( + from: outputFileURL, + usedFrames: usedFrames, + videoWidth: Int(videoWidth), + videoHeight: Int(videoHeight) + ) + completion(.success(result)) } catch { - finishError = error + SentryLog.warning("[Session Replay] Failed to create video info from file attributes, reason: \(error.localizedDescription)") + completion(.failure(error)) } + case .failed: + SentryLog.warning("[Session Replay] Finish writing video failed, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) + case .unknown: + SentryLog.warning("[Session Replay] Finish writing video with unknown status, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) + @unknown default: + SentryLog.warning("[Session Replay] Finish writing video failed, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + completion(.failure(SentryOnDemandReplayError.errorRenderingVideo)) } } - group.wait() - - if let finishError = finishError { return .failure(finishError) } - return .success(result) } - - private func filterFrames(beginning: Date, end: Date) -> [SentryReplayFrame] { - var frames = [SentryReplayFrame]() - //Using dispatch queue as sync mechanism since we need a queue already to generate the video. - workingQueue.dispatchSync({ - frames = self._frames.filter { $0.time >= beginning && $0.time <= end } - }) - return frames + + fileprivate func getVideoInfo(from outputFileURL: URL, usedFrames: [SentryReplayFrame], videoWidth: Int, videoHeight: Int) throws -> SentryVideoInfo { + let fileAttributes = try FileManager.default.attributesOfItem(atPath: outputFileURL.path) + guard let fileSize = fileAttributes[FileAttributeKey.size] as? Int else { + SentryLog.warning("[Session Replay] Failed to read video size from video file, reason: size attribute not found") + throw SentryOnDemandReplayError.cantReadVideoSize + } + guard let start = usedFrames.min(by: { $0.time < $1.time })?.time else { + SentryLog.warning("[Session Replay] Failed to read video start time from used frames, reason: no frames found") + throw SentryOnDemandReplayError.cantReadVideoStartTime + } + let duration = TimeInterval(usedFrames.count / self.frameRate) + return SentryVideoInfo( + path: outputFileURL, + height: videoHeight, + width: videoWidth, + duration: duration, + frameCount: usedFrames.count, + frameRate: self.frameRate, + start: start, + end: start.addingTimeInterval(duration), + fileSize: fileSize, + screens: usedFrames.compactMap({ $0.screenName }) + ) } - + private func createVideoSettings(width: CGFloat, height: CGFloat) -> [String: Any] { return [ AVVideoCodecKey: AVVideoCodecType.h264, @@ -252,6 +367,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { ] } } +// swiftlint:enable type_body_length #endif // os(iOS) || os(tvOS) #endif // canImport(UIKit) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplayError.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplayError.swift new file mode 100644 index 00000000000..7b060ad97d1 --- /dev/null +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplayError.swift @@ -0,0 +1,6 @@ +enum SentryOnDemandReplayError: Error { + case cantReadVideoSize + case cantCreatePixelBuffer + case errorRenderingVideo + case cantReadVideoStartTime +} diff --git a/Sources/Swift/Integrations/SessionReplay/SentryReplayFrame.swift b/Sources/Swift/Integrations/SessionReplay/SentryReplayFrame.swift new file mode 100644 index 00000000000..0497b784f61 --- /dev/null +++ b/Sources/Swift/Integrations/SessionReplay/SentryReplayFrame.swift @@ -0,0 +1,7 @@ +import Foundation + +struct SentryReplayFrame { + let imagePath: String + let time: Date + let screenName: String? +} diff --git a/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift b/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift index 32831f38642..9ea18313eea 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift @@ -6,7 +6,7 @@ import UIKit protocol SentryReplayVideoMaker: NSObjectProtocol { func addFrameAsync(image: UIImage, forScreen: String?) func releaseFramesUntil(_ date: Date) - func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] + func createVideoAsyncWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) } extension SentryReplayVideoMaker { diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift index 49e856871ab..ae57daf7bad 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift @@ -21,7 +21,7 @@ protocol SentrySessionReplayDelegate: NSObjectProtocol { class SentrySessionReplay: NSObject { private(set) var isFullSession = false private(set) var sessionReplayId: SentryId? - + private var urlToCache: URL? private var rootView: UIView? private var lastScreenShot: Date? @@ -39,7 +39,6 @@ class SentrySessionReplay: NSObject { private let displayLink: SentryDisplayLinkWrapper private let dateProvider: SentryCurrentDateProvider private let touchTracker: SentryTouchTracker? - private let dispatchQueue: SentryDispatchQueueWrapper private let lock = NSLock() var replayTags: [String: Any]? @@ -50,18 +49,17 @@ class SentrySessionReplay: NSObject { var screenshotProvider: SentryViewScreenshotProvider var breadcrumbConverter: SentryReplayBreadcrumbConverter - init(replayOptions: SentryReplayOptions, - replayFolderPath: URL, - screenshotProvider: SentryViewScreenshotProvider, - replayMaker: SentryReplayVideoMaker, - breadcrumbConverter: SentryReplayBreadcrumbConverter, - touchTracker: SentryTouchTracker?, - dateProvider: SentryCurrentDateProvider, - delegate: SentrySessionReplayDelegate, - dispatchQueue: SentryDispatchQueueWrapper, - displayLinkWrapper: SentryDisplayLinkWrapper) { - - self.dispatchQueue = dispatchQueue + init( + replayOptions: SentryReplayOptions, + replayFolderPath: URL, + screenshotProvider: SentryViewScreenshotProvider, + replayMaker: SentryReplayVideoMaker, + breadcrumbConverter: SentryReplayBreadcrumbConverter, + touchTracker: SentryTouchTracker?, + dateProvider: SentryCurrentDateProvider, + delegate: SentrySessionReplayDelegate, + displayLinkWrapper: SentryDisplayLinkWrapper + ) { self.replayOptions = replayOptions self.dateProvider = dateProvider self.delegate = delegate @@ -74,7 +72,7 @@ class SentrySessionReplay: NSObject { } deinit { displayLink.invalidate() } - + func start(rootView: UIView, fullSession: Bool) { guard !isRunning else { return } displayLink.link(withTarget: self, selector: #selector(newFrame(_:))) @@ -84,19 +82,19 @@ class SentrySessionReplay: NSObject { currentSegmentId = 0 sessionReplayId = SentryId() imageCollection = [] - + if fullSession { startFullReplay() } } - + private func startFullReplay() { sessionStart = lastScreenShot isFullSession = true guard let sessionReplayId = sessionReplayId else { return } delegate?.sessionReplayStarted(replayId: sessionReplayId) } - + func pauseSessionMode() { lock.lock() defer { lock.unlock() } @@ -115,7 +113,7 @@ class SentrySessionReplay: NSObject { } isSessionPaused = false } - + func resume() { lock.lock() defer { lock.unlock() } @@ -131,57 +129,57 @@ class SentrySessionReplay: NSObject { videoSegmentStart = nil displayLink.link(withTarget: self, selector: #selector(newFrame(_:))) } - + func captureReplayFor(event: Event) { guard isRunning else { return } - + if isFullSession { setEventContext(event: event) return } - + guard (event.error != nil || event.exceptions?.isEmpty == false) - && captureReplay() else { return } + && captureReplay() else { return } setEventContext(event: event) } - + @discardableResult func captureReplay() -> Bool { guard isRunning else { return false } guard !isFullSession else { return true } - + guard delegate?.sessionReplayShouldCaptureReplayForError() == true else { return false } - + startFullReplay() let replayStart = dateProvider.date().addingTimeInterval(-replayOptions.errorReplayDuration - (Double(replayOptions.frameRate) / 2.0)) - - createAndCapture(startedAt: replayStart, replayType: .buffer) + + createAndCaptureAsync(startedAt: replayStart, replayType: .buffer) return true } - + private func setEventContext(event: Event) { guard let sessionReplayId = sessionReplayId, event.type != "replay_video" else { return } - + var context = event.context ?? [:] context["replay"] = ["replay_id": sessionReplayId.sentryIdString] event.context = context - + var tags = ["replayId": sessionReplayId.sentryIdString] if let eventTags = event.tags { tags.merge(eventTags) { (_, new) in new } } event.tags = tags } - + @objc private func newFrame(_ sender: CADisplayLink) { guard let lastScreenShot = lastScreenShot, isRunning && !(isFullSession && isSessionPaused) //If replay is in session mode but it is paused we dont take screenshots else { return } - + let now = dateProvider.date() if let sessionStart = sessionStart, isFullSession && now.timeIntervalSince(sessionStart) > replayOptions.maximumDuration { @@ -189,7 +187,7 @@ class SentrySessionReplay: NSObject { pause() return } - + if now.timeIntervalSince(lastScreenShot) >= Double(1 / replayOptions.frameRate) { takeScreenshot() self.lastScreenShot = now @@ -202,11 +200,11 @@ class SentrySessionReplay: NSObject { } } } - + private func prepareSegmentUntil(date: Date) { guard var pathToSegment = urlToCache?.appendingPathComponent("segments") else { return } let fileManager = FileManager.default - + if !fileManager.fileExists(atPath: pathToSegment.path) { do { try fileManager.createDirectory(atPath: pathToSegment.path, withIntermediateDirectories: true, attributes: nil) @@ -215,30 +213,32 @@ class SentrySessionReplay: NSObject { return } } - + pathToSegment = pathToSegment.appendingPathComponent("\(currentSegmentId).mp4") let segmentStart = videoSegmentStart ?? dateProvider.date().addingTimeInterval(-replayOptions.sessionSegmentDuration) - - createAndCapture(startedAt: segmentStart, replayType: .session) + + createAndCaptureAsync(startedAt: segmentStart, replayType: .session) } - - private func createAndCapture(startedAt: Date, replayType: SentryReplayType) { - //Creating a video is heavy and blocks the thread - //Since this function is always called in the main thread - //we dispatch it to a background thread. - dispatchQueue.dispatchAsync { - do { - let videos = try self.replayMaker.createVideoWith(beginning: startedAt, end: self.dateProvider.date()) + + private func createAndCaptureAsync(startedAt: Date, replayType: SentryReplayType) { + SentryLog.debug("[Session Replay] Creating replay video started at date: \(startedAt), replayType: \(replayType)") + // Creating a video is computationally expensive, therefore perform it on a background queue. + self.replayMaker.createVideoAsyncWith(beginning: startedAt, end: self.dateProvider.date()) { videos, error in + if let error = error { + SentryLog.debug("[Session Replay] Could not create replay video - \(error.localizedDescription)") + } else if let videos = videos { + SentryLog.debug("[Session Replay] Created replay video with \(videos.count) segments") for video in videos { self.newSegmentAvailable(videoInfo: video, replayType: replayType) } - } catch { - SentryLog.debug("Could not create replay video - \(error.localizedDescription)") + } else { + SentryLog.debug("[Session Replay] Finished replay video creation without any segments") } } } - + private func newSegmentAvailable(videoInfo: SentryVideoInfo, replayType: SentryReplayType) { + SentryLog.debug("[Session Replay] New segment available for replayType: \(replayType), videoInfo: \(videoInfo)") guard let sessionReplayId = sessionReplayId else { return } captureSegment(segment: currentSegmentId, video: videoInfo, replayId: sessionReplayId, replayType: replayType) replayMaker.releaseFramesUntil(videoInfo.end) @@ -254,7 +254,7 @@ class SentrySessionReplay: NSObject { replayEvent.urls = video.screens let breadcrumbs = delegate?.breadcrumbsForSessionReplay() ?? [] - + var events = convertBreadcrumbs(breadcrumbs: breadcrumbs, from: video.start, until: video.end) if let touchTracker = touchTracker { events.append(contentsOf: touchTracker.replayEvents(from: videoSegmentStart ?? video.start, until: video.end)) @@ -270,9 +270,9 @@ class SentrySessionReplay: NSObject { } let recording = SentryReplayRecording(segmentId: segment, video: video, extraEvents: events) - + delegate?.sessionReplayNewSegment(replayEvent: replayEvent, replayRecording: recording, videoUrl: video.path) - + do { try FileManager.default.removeItem(at: video.path) } catch { @@ -302,7 +302,7 @@ class SentrySessionReplay: NSObject { private func takeScreenshot() { guard let rootView = rootView, !processingScreenshot else { return } - + lock.lock() guard !processingScreenshot else { lock.unlock() @@ -310,14 +310,14 @@ class SentrySessionReplay: NSObject { } processingScreenshot = true lock.unlock() - + let screenName = delegate?.currentScreenNameForSessionReplay() screenshotProvider.image(view: rootView) { [weak self] screenshot in self?.newImage(image: screenshot, forScreen: screenName) } } - + private func newImage(image: UIImage, forScreen screen: String?) { lock.synchronized { processingScreenshot = false From 2fda9c4f94479d4929451fac7ba36ec538c0e156 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Tue, 25 Mar 2025 15:36:07 +0100 Subject: [PATCH 02/26] small fixes --- .../iOS-Swift/SentrySDKWrapper.swift | 2 +- .../SessionReplay/SentrySessionReplay.swift | 73 ++++++++++--------- 2 files changed, 38 insertions(+), 37 deletions(-) diff --git a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift index 26dd4425c9f..40453c79ad5 100644 --- a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift +++ b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift @@ -19,7 +19,7 @@ struct SentrySDKWrapper { options.debug = true if #available(iOS 16.0, *), enableSessionReplay { - options.sessionReplay = SentryReplayOptions(sessionSampleRate: 1.0, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true) + options.sessionReplay = SentryReplayOptions(sessionSampleRate: 0, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true) options.sessionReplay.quality = .high } diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift index ae57daf7bad..363def8ed1c 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift @@ -21,7 +21,7 @@ protocol SentrySessionReplayDelegate: NSObjectProtocol { class SentrySessionReplay: NSObject { private(set) var isFullSession = false private(set) var sessionReplayId: SentryId? - + private var urlToCache: URL? private var rootView: UIView? private var lastScreenShot: Date? @@ -72,7 +72,7 @@ class SentrySessionReplay: NSObject { } deinit { displayLink.invalidate() } - + func start(rootView: UIView, fullSession: Bool) { guard !isRunning else { return } displayLink.link(withTarget: self, selector: #selector(newFrame(_:))) @@ -82,19 +82,19 @@ class SentrySessionReplay: NSObject { currentSegmentId = 0 sessionReplayId = SentryId() imageCollection = [] - + if fullSession { startFullReplay() } } - + private func startFullReplay() { sessionStart = lastScreenShot isFullSession = true guard let sessionReplayId = sessionReplayId else { return } delegate?.sessionReplayStarted(replayId: sessionReplayId) } - + func pauseSessionMode() { lock.lock() defer { lock.unlock() } @@ -113,7 +113,7 @@ class SentrySessionReplay: NSObject { } isSessionPaused = false } - + func resume() { lock.lock() defer { lock.unlock() } @@ -129,57 +129,57 @@ class SentrySessionReplay: NSObject { videoSegmentStart = nil displayLink.link(withTarget: self, selector: #selector(newFrame(_:))) } - + func captureReplayFor(event: Event) { guard isRunning else { return } - + if isFullSession { setEventContext(event: event) return } - + guard (event.error != nil || event.exceptions?.isEmpty == false) && captureReplay() else { return } setEventContext(event: event) } - + @discardableResult func captureReplay() -> Bool { guard isRunning else { return false } guard !isFullSession else { return true } - + guard delegate?.sessionReplayShouldCaptureReplayForError() == true else { return false } - + startFullReplay() let replayStart = dateProvider.date().addingTimeInterval(-replayOptions.errorReplayDuration - (Double(replayOptions.frameRate) / 2.0)) - + createAndCaptureAsync(startedAt: replayStart, replayType: .buffer) return true } - + private func setEventContext(event: Event) { guard let sessionReplayId = sessionReplayId, event.type != "replay_video" else { return } - + var context = event.context ?? [:] context["replay"] = ["replay_id": sessionReplayId.sentryIdString] event.context = context - + var tags = ["replayId": sessionReplayId.sentryIdString] if let eventTags = event.tags { tags.merge(eventTags) { (_, new) in new } } event.tags = tags } - + @objc private func newFrame(_ sender: CADisplayLink) { guard let lastScreenShot = lastScreenShot, isRunning && !(isFullSession && isSessionPaused) //If replay is in session mode but it is paused we dont take screenshots else { return } - + let now = dateProvider.date() if let sessionStart = sessionStart, isFullSession && now.timeIntervalSince(sessionStart) > replayOptions.maximumDuration { @@ -187,7 +187,7 @@ class SentrySessionReplay: NSObject { pause() return } - + if now.timeIntervalSince(lastScreenShot) >= Double(1 / replayOptions.frameRate) { takeScreenshot() self.lastScreenShot = now @@ -200,11 +200,11 @@ class SentrySessionReplay: NSObject { } } } - + private func prepareSegmentUntil(date: Date) { guard var pathToSegment = urlToCache?.appendingPathComponent("segments") else { return } let fileManager = FileManager.default - + if !fileManager.fileExists(atPath: pathToSegment.path) { do { try fileManager.createDirectory(atPath: pathToSegment.path, withIntermediateDirectories: true, attributes: nil) @@ -213,30 +213,31 @@ class SentrySessionReplay: NSObject { return } } - + pathToSegment = pathToSegment.appendingPathComponent("\(currentSegmentId).mp4") let segmentStart = videoSegmentStart ?? dateProvider.date().addingTimeInterval(-replayOptions.sessionSegmentDuration) - + createAndCaptureAsync(startedAt: segmentStart, replayType: .session) } - + private func createAndCaptureAsync(startedAt: Date, replayType: SentryReplayType) { SentryLog.debug("[Session Replay] Creating replay video started at date: \(startedAt), replayType: \(replayType)") // Creating a video is computationally expensive, therefore perform it on a background queue. self.replayMaker.createVideoAsyncWith(beginning: startedAt, end: self.dateProvider.date()) { videos, error in if let error = error { SentryLog.debug("[Session Replay] Could not create replay video - \(error.localizedDescription)") - } else if let videos = videos { - SentryLog.debug("[Session Replay] Created replay video with \(videos.count) segments") - for video in videos { - self.newSegmentAvailable(videoInfo: video, replayType: replayType) - } - } else { + } + guard let videos = videos else { SentryLog.debug("[Session Replay] Finished replay video creation without any segments") + return + } + SentryLog.debug("[Session Replay] Created replay video with \(videos.count) segments") + for video in videos { + self.newSegmentAvailable(videoInfo: video, replayType: replayType) } } } - + private func newSegmentAvailable(videoInfo: SentryVideoInfo, replayType: SentryReplayType) { SentryLog.debug("[Session Replay] New segment available for replayType: \(replayType), videoInfo: \(videoInfo)") guard let sessionReplayId = sessionReplayId else { return } @@ -254,7 +255,7 @@ class SentrySessionReplay: NSObject { replayEvent.urls = video.screens let breadcrumbs = delegate?.breadcrumbsForSessionReplay() ?? [] - + var events = convertBreadcrumbs(breadcrumbs: breadcrumbs, from: video.start, until: video.end) if let touchTracker = touchTracker { events.append(contentsOf: touchTracker.replayEvents(from: videoSegmentStart ?? video.start, until: video.end)) @@ -270,9 +271,9 @@ class SentrySessionReplay: NSObject { } let recording = SentryReplayRecording(segmentId: segment, video: video, extraEvents: events) - + delegate?.sessionReplayNewSegment(replayEvent: replayEvent, replayRecording: recording, videoUrl: video.path) - + do { try FileManager.default.removeItem(at: video.path) } catch { @@ -302,7 +303,7 @@ class SentrySessionReplay: NSObject { private func takeScreenshot() { guard let rootView = rootView, !processingScreenshot else { return } - + lock.lock() guard !processingScreenshot else { lock.unlock() @@ -317,7 +318,7 @@ class SentrySessionReplay: NSObject { self?.newImage(image: screenshot, forScreen: screenName) } } - + private func newImage(image: UIImage, forScreen screen: String?) { lock.synchronized { processingScreenshot = false From aa2691583f9cc30f7a47711210dbed8968337dd1 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Tue, 25 Mar 2025 15:38:13 +0100 Subject: [PATCH 03/26] improvements --- .../SessionReplay/SentrySessionReplay.swift | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift index 363def8ed1c..a46fe461a32 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift @@ -138,8 +138,9 @@ class SentrySessionReplay: NSObject { return } - guard (event.error != nil || event.exceptions?.isEmpty == false) - && captureReplay() else { return } + guard (event.error != nil || event.exceptions?.isEmpty == false) && captureReplay() else { + return + } setEventContext(event: event) } @@ -225,16 +226,17 @@ class SentrySessionReplay: NSObject { // Creating a video is computationally expensive, therefore perform it on a background queue. self.replayMaker.createVideoAsyncWith(beginning: startedAt, end: self.dateProvider.date()) { videos, error in if let error = error { - SentryLog.debug("[Session Replay] Could not create replay video - \(error.localizedDescription)") + SentryLog.error("[Session Replay] Could not create replay video - \(error.localizedDescription)") } guard let videos = videos else { - SentryLog.debug("[Session Replay] Finished replay video creation without any segments") + SentryLog.warning("[Session Replay] Finished replay video creation without any segments") return } SentryLog.debug("[Session Replay] Created replay video with \(videos.count) segments") for video in videos { self.newSegmentAvailable(videoInfo: video, replayType: replayType) } + SentryLog.debug("[Session Replay] Finished replay video creation with \(videos.count) segments") } } From a27d74d4f8cd7bcf0c136b4d242ef0e0faeae638 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Tue, 25 Mar 2025 15:42:35 +0100 Subject: [PATCH 04/26] WIP --- .../SessionReplay/SentryOnDemandReplay.swift | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 5f37dd62343..fc8841e9270 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -137,14 +137,14 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { func createVideoAsyncWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) { // Note: In Swift it is best practice to use `Result` instead of `(Value?, Error?)` // Due to interoperability with Objective-C and @objc, we can not use Result here. - SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") - let videoFrames = getFilteredFrames(beginning: beginning, end: end) + SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") // Dispatch the video creation to a background queue to avoid blocking the calling queue. - let outputPath = self._outputPath processingQueue.dispatchAsync { SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end), current queue: \(self.processingQueue.queue.label)") + let videoFrames = self._frames.filter { $0.time >= beginning && $0.time <= end } + do { // Use a semaphore to wait for each video segment to finish. let semaphore = DispatchSemaphore(value: 0) @@ -152,7 +152,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { var frameCount = 0 var videos = [SentryVideoInfo]() while frameCount < videoFrames.count { - let outputFileURL = URL(fileURLWithPath: outputPath.appending("/\(videoFrames[frameCount].time.timeIntervalSinceReferenceDate).mp4")) + let outputFileURL = URL(fileURLWithPath: self._outputPath.appending("/\(videoFrames[frameCount].time.timeIntervalSinceReferenceDate).mp4")) self.renderVideo(with: videoFrames, from: &frameCount, at: outputFileURL) { result in // Do not use `processingQueue` here, since it will be blocked by the semaphore. @@ -189,15 +189,6 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } } - private func getFilteredFrames(beginning: Date, end: Date) -> [SentryReplayFrame] { - var frames = [SentryReplayFrame]() - // Using dispatch queue as sync mechanism since we need a queue already to generate the video. - processingQueue.dispatchSync { - frames = self._frames.filter { $0.time >= beginning && $0.time <= end } - } - return frames - } - // swiftlint:disable function_body_length private func renderVideo(with videoFrames: [SentryReplayFrame], from: inout Int, at outputFileURL: URL, completion: @escaping (Result) -> Void) { SentryLog.debug("[Session Replay] Rendering video with \(videoFrames.count) frames, from index: \(from), to output url: \(outputFileURL)") From 86341269caa65efb2360f7899dcda7186a8ff6e3 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 11 Apr 2025 12:41:06 +0200 Subject: [PATCH 05/26] wip --- .../iOS-Swift/SentrySDKWrapper.swift | 7 +- Sentry.xcodeproj/project.pbxproj | 4 + .../SessionReplay/SentryOnDemandReplay.swift | 95 +++++++++++++------ .../SentryRenderVideoResult.swift | 4 + .../SentrySessionReplayTests.swift | 8 +- 5 files changed, 88 insertions(+), 30 deletions(-) create mode 100644 Sources/Swift/Integrations/SessionReplay/SentryRenderVideoResult.swift diff --git a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift index 1b7590fbef3..cf1119685d0 100644 --- a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift +++ b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift @@ -19,7 +19,12 @@ struct SentrySDKWrapper { options.debug = true if #available(iOS 16.0, *), enableSessionReplay { - options.sessionReplay = SentryReplayOptions(sessionSampleRate: 0, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true) + options.sessionReplay = SentryReplayOptions( + sessionSampleRate: 0, + onErrorSampleRate: 1, + maskAllText: true, + maskAllImages: true + ) options.sessionReplay.quality = .high } diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index e1cddf21f07..32bbdd382fc 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -835,6 +835,7 @@ D4AF00212D2E92FD00F5F3D7 /* SentryNSFileManagerSwizzling.m in Sources */ = {isa = PBXBuildFile; fileRef = D4AF00202D2E92FD00F5F3D7 /* SentryNSFileManagerSwizzling.m */; }; D4AF00232D2E931000F5F3D7 /* SentryNSFileManagerSwizzling.h in Headers */ = {isa = PBXBuildFile; fileRef = D4AF00222D2E931000F5F3D7 /* SentryNSFileManagerSwizzling.h */; }; D4AF00252D2E93C400F5F3D7 /* SentryNSFileManagerSwizzlingTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D4AF00242D2E93C400F5F3D7 /* SentryNSFileManagerSwizzlingTests.m */; }; + D4B0DC7F2DA9257A00DE61B6 /* SentryRenderVideoResult.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4B0DC7E2DA9257200DE61B6 /* SentryRenderVideoResult.swift */; }; D4C5F59A2D4249E6002A9BF6 /* DataSentryTracingIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4C5F5992D4249E0002A9BF6 /* DataSentryTracingIntegrationTests.swift */; }; D4E3F35D2D4A864600F79E2B /* SentryNSDictionarySanitizeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D42E48582D48FC8F00D251BC /* SentryNSDictionarySanitizeTests.swift */; }; D4E3F35E2D4A877300F79E2B /* SentryNSDictionarySanitize+Tests.m in Sources */ = {isa = PBXBuildFile; fileRef = D41909942D490006002B83D0 /* SentryNSDictionarySanitize+Tests.m */; }; @@ -2014,6 +2015,7 @@ D4AF00202D2E92FD00F5F3D7 /* SentryNSFileManagerSwizzling.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryNSFileManagerSwizzling.m; sourceTree = ""; }; D4AF00222D2E931000F5F3D7 /* SentryNSFileManagerSwizzling.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryNSFileManagerSwizzling.h; path = include/SentryNSFileManagerSwizzling.h; sourceTree = ""; }; D4AF00242D2E93C400F5F3D7 /* SentryNSFileManagerSwizzlingTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryNSFileManagerSwizzlingTests.m; sourceTree = ""; }; + D4B0DC7E2DA9257200DE61B6 /* SentryRenderVideoResult.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryRenderVideoResult.swift; sourceTree = ""; }; D4C5F5992D4249E0002A9BF6 /* DataSentryTracingIntegrationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DataSentryTracingIntegrationTests.swift; sourceTree = ""; }; D4E829D12D75E2DE00D375AD /* SentryDefaultViewRenderer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryDefaultViewRenderer.swift; sourceTree = ""; }; D4E829D32D75E34A00D375AD /* SentryExperimentalViewRenderer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryExperimentalViewRenderer.swift; sourceTree = ""; }; @@ -4269,6 +4271,7 @@ D8CAC02A2BA0663E00E38F34 /* SentryReplayOptions.swift */, D8CAC02B2BA0663E00E38F34 /* SentryVideoInfo.swift */, D802994D2BA836EF000F0081 /* SentryOnDemandReplay.swift */, + D4B0DC7E2DA9257200DE61B6 /* SentryRenderVideoResult.swift */, D451ED5E2D92ECDE00C9BEA8 /* SentryReplayFrame.swift */, D451ED5C2D92ECD200C9BEA8 /* SentryOnDemandReplayError.swift */, D802994F2BA83A88000F0081 /* SentryPixelBuffer.swift */, @@ -4959,6 +4962,7 @@ 7B98D7D325FB65AE00C5A389 /* SentryWatchdogTerminationTracker.m in Sources */, 8E564AE8267AF22600FE117D /* SentryNetworkTrackingIntegration.m in Sources */, 63AA75EF1EB8B3C400D153DE /* SentryClient.m in Sources */, + D4B0DC7F2DA9257A00DE61B6 /* SentryRenderVideoResult.swift in Sources */, 7B7D873624864C9D00D2ECFF /* SentryCrashDefaultMachineContextWrapper.m in Sources */, 63FE712F20DA4C1100CDBAE8 /* SentryCrashSysCtl.c in Sources */, 62212B872D520CB00062C2FA /* SentryEventCodable.swift in Sources */, diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index fc8841e9270..9312be314c3 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -1,3 +1,4 @@ +// swiftlint:disable file_length #if canImport(UIKit) && !SENTRY_NO_UIKIT #if os(iOS) || os(tvOS) @@ -65,19 +66,21 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { SentryLog.debug("[Session Replay] Loading frames from path: \(path)") do { let content = try FileManager.default.contentsOfDirectory(atPath: path) - _frames = content.compactMap { frameFilePath in - guard frameFilePath.hasSuffix(".png") else { return SentryReplayFrame?.none } + _frames = content.compactMap { frameFilePath -> SentryReplayFrame? in + guard frameFilePath.hasSuffix(".png") else { return nil } guard let time = Double(frameFilePath.dropLast(4)) else { return nil } let timestamp = Date(timeIntervalSinceReferenceDate: time) return SentryReplayFrame(imagePath: "\(path)/\(frameFilePath)", time: timestamp, screenName: nil) }.sorted { $0.time < $1.time } SentryLog.debug("[Session Replay] Loaded \(content.count) files into \(_frames.count) frames from path: \(path)") } catch { - SentryLog.debug("[Session Replay] Could not list frames from replay: \(error.localizedDescription)") + SentryLog.error("[Session Replay] Could not list frames from replay: \(error.localizedDescription)") } } func addFrameAsync(image: UIImage, forScreen: String?) { + // Dispatch the frame addition to a background queue to avoid blocking the main queue. + // This must be on the processing queue to avoid deadlocks. processingQueue.dispatchAsync { self.addFrame(image: image, forScreen: forScreen) } @@ -91,11 +94,12 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { do { try data.write(to: URL(fileURLWithPath: imagePath)) } catch { - SentryLog.debug("[Session Replay] Could not save replay frame. Error: \(error)") + SentryLog.error("[Session Replay] Could not save replay frame. Error: \(error)") return } _frames.append(SentryReplayFrame(imagePath: imagePath, time: date, screenName: forScreen)) - + + // Remove the oldest frames if the cache size exceeds the maximum size. while _frames.count > cacheMaxSize { let first = _frames.removeFirst() try? FileManager.default.removeItem(at: URL(fileURLWithPath: first.imagePath)) @@ -120,11 +124,10 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { self._frames.removeFirst() let fileUrl = URL(fileURLWithPath: first.imagePath) do { - SentryLog.debug("[Session Replay] Removing frame at url: \(fileUrl.path)") try FileManager.default.removeItem(at: fileUrl) SentryLog.debug("[Session Replay] Removed frame at url: \(fileUrl.path)") } catch { - SentryLog.warning("[Session Replay] Failed to remove frame at: \(fileUrl.path), reason: \(error.localizedDescription), ignoring error") + SentryLog.error("[Session Replay] Failed to remove frame at: \(fileUrl.path), reason: \(error.localizedDescription), ignoring error") } } } @@ -149,19 +152,23 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // Use a semaphore to wait for each video segment to finish. let semaphore = DispatchSemaphore(value: 0) var currentError: Error? - var frameCount = 0 + var frameIndex = 0 var videos = [SentryVideoInfo]() - while frameCount < videoFrames.count { - let outputFileURL = URL(fileURLWithPath: self._outputPath.appending("/\(videoFrames[frameCount].time.timeIntervalSinceReferenceDate).mp4")) - - self.renderVideo(with: videoFrames, from: &frameCount, at: outputFileURL) { result in + while frameIndex < videoFrames.count { + let frame = videoFrames[frameIndex] + let outputFileURL = URL(fileURLWithPath: self._outputPath) + .appendingPathComponent("\(frame.time.timeIntervalSinceReferenceDate)") + .appendingPathExtension("mp4") + self.renderVideo(with: videoFrames, from: frameIndex, at: outputFileURL) { result in // Do not use `processingQueue` here, since it will be blocked by the semaphore. switch result { - case .success(let videoInfo): - if let videoInfo = videoInfo { + case .success(let videoResult): + frameIndex = videoResult.finalFrameIndex + if let videoInfo = videoResult.info { videos.append(videoInfo) } case .failure(let error): + SentryLog.error("[Session Replay] Failed to render video with error: \(error)") currentError = error } semaphore.signal() @@ -172,6 +179,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // The queue used by render video must have a higher priority than the processing queue to reduce thread inversion. // Otherwise, it could lead to queue starvation and a deadlock. if semaphore.wait(timeout: .now() + 2) == .timedOut { + SentryLog.error("[Session Replay] Timeout while waiting for video rendering to finish.") currentError = SentryOnDemandReplayError.errorRenderingVideo break } @@ -180,21 +188,26 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { if let error = currentError { throw error } + + SentryLog.debug("[Session Replay] Finished rendering video, frame count moved to: \(frameIndex)") } completion(videos, nil) } catch { - SentryLog.debug("[Session Replay] Failed to create video with error: \(error)") + SentryLog.error("[Session Replay] Failed to create video with error: \(error)") completion(nil, error) } } } - // swiftlint:disable function_body_length - private func renderVideo(with videoFrames: [SentryReplayFrame], from: inout Int, at outputFileURL: URL, completion: @escaping (Result) -> Void) { + // swiftlint:disable function_body_length cyclomatic_complexity + private func renderVideo(with videoFrames: [SentryReplayFrame], from: Int, at outputFileURL: URL, completion: @escaping (Result) -> Void) { SentryLog.debug("[Session Replay] Rendering video with \(videoFrames.count) frames, from index: \(from), to output url: \(outputFileURL)") guard from < videoFrames.count, let image = UIImage(contentsOfFile: videoFrames[from].imagePath) else { SentryLog.debug("[Session Replay] Failed to render video, reason: index out of bounds or can't read image at path: \(videoFrames[from].imagePath)") - return completion(.success(nil)) + return completion(.success(SentryRenderVideoResult( + info: nil, + finalFrameIndex: from + ))) } let videoWidth = image.size.width * CGFloat(videoScale) @@ -232,31 +245,58 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // This is important to avoid a deadlock, as this method is called on the processing queue. var lastImageSize: CGSize = image.size var usedFrames = [SentryReplayFrame]() - var frameCount = from + var frameIndex = from + + // Convenience wrapper to handle the completion callback + let deferredCompletionCallback: (Result) -> Void = { result in + switch result { + case .success(let videoResult): + completion(.success(SentryRenderVideoResult( + info: videoResult, + finalFrameIndex: frameIndex + ))) + case .failure(let error): + completion(.failure(error)) + } + } videoWriterInput.requestMediaDataWhenReady(on: assetWorkerQueue.queue) { [weak self] in guard let self = self else { SentryLog.warning("[Session Replay] On-demand replay is deallocated, completing writing session without output video info") - return completion(.success(nil)) + return deferredCompletionCallback(.success(nil)) } guard videoWriter.status == .writing else { SentryLog.warning("[Session Replay] Video writer is not writing anymore, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") videoWriter.cancelWriting() return completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) } - guard frameCount < videoFrames.count else { + guard frameIndex < videoFrames.count else { SentryLog.debug("[Session Replay] No more frames available to process, finishing the video") - return self.finishVideo(outputFileURL: outputFileURL, usedFrames: usedFrames, videoHeight: Int(videoHeight), videoWidth: Int(videoWidth), videoWriter: videoWriter, onCompletion: completion) + return self.finishVideo( + outputFileURL: outputFileURL, + usedFrames: usedFrames, + videoHeight: Int(videoHeight), + videoWidth: Int(videoWidth), + videoWriter: videoWriter, + onCompletion: deferredCompletionCallback + ) } - let frame = videoFrames[frameCount] + let frame = videoFrames[frameIndex] if let image = UIImage(contentsOfFile: frame.imagePath) { guard lastImageSize == image.size else { SentryLog.debug("[Session Replay] Image size changed, finishing the video") - return self.finishVideo(outputFileURL: outputFileURL, usedFrames: usedFrames, videoHeight: Int(videoHeight), videoWidth: Int(videoWidth), videoWriter: videoWriter, onCompletion: completion) + return self.finishVideo( + outputFileURL: outputFileURL, + usedFrames: usedFrames, + videoHeight: Int(videoHeight), + videoWidth: Int(videoWidth), + videoWriter: videoWriter, + onCompletion: deferredCompletionCallback + ) } lastImageSize = image.size - let presentTime = CMTime(seconds: Double(frameCount), preferredTimescale: CMTimeScale(1 / self.frameRate)) + let presentTime = CMTime(seconds: Double(frameIndex), preferredTimescale: CMTimeScale(1 / self.frameRate)) guard currentPixelBuffer.append(image: image, presentationTime: presentTime) == true else { SentryLog.debug("[Session Replay] Failed to append image to pixel buffer, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") videoWriter.cancelWriting() @@ -264,10 +304,10 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } usedFrames.append(frame) } - frameCount += 1 + frameIndex += 1 } } - // swiftlint:enable function_body_length + // swiftlint:enable function_body_length cyclomatic_complexity private func finishVideo( outputFileURL: URL, @@ -362,3 +402,4 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { #endif // os(iOS) || os(tvOS) #endif // canImport(UIKit) +// swiftlint:enable file_length diff --git a/Sources/Swift/Integrations/SessionReplay/SentryRenderVideoResult.swift b/Sources/Swift/Integrations/SessionReplay/SentryRenderVideoResult.swift new file mode 100644 index 00000000000..4b6e6056700 --- /dev/null +++ b/Sources/Swift/Integrations/SessionReplay/SentryRenderVideoResult.swift @@ -0,0 +1,4 @@ +struct SentryRenderVideoResult { + let info: SentryVideoInfo? + let finalFrameIndex: Int +} diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift index f04d71a00e0..42eb29a097d 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift @@ -35,7 +35,11 @@ class SentrySessionReplayTests: XCTestCase { } var lastCallToCreateVideo: CreateVideoCall? - func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] { + func createVideoAsyncWith( + beginning: Date, + end: Date, + completion: @escaping ([Sentry.SentryVideoInfo]?, (any Error)?) -> Void + ) { lastCallToCreateVideo = CreateVideoCall(beginning: beginning, end: end) let outputFileURL = FileManager.default.temporaryDirectory.appendingPathComponent("tempvideo.mp4") @@ -43,7 +47,7 @@ class SentrySessionReplayTests: XCTestCase { let videoInfo = SentryVideoInfo(path: outputFileURL, height: 1_024, width: 480, duration: end.timeIntervalSince(overrideBeginning ?? beginning), frameCount: 5, frameRate: 1, start: overrideBeginning ?? beginning, end: end, fileSize: 10, screens: screens) createVideoCallBack?(videoInfo) - return [videoInfo] + return completion([videoInfo], nil) } var lastFrame: UIImage? From 3ff7c842cc3d98d9db57e61a5f08a8bbe693f6ff Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 11 Apr 2025 12:43:10 +0200 Subject: [PATCH 06/26] add changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67eeebb846a..9a2321e3f4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ - Crash in setMeasurement when name is nil (#5064) - Make setMeasurement thread safe (#5067, #5078) +- Fix thread inversion warning in session replay (#5018) ## 8.49.0 From 44c4af72f68c588083c5c351b853ed8b76821275 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 11 Apr 2025 15:49:33 +0200 Subject: [PATCH 07/26] Apply suggestions from code review --- .../Integrations/SessionReplay/SentryOnDemandReplay.swift | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index c628aa712b4..3cb85628bfc 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -139,7 +139,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { func createVideoAsyncWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) { // Note: In Swift it is best practice to use `Result` instead of `(Value?, Error?)` - // Due to interoperability with Objective-C and @objc, we can not use Result here. + // Due to interoperability with Objective-C and @objc, we can not use Result for the completion callback. SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") // Dispatch the video creation to a background queue to avoid blocking the calling queue. @@ -203,7 +203,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { private func renderVideo(with videoFrames: [SentryReplayFrame], from: Int, at outputFileURL: URL, completion: @escaping (Result) -> Void) { SentryLog.debug("[Session Replay] Rendering video with \(videoFrames.count) frames, from index: \(from), to output url: \(outputFileURL)") guard from < videoFrames.count, let image = UIImage(contentsOfFile: videoFrames[from].imagePath) else { - SentryLog.debug("[Session Replay] Failed to render video, reason: index out of bounds or can't read image at path: \(videoFrames[from].imagePath)") + SentryLog.error("[Session Replay] Failed to render video, reason: index out of bounds or can't read image at path: \(videoFrames[from].imagePath)") return completion(.success(SentryRenderVideoResult( info: nil, finalFrameIndex: from @@ -218,7 +218,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { do { videoWriter = try AVAssetWriter(url: outputFileURL, fileType: .mp4) } catch { - SentryLog.debug("[Session Replay] Failed to create video writer, reason: \(error)") + SentryLog.error("[Session Replay] Failed to create video writer, reason: \(error)") return completion(.failure(error)) } From 773fcadd69ea8faef80bbb70cd24f7217dbb4c9f Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 24 Apr 2025 10:30:17 +0200 Subject: [PATCH 08/26] reverting changes --- CHANGELOG.md | 6 + .../Sentry/SentrySessionReplayIntegration.m | 23 +--- .../SessionReplay/SentryOnDemandReplay.swift | 110 +++++++++--------- .../SentryReplayVideoMaker.swift | 3 +- .../SessionReplay/SentrySessionReplay.swift | 8 +- 5 files changed, 74 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 40d75dc3f26..391b4a5d7c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Fix thread inversion warning in session replay (#5018) + ## 8.49.1 ### Fixes diff --git a/Sources/Sentry/SentrySessionReplayIntegration.m b/Sources/Sentry/SentrySessionReplayIntegration.m index dffd0b9ef17..30ed5de58e9 100644 --- a/Sources/Sentry/SentrySessionReplayIntegration.m +++ b/Sources/Sentry/SentrySessionReplayIntegration.m @@ -223,25 +223,10 @@ - (void)resumePreviousSessionReplay:(SentryEvent *)event } NSDate *end = [beginning dateByAddingTimeInterval:duration]; - // This method is called from a background thread, so we can synchronize the creation of the - // video with a dispatch group. - __block NSArray *videos; - __block NSError *_Nullable error; - - dispatch_group_t group = dispatch_group_create(); - dispatch_group_enter(group); - [resumeReplayMaker - createVideoAsyncWithBeginning:beginning - end:end - completion:^(NSArray *_Nullable resultVideos, - NSError *_Nullable resultError) { - videos = resultVideos; - error = resultError; - dispatch_group_leave(group); - }]; - // Wait for the video creation to finish without a timeout, because the video creation is - // expected to finish in a reasonable time frame. - dispatch_group_wait(group, DISPATCH_TIME_FOREVER); + NSError *error; + NSArray *videos = [resumeReplayMaker createVideoWithBeginning:beginning + end:end + error:&error]; // Either error or videos should be set. if (error != nil) { diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 3cb85628bfc..4c01c960fb8 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -1,4 +1,4 @@ -// swiftlint:disable file_length +// swiftlint:disable file_length type_body_length #if canImport(UIKit) && !SENTRY_NO_UIKIT #if os(iOS) || os(tvOS) @@ -137,66 +137,72 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { return _frames.first?.time } - func createVideoAsyncWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) { + func createVideoInBackgroundWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) { // Note: In Swift it is best practice to use `Result` instead of `(Value?, Error?)` // Due to interoperability with Objective-C and @objc, we can not use Result for the completion callback. - SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") - - // Dispatch the video creation to a background queue to avoid blocking the calling queue. + SentryLog.debug("[Session Replay] Creating video in background with beginning: \(beginning), end: \(end)") processingQueue.dispatchAsync { - SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end), current queue: \(self.processingQueue.queue.label)") - - let videoFrames = self._frames.filter { $0.time >= beginning && $0.time <= end } - do { - // Use a semaphore to wait for each video segment to finish. - let semaphore = DispatchSemaphore(value: 0) - var currentError: Error? - var frameIndex = 0 - var videos = [SentryVideoInfo]() - while frameIndex < videoFrames.count { - let frame = videoFrames[frameIndex] - let outputFileURL = URL(fileURLWithPath: self._outputPath) - .appendingPathComponent("\(frame.time.timeIntervalSinceReferenceDate)") - .appendingPathExtension("mp4") - self.renderVideo(with: videoFrames, from: frameIndex, at: outputFileURL) { result in - // Do not use `processingQueue` here, since it will be blocked by the semaphore. - switch result { - case .success(let videoResult): - frameIndex = videoResult.finalFrameIndex - if let videoInfo = videoResult.info { - videos.append(videoInfo) - } - case .failure(let error): - SentryLog.error("[Session Replay] Failed to render video with error: \(error)") - currentError = error - } - semaphore.signal() - } + let videos = try self.createVideoWith(beginning: beginning, end: end) + SentryLog.debug("[Session Replay] Finished creating video in backgroundwith \(videos.count) segments") + completion(videos, nil) + } catch { + SentryLog.error("[Session Replay] Failed to create video in background with error: \(error)") + completion(nil, error) + } + } + } - // Calling semaphore.wait will block the `processingQueue` until the video rendering completes or a timeout occurs. - // It is imporant that the renderVideo completion block signals the semaphore. - // The queue used by render video must have a higher priority than the processing queue to reduce thread inversion. - // Otherwise, it could lead to queue starvation and a deadlock. - if semaphore.wait(timeout: .now() + 2) == .timedOut { - SentryLog.error("[Session Replay] Timeout while waiting for video rendering to finish.") - currentError = SentryOnDemandReplayError.errorRenderingVideo - break - } + func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] { + SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") - // If there was an error, throw it to exit the loop. - if let error = currentError { - throw error - } + let videoFrames = self._frames.filter { $0.time >= beginning && $0.time <= end } + + var frameCount = 0 + var videos = [SentryVideoInfo]() - SentryLog.debug("[Session Replay] Finished rendering video, frame count moved to: \(frameIndex)") + while frameCount < videoFrames.count { + let outputFileURL = URL(fileURLWithPath: _outputPath) + .appendingPathComponent("\(videoFrames[frameCount].time.timeIntervalSinceReferenceDate)") + .appendingPathExtension("mp4") + + let group = DispatchGroup() + var currentError: Error? + + group.enter() + self.renderVideo(with: videoFrames, from: frameCount, at: outputFileURL) { result in + // Do not use `processingQueue` here, since it will be blocked by the semaphore. + switch result { + case .success(let videoResult): + frameCount = videoResult.finalFrameIndex + if let videoInfo = videoResult.info { + videos.append(videoInfo) + } + case .failure(let error): + SentryLog.error("[Session Replay] Failed to render video with error: \(error)") + currentError = error } - completion(videos, nil) - } catch { - SentryLog.error("[Session Replay] Failed to create video with error: \(error)") - completion(nil, error) + group.leave() } + + // Calling semaphore.wait will block the `processingQueue` until the video rendering completes or a timeout occurs. + // It is imporant that the renderVideo completion block signals the semaphore. + // The queue used by render video must have a higher priority than the processing queue to reduce thread inversion. + // Otherwise, it could lead to queue starvation and a deadlock. + guard group.wait(timeout: .now() + 2) == .success else { + SentryLog.error("[Session Replay] Timeout while waiting for video rendering to finish.") + currentError = SentryOnDemandReplayError.errorRenderingVideo + break + } + + // If there was an error, throw it to exit the loop. + if let error = currentError { + throw error + } + + SentryLog.debug("[Session Replay] Finished rendering video, frame count moved to: \(frameCount)") } + return videos } // swiftlint:disable function_body_length cyclomatic_complexity @@ -402,4 +408,4 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { #endif // os(iOS) || os(tvOS) #endif // canImport(UIKit) -// swiftlint:enable file_length +// swiftlint:enable file_length type_body_length diff --git a/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift b/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift index 9ea18313eea..66a19bab4ef 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift @@ -6,7 +6,8 @@ import UIKit protocol SentryReplayVideoMaker: NSObjectProtocol { func addFrameAsync(image: UIImage, forScreen: String?) func releaseFramesUntil(_ date: Date) - func createVideoAsyncWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) + func createVideoInBackgroundWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) + func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] } extension SentryReplayVideoMaker { diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift index a46fe461a32..a179710f541 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift @@ -157,7 +157,7 @@ class SentrySessionReplay: NSObject { startFullReplay() let replayStart = dateProvider.date().addingTimeInterval(-replayOptions.errorReplayDuration - (Double(replayOptions.frameRate) / 2.0)) - createAndCaptureAsync(startedAt: replayStart, replayType: .buffer) + createAndCaptureInBackground(startedAt: replayStart, replayType: .buffer) return true } @@ -218,13 +218,13 @@ class SentrySessionReplay: NSObject { pathToSegment = pathToSegment.appendingPathComponent("\(currentSegmentId).mp4") let segmentStart = videoSegmentStart ?? dateProvider.date().addingTimeInterval(-replayOptions.sessionSegmentDuration) - createAndCaptureAsync(startedAt: segmentStart, replayType: .session) + createAndCaptureInBackground(startedAt: segmentStart, replayType: .session) } - private func createAndCaptureAsync(startedAt: Date, replayType: SentryReplayType) { + private func createAndCaptureInBackground(startedAt: Date, replayType: SentryReplayType) { SentryLog.debug("[Session Replay] Creating replay video started at date: \(startedAt), replayType: \(replayType)") // Creating a video is computationally expensive, therefore perform it on a background queue. - self.replayMaker.createVideoAsyncWith(beginning: startedAt, end: self.dateProvider.date()) { videos, error in + self.replayMaker.createVideoInBackgroundWith(beginning: startedAt, end: self.dateProvider.date()) { videos, error in if let error = error { SentryLog.error("[Session Replay] Could not create replay video - \(error.localizedDescription)") } From 0e9c3e2450a5ff7e330007ca741297c039f3bb85 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 24 Apr 2025 10:37:02 +0200 Subject: [PATCH 09/26] small changes --- CHANGELOG.md | 1 - .../Swift/Integrations/SessionReplay/SentrySessionReplay.swift | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 391b4a5d7c9..845233b4121 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,6 @@ - Crash in setMeasurement when name is nil (#5064) - Make setMeasurement thread safe (#5067, #5078) -- Fix thread inversion warning in session replay (#5018) - Truncation of Swift crash messages (#5036) - Add error logging for move current replay to last path (#5083) - Async safe log for backtrace in CPPException (#5098) diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift index a179710f541..82e2396e01f 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift @@ -227,6 +227,7 @@ class SentrySessionReplay: NSObject { self.replayMaker.createVideoInBackgroundWith(beginning: startedAt, end: self.dateProvider.date()) { videos, error in if let error = error { SentryLog.error("[Session Replay] Could not create replay video - \(error.localizedDescription)") + return } guard let videos = videos else { SentryLog.warning("[Session Replay] Finished replay video creation without any segments") @@ -236,7 +237,7 @@ class SentrySessionReplay: NSObject { for video in videos { self.newSegmentAvailable(videoInfo: video, replayType: replayType) } - SentryLog.debug("[Session Replay] Finished replay video creation with \(videos.count) segments") + SentryLog.debug("[Session Replay] Finished processing replay video with \(videos.count) segments") } } From c4ba77c7acd9ed9109593d18a91fed08d3e9f6bd Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 24 Apr 2025 10:56:08 +0200 Subject: [PATCH 10/26] WIP --- .../SessionReplay/SentryOnDemandReplay.swift | 19 ++++-- .../SentryOnDemandReplayTests.swift | 61 ++++++++++++------- .../SentrySessionReplayTests.swift | 18 ++++-- 3 files changed, 68 insertions(+), 30 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 4c01c960fb8..86b74999070 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -156,8 +156,8 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] { SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") - let videoFrames = self._frames.filter { $0.time >= beginning && $0.time <= end } - + let videoFrames = self.filterFrames(beginning: beginning, end: end) + var frameCount = 0 var videos = [SentryVideoInfo]() @@ -189,7 +189,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // It is imporant that the renderVideo completion block signals the semaphore. // The queue used by render video must have a higher priority than the processing queue to reduce thread inversion. // Otherwise, it could lead to queue starvation and a deadlock. - guard group.wait(timeout: .now() + 2) == .success else { + guard group.wait(timeout: .now() + 120) == .success else { SentryLog.error("[Session Replay] Timeout while waiting for video rendering to finish.") currentError = SentryOnDemandReplayError.errorRenderingVideo break @@ -304,7 +304,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { let presentTime = CMTime(seconds: Double(frameIndex), preferredTimescale: CMTimeScale(1 / self.frameRate)) guard currentPixelBuffer.append(image: image, presentationTime: presentTime) == true else { - SentryLog.debug("[Session Replay] Failed to append image to pixel buffer, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + SentryLog.error("[Session Replay] Failed to append image to pixel buffer, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") videoWriter.cancelWriting() return completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) } @@ -313,6 +313,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { frameIndex += 1 } } + // swiftlint:enable function_body_length cyclomatic_complexity private func finishVideo( @@ -367,7 +368,17 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } } + private func filterFrames(beginning: Date, end: Date) -> [SentryReplayFrame] { + var frames = [SentryReplayFrame]() + // Using dispatch queue as sync mechanism since we need a queue already to generate the video. + processingQueue.dispatchSync { + frames = self._frames.filter { $0.time >= beginning && $0.time <= end } + } + return frames + } + fileprivate func getVideoInfo(from outputFileURL: URL, usedFrames: [SentryReplayFrame], videoWidth: Int, videoHeight: Int) throws -> SentryVideoInfo { + SentryLog.debug("[Session Replay] Getting video info from file: \(outputFileURL.path), width: \(videoWidth), height: \(videoHeight), used frames count: \(usedFrames.count)") let fileAttributes = try FileManager.default.attributesOfItem(atPath: outputFileURL.path) guard let fileSize = fileAttributes[FileAttributeKey.size] as? Int else { SentryLog.warning("[Session Replay] Failed to read video size from video file, reason: size attribute not found") diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift index 85c59e15a69..7d5eab9e7a5 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift @@ -26,9 +26,12 @@ class SentryOnDemandReplayTests: XCTestCase { } private func getSut(trueDispatchQueueWrapper: Bool = false) -> SentryOnDemandReplay { - return SentryOnDemandReplay(outputPath: outputPath.path, - workingQueue: trueDispatchQueueWrapper ? SentryDispatchQueueWrapper() : TestSentryDispatchQueueWrapper(), - dateProvider: dateProvider) + return SentryOnDemandReplay( + outputPath: outputPath.path, + processingQueue: trueDispatchQueueWrapper ? SentryDispatchQueueWrapper() : TestSentryDispatchQueueWrapper(), + assetWorkerQueue: trueDispatchQueueWrapper ? SentryDispatchQueueWrapper() : TestSentryDispatchQueueWrapper(), + dateProvider: dateProvider + ) } func testAddFrame() { @@ -107,11 +110,15 @@ class SentryOnDemandReplayTests: XCTestCase { } func testAddFrameIsThreadSafe() { - let queue = SentryDispatchQueueWrapper() - let sut = SentryOnDemandReplay(outputPath: outputPath.path, - workingQueue: queue, - dateProvider: dateProvider) - + let processingQueue = SentryDispatchQueueWrapper() + let workerQueue = SentryDispatchQueueWrapper() + let sut = SentryOnDemandReplay( + outputPath: outputPath.path, + processingQueue: processingQueue, + assetWorkerQueue: workerQueue, + dateProvider: dateProvider + ) + dateProvider.driftTimeForEveryRead = true dateProvider.driftTimeInterval = 1 let group = DispatchGroup() @@ -125,16 +132,20 @@ class SentryOnDemandReplayTests: XCTestCase { } group.wait() - queue.queue.sync {} //Wait for all enqueued operation to finish + processingQueue.queue.sync {} // Wait for all enqueued operation to finish XCTAssertEqual(sut.frames.map({ ($0.imagePath as NSString).lastPathComponent }), (0..<10).map { "\($0).0.png" }) } func testReleaseIsThreadSafe() { - let queue = SentryDispatchQueueWrapper() - let sut = SentryOnDemandReplay(outputPath: outputPath.path, - workingQueue: queue, - dateProvider: dateProvider) - + let processingQueue = SentryDispatchQueueWrapper() + let workerQueue = SentryDispatchQueueWrapper() + let sut = SentryOnDemandReplay( + outputPath: outputPath.path, + processingQueue: processingQueue, + assetWorkerQueue: workerQueue, + dateProvider: dateProvider + ) + sut.frames = (0..<100).map { SentryReplayFrame(imagePath: outputPath.path + "/\($0).png", time: Date(timeIntervalSinceReferenceDate: Double($0)), screenName: nil) } let group = DispatchGroup() @@ -149,24 +160,30 @@ class SentryOnDemandReplayTests: XCTestCase { group.wait() - queue.queue.sync {} //Wait for all enqueued operation to finish + processingQueue.queue.sync {} //Wait for all enqueued operation to finish XCTAssertEqual(sut.frames.count, 0) } func testInvalidWriter() throws { - let queue = TestSentryDispatchQueueWrapper() - let sut = SentryOnDemandReplay(outputPath: outputPath.path, - workingQueue: queue, - dateProvider: dateProvider) - + // -- Arrange -- + let processingQueue = SentryDispatchQueueWrapper() + let workerQueue = SentryDispatchQueueWrapper() + let sut = SentryOnDemandReplay( + outputPath: outputPath.path, + processingQueue: processingQueue, + assetWorkerQueue: workerQueue, + dateProvider: dateProvider + ) + let start = dateProvider.date() sut.addFrameAsync(image: UIImage.add) dateProvider.advance(by: 1) let end = dateProvider.date() - //Creating a file where the replay would be written to cause an error in the writer + // Creating a file where the replay would be written to cause an error in the writer try Data("tempFile".utf8).write(to: outputPath.appendingPathComponent("0.0.mp4")) - + + // -- Act & Assert -- XCTAssertThrowsError(try sut.createVideoWith(beginning: start, end: end)) } diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift index 42eb29a097d..63ae9f1da96 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift @@ -35,11 +35,22 @@ class SentrySessionReplayTests: XCTestCase { } var lastCallToCreateVideo: CreateVideoCall? - func createVideoAsyncWith( + func createVideoInBackgroundWith( beginning: Date, end: Date, completion: @escaping ([Sentry.SentryVideoInfo]?, (any Error)?) -> Void ) { + // Note: This implementation is just to satisfy the protocol. + // If possible, keep the tests logic the synchronous version `createVideoWith` + do { + let videos = try createVideoWith(beginning: beginning, end: end) + completion(videos, nil) + } catch { + completion(nil, error) + } + } + + func createVideoWith(beginning: Date, end: Date) throws -> [Sentry.SentryVideoInfo] { lastCallToCreateVideo = CreateVideoCall(beginning: beginning, end: end) let outputFileURL = FileManager.default.temporaryDirectory.appendingPathComponent("tempvideo.mp4") @@ -47,7 +58,7 @@ class SentrySessionReplayTests: XCTestCase { let videoInfo = SentryVideoInfo(path: outputFileURL, height: 1_024, width: 480, duration: end.timeIntervalSince(overrideBeginning ?? beginning), frameCount: 5, frameRate: 1, start: overrideBeginning ?? beginning, end: end, fileSize: 10, screens: screens) createVideoCallBack?(videoInfo) - return completion([videoInfo], nil) + return [videoInfo] } var lastFrame: UIImage? @@ -80,7 +91,7 @@ class SentrySessionReplayTests: XCTestCase { var lastReplayId: SentryId? var currentScreen: String? - func getSut(options: SentryReplayOptions = .init(sessionSampleRate: 0, onErrorSampleRate: 0), dispatchQueue: SentryDispatchQueueWrapper = TestSentryDispatchQueueWrapper(), touchTracker: SentryTouchTracker? = nil) -> SentrySessionReplay { + func getSut(options: SentryReplayOptions = .init(sessionSampleRate: 0, onErrorSampleRate: 0), touchTracker: SentryTouchTracker? = nil) -> SentrySessionReplay { return SentrySessionReplay(replayOptions: options, replayFolderPath: cacheFolder, screenshotProvider: screenshotProvider, @@ -89,7 +100,6 @@ class SentrySessionReplayTests: XCTestCase { touchTracker: touchTracker ?? SentryTouchTracker(dateProvider: dateProvider, scale: 0), dateProvider: dateProvider, delegate: self, - dispatchQueue: dispatchQueue, displayLinkWrapper: displayLink) } From ef3d279bec4968cc5447cac84aef9d62c9f45645 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 25 Apr 2025 10:43:02 +0200 Subject: [PATCH 11/26] fix merge error --- .../Integrations/SessionReplay/SentryOnDemandReplay.swift | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 4db68859eaa..0965221f0ee 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -304,7 +304,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { lastImageSize = image.size let presentTime = SentryOnDemandReplay.calculatePresentationTime( - forFrameAtIndex: frameCount, + forFrameAtIndex: frameIndex, frameRate: self.frameRate ).timeValue guard currentPixelBuffer.append(image: image, presentationTime: presentTime) == true else { @@ -319,7 +319,6 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } // swiftlint:enable function_body_length cyclomatic_complexity - private func finishVideo( outputFileURL: URL, usedFrames: [SentryReplayFrame], From 51beda1ac171c65c4626b55ede7764371f413ee4 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 25 Apr 2025 10:57:10 +0200 Subject: [PATCH 12/26] small fixes --- CHANGELOG.md | 8 ++++---- .../Integrations/SessionReplay/SentryOnDemandReplay.swift | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 154e5f865b1..564513cbc5e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,14 +2,14 @@ ## Unreleased -### Fixes - -- Fix thread inversion warning in session replay (#5018) - ### Features - Added ability to bring your own button for user feedback form display (#5107) +### Fixes + +- Fix thread inversion warning in session replay (#5018) + ### Improvements - More logging for Session Replay video info (#5132) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 0965221f0ee..e494d02e657 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -157,9 +157,9 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] { SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") - let videoFrames = self.filterFrames(beginning: beginning, end: end) - + let videoFrames = filterFrames(beginning: beginning, end: end) var frameCount = 0 + var videos = [SentryVideoInfo]() while frameCount < videoFrames.count { From 8614798932fb0dc30695cbc501959cfeeb3c0e20 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 25 Apr 2025 13:51:52 +0200 Subject: [PATCH 13/26] small reverts --- .../SessionReplay/SentryOnDemandReplay.swift | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index e494d02e657..25ffa40262a 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -236,25 +236,15 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { return completion(.failure(SentryOnDemandReplayError.cantCreatePixelBuffer)) } videoWriter.add(videoWriterInput) - videoWriter.startWriting() videoWriter.startSession(atSourceTime: .zero) - // Append frames to the video writer input in a pull-style manner when the input is ready to receive more media data. - // - // Inside the callback: - // 1. We append media data until `isReadyForMoreMediaData` becomes false - // 2. Or until there's no more media data to process (then we mark input as finished) - // 3. If we don't mark the input as finished, the callback will be invoked again - // when the input is ready for more data - // - // By setting the queue to the asset worker queue, we ensure that the callback is invoked on the asset worker queue. - // This is important to avoid a deadlock, as this method is called on the processing queue. var lastImageSize: CGSize = image.size var usedFrames = [SentryReplayFrame]() var frameIndex = from - // Convenience wrapper to handle the completion callback + // Convenience wrapper to handle the completion callback to return the video info and the final frame index + // It is not possible to use an inout frame index here, because the closure is escaping and the frameIndex variable is captured. let deferredCompletionCallback: (Result) -> Void = { result in switch result { case .success(let videoResult): @@ -266,19 +256,30 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { completion(.failure(error)) } } + + // Append frames to the video writer input in a pull-style manner when the input is ready to receive more media data. + // + // Inside the callback: + // 1. We append media data until `isReadyForMoreMediaData` becomes false + // 2. Or until there's no more media data to process (then we mark input as finished) + // 3. If we don't mark the input as finished, the callback will be invoked again + // when the input is ready for more data + // + // By setting the queue to the asset worker queue, we ensure that the callback is invoked on the asset worker queue. + // This is important to avoid a deadlock, as this method is called on the processing queue. videoWriterInput.requestMediaDataWhenReady(on: assetWorkerQueue.queue) { [weak self] in - guard let self = self else { + guard let strongSelf = self else { SentryLog.warning("[Session Replay] On-demand replay is deallocated, completing writing session without output video info") return deferredCompletionCallback(.success(nil)) } guard videoWriter.status == .writing else { SentryLog.warning("[Session Replay] Video writer is not writing anymore, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") videoWriter.cancelWriting() - return completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) + return deferredCompletionCallback(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) } guard frameIndex < videoFrames.count else { SentryLog.debug("[Session Replay] No more frames available to process, finishing the video") - return self.finishVideo( + return strongSelf.finishVideo( outputFileURL: outputFileURL, usedFrames: usedFrames, videoHeight: Int(videoHeight), @@ -292,7 +293,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { if let image = UIImage(contentsOfFile: frame.imagePath) { guard lastImageSize == image.size else { SentryLog.debug("[Session Replay] Image size changed, finishing the video") - return self.finishVideo( + return strongSelf.finishVideo( outputFileURL: outputFileURL, usedFrames: usedFrames, videoHeight: Int(videoHeight), @@ -305,12 +306,12 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { let presentTime = SentryOnDemandReplay.calculatePresentationTime( forFrameAtIndex: frameIndex, - frameRate: self.frameRate + frameRate: strongSelf.frameRate ).timeValue guard currentPixelBuffer.append(image: image, presentationTime: presentTime) == true else { SentryLog.error("[Session Replay] Failed to append image to pixel buffer, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") videoWriter.cancelWriting() - return completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) + return deferredCompletionCallback(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) } usedFrames.append(frame) } From bdfd4bc64019d3c25e07081510cdbfc0280f1494 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 25 Apr 2025 14:01:59 +0200 Subject: [PATCH 14/26] small reverts --- .../SessionReplay/SentryOnDemandReplay.swift | 25 ++++++++++++------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 25ffa40262a..e52104ad8eb 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -119,8 +119,8 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } func releaseFramesUntil(_ date: Date) { + SentryLog.debug("[Session Replay] Releasing frames until date: \(date)") processingQueue.dispatchAsync { - SentryLog.debug("[Session Replay] Releasing frames until date: \(date)") while let first = self._frames.first, first.time < date { self._frames.removeFirst() let fileUrl = URL(fileURLWithPath: first.imagePath) @@ -163,8 +163,9 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { var videos = [SentryVideoInfo]() while frameCount < videoFrames.count { + let frame = videoFrames[frameCount] let outputFileURL = URL(fileURLWithPath: _outputPath) - .appendingPathComponent("\(videoFrames[frameCount].time.timeIntervalSinceReferenceDate)") + .appendingPathComponent("\(frame.time.timeIntervalSinceReferenceDate)") .appendingPathExtension("mp4") let group = DispatchGroup() @@ -172,10 +173,14 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { group.enter() self.renderVideo(with: videoFrames, from: frameCount, at: outputFileURL) { result in - // Do not use `processingQueue` here, since it will be blocked by the semaphore. switch result { case .success(let videoResult): + // Set the frame count/offset to the new index that is returned by the completion block. + // This is important to avoid processing the same frame multiple times. frameCount = videoResult.finalFrameIndex + + // Append the video info to the videos array. + // In case no video info is returned, skip the segment. if let videoInfo = videoResult.info { videos.append(videoInfo) } @@ -186,11 +191,11 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { group.leave() } - // Calling semaphore.wait will block the `processingQueue` until the video rendering completes or a timeout occurs. - // It is imporant that the renderVideo completion block signals the semaphore. + // Calling group.wait will block the `processingQueue` until the video rendering completes or a timeout occurs. + // It is imporant that the renderVideo completion block signals the group. // The queue used by render video must have a higher priority than the processing queue to reduce thread inversion. - // Otherwise, it could lead to queue starvation and a deadlock. - guard group.wait(timeout: .now() + 120) == .success else { + // Otherwise, it could lead to queue starvation and a deadlock/timeout. + guard group.wait(timeout: .now() + 2) == .success else { SentryLog.error("[Session Replay] Timeout while waiting for video rendering to finish.") currentError = SentryOnDemandReplayError.errorRenderingVideo break @@ -232,7 +237,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { SentryLog.debug("[Session Replay] Creating pixel buffer based video writer input") let videoWriterInput = AVAssetWriterInput(mediaType: .video, outputSettings: createVideoSettings(width: videoWidth, height: videoHeight)) guard let currentPixelBuffer = SentryPixelBuffer(size: pixelSize, videoWriterInput: videoWriterInput) else { - SentryLog.debug("[Session Replay] Failed to create pixel buffer, reason: \(SentryOnDemandReplayError.cantCreatePixelBuffer)") + SentryLog.error("[Session Replay] Failed to create pixel buffer, reason: \(SentryOnDemandReplayError.cantCreatePixelBuffer)") return completion(.failure(SentryOnDemandReplayError.cantCreatePixelBuffer)) } videoWriter.add(videoWriterInput) @@ -315,6 +320,9 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } usedFrames.append(frame) } + + // Increment the frame index even if the image could not be appended to the pixel buffer. + // This is important to avoid an infinite loop. frameIndex += 1 } } @@ -388,7 +396,6 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { SentryLog.warning("[Session Replay] Failed to read video size from video file, reason: size attribute not found") throw SentryOnDemandReplayError.cantReadVideoSize } - let minFrame = usedFrames.min(by: { $0.time < $1.time }) guard let start = minFrame?.time else { // Note: This code path is currently not reached, because the `getVideoInfo` method is only called after the video is successfully created, therefore at least one frame was used. From 0cfab3885b1e60798969fd66c501557165961b8c Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Wed, 30 Apr 2025 13:11:59 +0200 Subject: [PATCH 15/26] fix sync filtering --- .../SessionReplay/SentryOnDemandReplay.swift | 21 ++++++------------- .../SentryOnDemandReplayTests.swift | 3 +++ 2 files changed, 9 insertions(+), 15 deletions(-) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index e52104ad8eb..db5e332d776 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -155,9 +155,10 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] { - SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") - - let videoFrames = filterFrames(beginning: beginning, end: end) + SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") + // Note: In previous implementations this method was wrapped by a sync call to the processing queue. + // As this method is already called from the processing queue, we must remove the sync call. + let videoFrames = self._frames.filter { $0.time >= beginning && $0.time <= end } var frameCount = 0 var videos = [SentryVideoInfo]() @@ -195,10 +196,9 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // It is imporant that the renderVideo completion block signals the group. // The queue used by render video must have a higher priority than the processing queue to reduce thread inversion. // Otherwise, it could lead to queue starvation and a deadlock/timeout. - guard group.wait(timeout: .now() + 2) == .success else { + guard group.wait(timeout: .now() + 120) == .success else { SentryLog.error("[Session Replay] Timeout while waiting for video rendering to finish.") - currentError = SentryOnDemandReplayError.errorRenderingVideo - break + throw SentryOnDemandReplayError.errorRenderingVideo } // If there was an error, throw it to exit the loop. @@ -380,15 +380,6 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } } - private func filterFrames(beginning: Date, end: Date) -> [SentryReplayFrame] { - var frames = [SentryReplayFrame]() - // Using dispatch queue as sync mechanism since we need a queue already to generate the video. - processingQueue.dispatchSync { - frames = self._frames.filter { $0.time >= beginning && $0.time <= end } - } - return frames - } - fileprivate func getVideoInfo(from outputFileURL: URL, usedFrames: [SentryReplayFrame], videoWidth: Int, videoHeight: Int) throws -> SentryVideoInfo { SentryLog.debug("[Session Replay] Getting video info from file: \(outputFileURL.path), width: \(videoWidth), height: \(videoHeight), used frames count: \(usedFrames.count)") let fileAttributes = try FileManager.default.attributesOfItem(atPath: outputFileURL.path) diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift index 202179c13ec..07d60f86421 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift @@ -179,6 +179,9 @@ class SentryOnDemandReplayTests: XCTestCase { let start = dateProvider.date() sut.addFrameAsync(image: UIImage.add) + processingQueue.dispatchSync { + // Wait for the frame to be added by adding a sync operation to the serial queue + } dateProvider.advance(by: 1) let end = dateProvider.date() From 015e2ee6f593a5c67e255d66d010ea308b23d47c Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Wed, 30 Apr 2025 13:26:10 +0200 Subject: [PATCH 16/26] Added more logs and tests; moved types to own files due to linter warnings --- .../iOS-Swift/SentrySDKWrapper.swift | 2 +- Sentry.xcodeproj/project.pbxproj | 12 +++++++ .../SessionReplay/SentryOnDemandReplay.swift | 28 +++++++++++---- .../SessionReplay/SentryReplayType.swift | 8 +++++ .../SessionReplay/SentrySessionReplay.swift | 21 ++++-------- .../SentrySessionReplayDelegate.swift | 14 ++++++++ .../SessionReplay/SessionReplayError.swift | 4 +++ .../SessionReplay/SentryReplayTypeTests.swift | 34 +++++++++++++++++++ 8 files changed, 100 insertions(+), 23 deletions(-) create mode 100644 Sources/Swift/Integrations/SessionReplay/SentrySessionReplayDelegate.swift create mode 100644 Sources/Swift/Integrations/SessionReplay/SessionReplayError.swift create mode 100644 Tests/SentryTests/Integrations/SessionReplay/SentryReplayTypeTests.swift diff --git a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift index 4695f86e2d4..c4aba0df64e 100644 --- a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift +++ b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift @@ -29,7 +29,7 @@ struct SentrySDKWrapper { if #available(iOS 16.0, *), !SentrySDKOverrides.Other.disableSessionReplay.boolValue { options.sessionReplay = SentryReplayOptions( - sessionSampleRate: 0, + sessionSampleRate: 1, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index 078308bf957..e2bd4f19e62 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -835,6 +835,9 @@ D48724E22D354D16005DE483 /* SentryTraceOriginTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D48724E12D354D16005DE483 /* SentryTraceOriginTests.swift */; }; D48E8B8B2D3E79610032E35E /* SentryTraceOrigin.swift in Sources */ = {isa = PBXBuildFile; fileRef = D48E8B8A2D3E79610032E35E /* SentryTraceOrigin.swift */; }; D48E8B9D2D3E82AC0032E35E /* SentrySpanOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = D48E8B9C2D3E82AC0032E35E /* SentrySpanOperation.swift */; }; + D49480D32DC23E9300A3B6E9 /* SentryReplayTypeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D49480D22DC23E8E00A3B6E9 /* SentryReplayTypeTests.swift */; }; + D49480D52DC23FD500A3B6E9 /* SessionReplayError.swift in Sources */ = {isa = PBXBuildFile; fileRef = D49480D42DC23FD500A3B6E9 /* SessionReplayError.swift */; }; + D49480D72DC23FE300A3B6E9 /* SentrySessionReplayDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = D49480D62DC23FE200A3B6E9 /* SentrySessionReplayDelegate.swift */; }; D4AF00212D2E92FD00F5F3D7 /* SentryNSFileManagerSwizzling.m in Sources */ = {isa = PBXBuildFile; fileRef = D4AF00202D2E92FD00F5F3D7 /* SentryNSFileManagerSwizzling.m */; }; D4AF00232D2E931000F5F3D7 /* SentryNSFileManagerSwizzling.h in Headers */ = {isa = PBXBuildFile; fileRef = D4AF00222D2E931000F5F3D7 /* SentryNSFileManagerSwizzling.h */; }; D4AF00252D2E93C400F5F3D7 /* SentryNSFileManagerSwizzlingTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D4AF00242D2E93C400F5F3D7 /* SentryNSFileManagerSwizzlingTests.m */; }; @@ -2010,6 +2013,9 @@ D48724E12D354D16005DE483 /* SentryTraceOriginTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryTraceOriginTests.swift; sourceTree = ""; }; D48E8B8A2D3E79610032E35E /* SentryTraceOrigin.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryTraceOrigin.swift; sourceTree = ""; }; D48E8B9C2D3E82AC0032E35E /* SentrySpanOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySpanOperation.swift; sourceTree = ""; }; + D49480D22DC23E8E00A3B6E9 /* SentryReplayTypeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryReplayTypeTests.swift; sourceTree = ""; }; + D49480D42DC23FD500A3B6E9 /* SessionReplayError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionReplayError.swift; sourceTree = ""; }; + D49480D62DC23FE200A3B6E9 /* SentrySessionReplayDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySessionReplayDelegate.swift; sourceTree = ""; }; D4A236012D5F838200D55C58 /* macOS-Swift_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "macOS-Swift_Base.xctestplan"; sourceTree = ""; }; D4A236062D5F846200D55C58 /* iOS-ObjectiveC_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "iOS-ObjectiveC_Base.xctestplan"; sourceTree = ""; }; D4A236072D5F846F00D55C58 /* iOS-Swift6_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "iOS-Swift6_Base.xctestplan"; sourceTree = ""; }; @@ -3964,6 +3970,7 @@ D80694C12B7CC85800B820E6 /* SessionReplay */ = { isa = PBXGroup; children = ( + D49480D22DC23E8E00A3B6E9 /* SentryReplayTypeTests.swift */, D80694C22B7CC86E00B820E6 /* SentryReplayEventTests.swift */, D80694C52B7CCFA100B820E6 /* SentryReplayRecordingTests.swift */, D86130112BB563FD004C0F5E /* SentrySessionReplayIntegrationTests.swift */, @@ -4282,12 +4289,14 @@ D8CAC02A2BA0663E00E38F34 /* SentryReplayOptions.swift */, D8CAC02B2BA0663E00E38F34 /* SentryVideoInfo.swift */, D802994D2BA836EF000F0081 /* SentryOnDemandReplay.swift */, + D49480D42DC23FD500A3B6E9 /* SessionReplayError.swift */, D4B0DC7E2DA9257200DE61B6 /* SentryRenderVideoResult.swift */, D451ED5E2D92ECDE00C9BEA8 /* SentryReplayFrame.swift */, D451ED5C2D92ECD200C9BEA8 /* SentryOnDemandReplayError.swift */, D802994F2BA83A88000F0081 /* SentryPixelBuffer.swift */, D8AFC03C2BDA79BF00118BE1 /* SentryReplayVideoMaker.swift */, D8F67B1A2BE9728600C9197B /* SentrySRDefaultBreadcrumbConverter.swift */, + D49480D62DC23FE200A3B6E9 /* SentrySessionReplayDelegate.swift */, D81988BF2BEBFFF70020E36C /* SentryReplayRecording.swift */, D8BC28C72BFF5EBB0054DA4D /* SentryTouchTracker.swift */, D84D2CC22C29AD120011AF8A /* SentrySessionReplay.swift */, @@ -5054,6 +5063,7 @@ 63FE711D20DA4C1000CDBAE8 /* SentryCrashCPU_arm64.c in Sources */, 844EDC77294144DB00C86F34 /* SentrySystemWrapper.mm in Sources */, D451ED5F2D92ECDE00C9BEA8 /* SentryReplayFrame.swift in Sources */, + D49480D72DC23FE300A3B6E9 /* SentrySessionReplayDelegate.swift in Sources */, D8739CF92BECFFB5007D2F66 /* SentryTransactionNameSource.swift in Sources */, 630435FF1EBCA9D900C4D3FA /* SentryNSURLRequest.m in Sources */, 6281C5722D3E4F12009D0978 /* DecodeArbitraryData.swift in Sources */, @@ -5116,6 +5126,7 @@ 63FE715F20DA4C1100CDBAE8 /* SentryCrashID.c in Sources */, 84B0E0072CD963FD007FB332 /* SentryIconography.swift in Sources */, 7DB3A687238EA75E00A2D442 /* SentryHttpTransport.m in Sources */, + D49480D52DC23FD500A3B6E9 /* SessionReplayError.swift in Sources */, 63FE70D520DA4C1000CDBAE8 /* SentryCrashMonitor_NSException.m in Sources */, D80CD8D12B751442002F710B /* HTTPHeaderSanitizer.swift in Sources */, 62F70E952D423BCD00634054 /* SentryMechanismCodable.swift in Sources */, @@ -5331,6 +5342,7 @@ 7B3B473E25D6CEA500D01640 /* SentryNSErrorTests.swift in Sources */, 632331F62404FFA8008D91D6 /* SentryScopeTests.m in Sources */, D808FB88281AB33C009A2A33 /* SentryUIEventTrackerTests.swift in Sources */, + D49480D32DC23E9300A3B6E9 /* SentryReplayTypeTests.swift in Sources */, D8F8F5572B835BC600AC5465 /* SentryMsgPackSerializerTests.m in Sources */, 0A283E79291A67E000EF4126 /* SentryUIDeviceWrapperTests.swift in Sources */, 63FE720D20DA66EC00CDBAE8 /* SentryCrashNSErrorUtilTests.m in Sources */, diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index db5e332d776..83991ef88eb 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -64,8 +64,8 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { /// /// - Parameter path: The path to the directory containing the frames. private func loadFrames(fromPath path: String) { - SentryLog.debug("[Session Replay] Loading frames from path: \(path)") do { + SentryLog.debug("[Session Replay] Loading frames from path: \(path)") let content = try FileManager.default.contentsOfDirectory(atPath: path) _frames = content.compactMap { frameFilePath -> SentryReplayFrame? in guard frameFilePath.hasSuffix(".png") else { return nil } @@ -75,11 +75,12 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { }.sorted { $0.time < $1.time } SentryLog.debug("[Session Replay] Loaded \(content.count) files into \(_frames.count) frames from path: \(path)") } catch { - SentryLog.error("[Session Replay] Could not list frames from replay: \(error.localizedDescription)") + SentryLog.error("[Session Replay] Could not list frames from replay, reason: \(error.localizedDescription)") } } func addFrameAsync(image: UIImage, forScreen: String?) { + SentryLog.debug("[Session Replay] Adding frame async for screen: \(forScreen ?? "nil")") // Dispatch the frame addition to a background queue to avoid blocking the main queue. // This must be on the processing queue to avoid deadlocks. processingQueue.dispatchAsync { @@ -88,14 +89,20 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } private func addFrame(image: UIImage, forScreen: String?) { - guard let data = rescaleImage(image)?.pngData() else { return } - + SentryLog.debug("[Session Replay] Adding frame for screen: \(forScreen ?? "nil")") + guard let data = rescaleImage(image)?.pngData() else { + SentryLog.warning("[Session Replay] Could not rescale image to PNG data, ignoring frame") + return + } + let date = dateProvider.date() let imagePath = (_outputPath as NSString).appendingPathComponent("\(date.timeIntervalSinceReferenceDate).png") do { - try data.write(to: URL(fileURLWithPath: imagePath)) + let url = URL(fileURLWithPath: imagePath) + SentryLog.debug("[Session Replay] Saving replay frame to: \(url.path)") + try data.write(to: url) } catch { - SentryLog.error("[Session Replay] Could not save replay frame. Error: \(error)") + SentryLog.error("[Session Replay] Could not save replay frame, reason: \(error)") return } _frames.append(SentryReplayFrame(imagePath: imagePath, time: date, screenName: forScreen)) @@ -103,9 +110,12 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // Remove the oldest frames if the cache size exceeds the maximum size. while _frames.count > cacheMaxSize { let first = _frames.removeFirst() - try? FileManager.default.removeItem(at: URL(fileURLWithPath: first.imagePath)) + let url = URL(fileURLWithPath: first.imagePath) + SentryLog.debug("[Session Replay] Removing frame at url: \(url.path)") + try? FileManager.default.removeItem(at: url) } _totalFrames += 1 + SentryLog.debug("[Session Replay] Added frame, total frames counter: \(_totalFrames), current frames count: \(_frames.count)") } private func rescaleImage(_ originalImage: UIImage) -> UIImage? { @@ -179,6 +189,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // Set the frame count/offset to the new index that is returned by the completion block. // This is important to avoid processing the same frame multiple times. frameCount = videoResult.finalFrameIndex + SentryLog.debug("[Session Replay] Finished rendering video, frame count moved to: \(frameCount)") // Append the video info to the videos array. // In case no video info is returned, skip the segment. @@ -203,11 +214,14 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // If there was an error, throw it to exit the loop. if let error = currentError { + SentryLog.error("[Session Replay] Error while rendering video: \(error), cancelling video segment creation") throw error } SentryLog.debug("[Session Replay] Finished rendering video, frame count moved to: \(frameCount)") } + + SentryLog.debug("[Session Replay] Finished creating video with \(videos.count) segments") return videos } diff --git a/Sources/Swift/Integrations/SessionReplay/SentryReplayType.swift b/Sources/Swift/Integrations/SessionReplay/SentryReplayType.swift index 32579786b2a..c7f0c8f55f8 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryReplayType.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryReplayType.swift @@ -6,6 +6,14 @@ enum SentryReplayType: Int { case buffer } +// Implementing the CustomStringConvertible protocol to provide a string representation of the enum values. +// This method will be called by the Swift runtime when converting the enum to a string, i.e. in String interpolations. +extension SentryReplayType: CustomStringConvertible { + var description: String { + return toString() + } +} + extension SentryReplayType { func toString() -> String { switch self { diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift index 82e2396e01f..1334323052a 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift @@ -3,20 +3,7 @@ import Foundation @_implementationOnly import _SentryPrivate import UIKit -enum SessionReplayError: Error { - case cantCreateReplayDirectory - case noFramesAvailable -} - -@objc -protocol SentrySessionReplayDelegate: NSObjectProtocol { - func sessionReplayShouldCaptureReplayForError() -> Bool - func sessionReplayNewSegment(replayEvent: SentryReplayEvent, replayRecording: SentryReplayRecording, videoUrl: URL) - func sessionReplayStarted(replayId: SentryId) - func breadcrumbsForSessionReplay() -> [Breadcrumb] - func currentScreenNameForSessionReplay() -> String? -} - +// swiftlint:disable type_body_length @objcMembers class SentrySessionReplay: NSObject { private(set) var isFullSession = false @@ -243,7 +230,10 @@ class SentrySessionReplay: NSObject { private func newSegmentAvailable(videoInfo: SentryVideoInfo, replayType: SentryReplayType) { SentryLog.debug("[Session Replay] New segment available for replayType: \(replayType), videoInfo: \(videoInfo)") - guard let sessionReplayId = sessionReplayId else { return } + guard let sessionReplayId = sessionReplayId else { + SentryLog.warning("[Session Replay] No session replay ID available, ignoring segment.") + return + } captureSegment(segment: currentSegmentId, video: videoInfo, replayId: sessionReplayId, replayType: replayType) replayMaker.releaseFramesUntil(videoInfo.end) videoSegmentStart = videoInfo.end @@ -329,5 +319,6 @@ class SentrySessionReplay: NSObject { } } } +// swiftlint:enable type_body_length #endif diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplayDelegate.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplayDelegate.swift new file mode 100644 index 00000000000..23048bace74 --- /dev/null +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplayDelegate.swift @@ -0,0 +1,14 @@ +import Foundation +#if (os(iOS) || os(tvOS)) && !SENTRY_NO_UIKIT +@_implementationOnly import _SentryPrivate + +@objc +protocol SentrySessionReplayDelegate: NSObjectProtocol { + func sessionReplayShouldCaptureReplayForError() -> Bool + func sessionReplayNewSegment(replayEvent: SentryReplayEvent, replayRecording: SentryReplayRecording, videoUrl: URL) + func sessionReplayStarted(replayId: SentryId) + func breadcrumbsForSessionReplay() -> [Breadcrumb] + func currentScreenNameForSessionReplay() -> String? +} + +#endif diff --git a/Sources/Swift/Integrations/SessionReplay/SessionReplayError.swift b/Sources/Swift/Integrations/SessionReplay/SessionReplayError.swift new file mode 100644 index 00000000000..d79fa500cef --- /dev/null +++ b/Sources/Swift/Integrations/SessionReplay/SessionReplayError.swift @@ -0,0 +1,4 @@ +enum SessionReplayError: Error { + case cantCreateReplayDirectory + case noFramesAvailable +} diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentryReplayTypeTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentryReplayTypeTests.swift new file mode 100644 index 00000000000..546f01b4859 --- /dev/null +++ b/Tests/SentryTests/Integrations/SessionReplay/SentryReplayTypeTests.swift @@ -0,0 +1,34 @@ +@testable import Sentry +import XCTest + +class SentryReplayTypeTests: XCTestCase { + func testRawValue_bufferMode_shouldBeCorrect() { + let buffer = SentryReplayType.buffer.rawValue + XCTAssertEqual(buffer, 0) + } + + func testRawValue_sessionMode_shouldBeCorrect() { + let replay = SentryReplayType.session.rawValue + XCTAssertEqual(replay, 1) + } + + func testDescription_bufferMode_shouldBeCorrect() { + let replay = SentryReplayType.session.description + XCTAssertEqual(replay, "buffer") + } + + func testDescription_sessionMode_shouldBeCorrect() { + let replay = SentryReplayType.session.description + XCTAssertEqual(replay, "session") + } + + func testToString_bufferMode_shouldBeCorrect() { + let replay = SentryReplayType.buffer + XCTAssertEqual(replay.toString(), "buffer") + } + + func testToString_sessionMode_shouldBeCorrect() { + let replay = SentryReplayType.session + XCTAssertEqual(replay.toString(), "session") + } +} From e1ef89dd59284568d6b214f113f69f76134c20f3 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Wed, 30 Apr 2025 13:28:05 +0200 Subject: [PATCH 17/26] removed unused type --- Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift | 2 +- Sentry.xcodeproj/project.pbxproj | 6 +----- .../Integrations/SessionReplay/SessionReplayError.swift | 4 ---- 3 files changed, 2 insertions(+), 10 deletions(-) delete mode 100644 Sources/Swift/Integrations/SessionReplay/SessionReplayError.swift diff --git a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift index c4aba0df64e..4695f86e2d4 100644 --- a/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift +++ b/Samples/iOS-Swift/iOS-Swift/SentrySDKWrapper.swift @@ -29,7 +29,7 @@ struct SentrySDKWrapper { if #available(iOS 16.0, *), !SentrySDKOverrides.Other.disableSessionReplay.boolValue { options.sessionReplay = SentryReplayOptions( - sessionSampleRate: 1, + sessionSampleRate: 0, onErrorSampleRate: 1, maskAllText: true, maskAllImages: true diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index e2bd4f19e62..bcf19bc6fe0 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -836,7 +836,6 @@ D48E8B8B2D3E79610032E35E /* SentryTraceOrigin.swift in Sources */ = {isa = PBXBuildFile; fileRef = D48E8B8A2D3E79610032E35E /* SentryTraceOrigin.swift */; }; D48E8B9D2D3E82AC0032E35E /* SentrySpanOperation.swift in Sources */ = {isa = PBXBuildFile; fileRef = D48E8B9C2D3E82AC0032E35E /* SentrySpanOperation.swift */; }; D49480D32DC23E9300A3B6E9 /* SentryReplayTypeTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D49480D22DC23E8E00A3B6E9 /* SentryReplayTypeTests.swift */; }; - D49480D52DC23FD500A3B6E9 /* SessionReplayError.swift in Sources */ = {isa = PBXBuildFile; fileRef = D49480D42DC23FD500A3B6E9 /* SessionReplayError.swift */; }; D49480D72DC23FE300A3B6E9 /* SentrySessionReplayDelegate.swift in Sources */ = {isa = PBXBuildFile; fileRef = D49480D62DC23FE200A3B6E9 /* SentrySessionReplayDelegate.swift */; }; D4AF00212D2E92FD00F5F3D7 /* SentryNSFileManagerSwizzling.m in Sources */ = {isa = PBXBuildFile; fileRef = D4AF00202D2E92FD00F5F3D7 /* SentryNSFileManagerSwizzling.m */; }; D4AF00232D2E931000F5F3D7 /* SentryNSFileManagerSwizzling.h in Headers */ = {isa = PBXBuildFile; fileRef = D4AF00222D2E931000F5F3D7 /* SentryNSFileManagerSwizzling.h */; }; @@ -2014,7 +2013,6 @@ D48E8B8A2D3E79610032E35E /* SentryTraceOrigin.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryTraceOrigin.swift; sourceTree = ""; }; D48E8B9C2D3E82AC0032E35E /* SentrySpanOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySpanOperation.swift; sourceTree = ""; }; D49480D22DC23E8E00A3B6E9 /* SentryReplayTypeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryReplayTypeTests.swift; sourceTree = ""; }; - D49480D42DC23FD500A3B6E9 /* SessionReplayError.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SessionReplayError.swift; sourceTree = ""; }; D49480D62DC23FE200A3B6E9 /* SentrySessionReplayDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySessionReplayDelegate.swift; sourceTree = ""; }; D4A236012D5F838200D55C58 /* macOS-Swift_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "macOS-Swift_Base.xctestplan"; sourceTree = ""; }; D4A236062D5F846200D55C58 /* iOS-ObjectiveC_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "iOS-ObjectiveC_Base.xctestplan"; sourceTree = ""; }; @@ -4289,17 +4287,16 @@ D8CAC02A2BA0663E00E38F34 /* SentryReplayOptions.swift */, D8CAC02B2BA0663E00E38F34 /* SentryVideoInfo.swift */, D802994D2BA836EF000F0081 /* SentryOnDemandReplay.swift */, - D49480D42DC23FD500A3B6E9 /* SessionReplayError.swift */, D4B0DC7E2DA9257200DE61B6 /* SentryRenderVideoResult.swift */, D451ED5E2D92ECDE00C9BEA8 /* SentryReplayFrame.swift */, D451ED5C2D92ECD200C9BEA8 /* SentryOnDemandReplayError.swift */, D802994F2BA83A88000F0081 /* SentryPixelBuffer.swift */, D8AFC03C2BDA79BF00118BE1 /* SentryReplayVideoMaker.swift */, D8F67B1A2BE9728600C9197B /* SentrySRDefaultBreadcrumbConverter.swift */, - D49480D62DC23FE200A3B6E9 /* SentrySessionReplayDelegate.swift */, D81988BF2BEBFFF70020E36C /* SentryReplayRecording.swift */, D8BC28C72BFF5EBB0054DA4D /* SentryTouchTracker.swift */, D84D2CC22C29AD120011AF8A /* SentrySessionReplay.swift */, + D49480D62DC23FE200A3B6E9 /* SentrySessionReplayDelegate.swift */, D84D2CDC2C2BF7370011AF8A /* SentryReplayEvent.swift */, D84D2CDE2C2BF9370011AF8A /* SentryReplayType.swift */, ); @@ -5126,7 +5123,6 @@ 63FE715F20DA4C1100CDBAE8 /* SentryCrashID.c in Sources */, 84B0E0072CD963FD007FB332 /* SentryIconography.swift in Sources */, 7DB3A687238EA75E00A2D442 /* SentryHttpTransport.m in Sources */, - D49480D52DC23FD500A3B6E9 /* SessionReplayError.swift in Sources */, 63FE70D520DA4C1000CDBAE8 /* SentryCrashMonitor_NSException.m in Sources */, D80CD8D12B751442002F710B /* HTTPHeaderSanitizer.swift in Sources */, 62F70E952D423BCD00634054 /* SentryMechanismCodable.swift in Sources */, diff --git a/Sources/Swift/Integrations/SessionReplay/SessionReplayError.swift b/Sources/Swift/Integrations/SessionReplay/SessionReplayError.swift deleted file mode 100644 index d79fa500cef..00000000000 --- a/Sources/Swift/Integrations/SessionReplay/SessionReplayError.swift +++ /dev/null @@ -1,4 +0,0 @@ -enum SessionReplayError: Error { - case cantCreateReplayDirectory - case noFramesAvailable -} From d723f27921e40c72e725f19408e61de1e17f2478 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Wed, 30 Apr 2025 14:03:31 +0200 Subject: [PATCH 18/26] fix tests --- .../SessionReplay/SentryReplayTypeTests.swift | 34 +++++++++---------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentryReplayTypeTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentryReplayTypeTests.swift index 546f01b4859..3e0a7ab7b3d 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentryReplayTypeTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentryReplayTypeTests.swift @@ -2,33 +2,33 @@ import XCTest class SentryReplayTypeTests: XCTestCase { - func testRawValue_bufferMode_shouldBeCorrect() { - let buffer = SentryReplayType.buffer.rawValue - XCTAssertEqual(buffer, 0) - } - func testRawValue_sessionMode_shouldBeCorrect() { - let replay = SentryReplayType.session.rawValue - XCTAssertEqual(replay, 1) + let replayType = SentryReplayType.session.rawValue + XCTAssertEqual(replayType, 0) } - func testDescription_bufferMode_shouldBeCorrect() { - let replay = SentryReplayType.session.description - XCTAssertEqual(replay, "buffer") + func testRawValue_bufferMode_shouldBeCorrect() { + let replayType = SentryReplayType.buffer.rawValue + XCTAssertEqual(replayType, 1) } func testDescription_sessionMode_shouldBeCorrect() { - let replay = SentryReplayType.session.description - XCTAssertEqual(replay, "session") + let replayType = SentryReplayType.session.description + XCTAssertEqual(replayType, "session") } - func testToString_bufferMode_shouldBeCorrect() { - let replay = SentryReplayType.buffer - XCTAssertEqual(replay.toString(), "buffer") + func testDescription_bufferMode_shouldBeCorrect() { + let replayType = SentryReplayType.buffer.description + XCTAssertEqual(replayType, "buffer") } func testToString_sessionMode_shouldBeCorrect() { - let replay = SentryReplayType.session - XCTAssertEqual(replay.toString(), "session") + let replayType = SentryReplayType.session + XCTAssertEqual(replayType.toString(), "session") + } + + func testToString_bufferMode_shouldBeCorrect() { + let replayType = SentryReplayType.buffer + XCTAssertEqual(replayType.toString(), "buffer") } } From 01ea9652d99890be7be28e0c81ed77aeb83383c8 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 15 May 2025 13:58:44 +0200 Subject: [PATCH 19/26] update changelog --- CHANGELOG.md | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b6f6720086d..6e937054434 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Fix thread inversion warning in session replay (#5018) + ## 8.50.2 ### Fixes @@ -41,10 +47,6 @@ - Correctly rate limit envelopes from the new UI profiling system (#5131) - Race condition in ANRTrackerV1 (#5137) -### Fixes - -- Fix thread inversion warning in session replay (#5018) - ### Improvements - More logging for Session Replay video info (#5132) From fd16c22008775e179de991e339b5660f95c5a6c6 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Thu, 15 May 2025 15:16:10 +0200 Subject: [PATCH 20/26] WIP --- Sentry.xcodeproj/project.pbxproj | 17 +++-- .../xcshareddata/xcschemes/Sentry.xcscheme | 73 +++++++++++++++++++ Sources/Sentry/SentryDependencyContainer.m | 2 + Sources/Sentry/SentryDispatchQueueProvider.m | 13 ++++ Sources/Sentry/SentryDispatchQueueWrapper.m | 8 -- .../Sentry/SentrySessionReplayIntegration.m | 24 ++++-- .../HybridPublic/SentryDependencyContainer.h | 2 + .../include/SentryDispatchQueueProvider.h | 10 +++ .../SentryDispatchQueueProviderProtocol.h | 22 ++++++ .../include/SentryDispatchQueueWrapper.h | 2 - .../SessionReplay/SentryOnDemandReplay.swift | 25 ++++--- .../SessionReplay/SentryReplayType.swift | 10 +-- .../SessionReplay/SentrySessionReplay.swift | 2 +- 13 files changed, 167 insertions(+), 43 deletions(-) create mode 100644 Sources/Sentry/SentryDispatchQueueProvider.m create mode 100644 Sources/Sentry/include/SentryDispatchQueueProvider.h create mode 100644 Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index d6efc9deb0c..de84ef6d4ea 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -818,6 +818,9 @@ A8F17B2E2901765900990B25 /* SentryRequest.m in Sources */ = {isa = PBXBuildFile; fileRef = A8F17B2D2901765900990B25 /* SentryRequest.m */; }; A8F17B342902870300990B25 /* SentryHttpStatusCodeRange.m in Sources */ = {isa = PBXBuildFile; fileRef = A8F17B332902870300990B25 /* SentryHttpStatusCodeRange.m */; }; D4009EB22D771BC20007AF30 /* SentryFileIOTrackerSwiftHelpersTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4009EB12D771BB90007AF30 /* SentryFileIOTrackerSwiftHelpersTests.swift */; }; + D4291A612DD61A1400772088 /* SentryDispatchQueueProvider.m in Sources */ = {isa = PBXBuildFile; fileRef = D4291A602DD61A1400772088 /* SentryDispatchQueueProvider.m */; }; + D4291A682DD61A3F00772088 /* SentryDispatchQueueProvider.h in Headers */ = {isa = PBXBuildFile; fileRef = D4291A662DD61A3F00772088 /* SentryDispatchQueueProvider.h */; }; + D4291A692DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h in Headers */ = {isa = PBXBuildFile; fileRef = D4291A672DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h */; }; D42E48572D48DF1600D251BC /* SentryBuildAppStartSpansTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D42E48562D48DF1600D251BC /* SentryBuildAppStartSpansTests.swift */; }; D43647F12D5CFB71001468E0 /* SentrySpanKeyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43647F02D5CFB71001468E0 /* SentrySpanKeyTests.swift */; }; D43647F32D5CFBC7001468E0 /* FileManagerTracingIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43647F22D5CFBC2001468E0 /* FileManagerTracingIntegrationTests.swift */; }; @@ -1993,6 +1996,9 @@ D4009EB12D771BB90007AF30 /* SentryFileIOTrackerSwiftHelpersTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryFileIOTrackerSwiftHelpersTests.swift; sourceTree = ""; }; D41909922D48FFF6002B83D0 /* SentryNSDictionarySanitize+Tests.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "SentryNSDictionarySanitize+Tests.h"; sourceTree = ""; }; D41909942D490006002B83D0 /* SentryNSDictionarySanitize+Tests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "SentryNSDictionarySanitize+Tests.m"; sourceTree = ""; }; + D4291A602DD61A1400772088 /* SentryDispatchQueueProvider.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryDispatchQueueProvider.m; sourceTree = ""; }; + D4291A662DD61A3F00772088 /* SentryDispatchQueueProvider.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDispatchQueueProvider.h; path = include/SentryDispatchQueueProvider.h; sourceTree = ""; }; + D4291A672DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDispatchQueueProviderProtocol.h; path = include/SentryDispatchQueueProviderProtocol.h; sourceTree = ""; }; D42E48562D48DF1600D251BC /* SentryBuildAppStartSpansTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryBuildAppStartSpansTests.swift; sourceTree = ""; }; D42E48582D48FC8F00D251BC /* SentryNSDictionarySanitizeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryNSDictionarySanitizeTests.swift; sourceTree = ""; }; D43647F02D5CFB71001468E0 /* SentrySpanKeyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SentrySpanKeyTests.swift; sourceTree = ""; }; @@ -2021,11 +2027,6 @@ D48E8B9C2D3E82AC0032E35E /* SentrySpanOperation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySpanOperation.swift; sourceTree = ""; }; D49480D22DC23E8E00A3B6E9 /* SentryReplayTypeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryReplayTypeTests.swift; sourceTree = ""; }; D49480D62DC23FE200A3B6E9 /* SentrySessionReplayDelegate.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentrySessionReplayDelegate.swift; sourceTree = ""; }; - D4A236012D5F838200D55C58 /* macOS-Swift_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "macOS-Swift_Base.xctestplan"; sourceTree = ""; }; - D4A236062D5F846200D55C58 /* iOS-ObjectiveC_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "iOS-ObjectiveC_Base.xctestplan"; sourceTree = ""; }; - D4A236072D5F846F00D55C58 /* iOS-Swift6_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "iOS-Swift6_Base.xctestplan"; sourceTree = ""; }; - D4A236082D5F847800D55C58 /* iOS15-SwiftUI.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "iOS15-SwiftUI.xctestplan"; sourceTree = ""; }; - D4A236092D5F84CF00D55C58 /* macOS-SwiftUI_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = "macOS-SwiftUI_Base.xctestplan"; sourceTree = ""; }; D4A2360A2D5F84FA00D55C58 /* SwiftUITestSample_Base.xctestplan */ = {isa = PBXFileReference; lastKnownFileType = text; path = SwiftUITestSample_Base.xctestplan; sourceTree = ""; }; D4AF00202D2E92FD00F5F3D7 /* SentryNSFileManagerSwizzling.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryNSFileManagerSwizzling.m; sourceTree = ""; }; D4AF00222D2E931000F5F3D7 /* SentryNSFileManagerSwizzling.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryNSFileManagerSwizzling.h; path = include/SentryNSFileManagerSwizzling.h; sourceTree = ""; }; @@ -2426,6 +2427,9 @@ 638DC99F1EBC6B6400A66E41 /* SentryRequestOperation.m */, 7BDB03B6251364F800BAE198 /* SentryDispatchQueueWrapper.h */, 7BDB03BA2513652900BAE198 /* SentryDispatchQueueWrapper.m */, + D4291A672DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h */, + D4291A662DD61A3F00772088 /* SentryDispatchQueueProvider.h */, + D4291A602DD61A1400772088 /* SentryDispatchQueueProvider.m */, 84AC61D029F7541E009EEF61 /* SentryDispatchSourceWrapper.h */, 84AC61D429F75A98009EEF61 /* SentryDispatchFactory.h */, 84AC61D529F75A98009EEF61 /* SentryDispatchFactory.m */, @@ -4379,6 +4383,8 @@ 84DEE86B2B686BD400A7BC17 /* SentrySamplerDecision.h in Headers */, 7B4E23BE251A2BD500060D68 /* SentryCrashIntegrationSessionHandler.h in Headers */, 7B88F2FE24BC5A4C00ADF90A /* SentrySdkInfo.h in Headers */, + D4291A682DD61A3F00772088 /* SentryDispatchQueueProvider.h in Headers */, + D4291A692DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h in Headers */, 7BCFBD672681C95000BC27D8 /* SentryScopeObserver.h in Headers */, D81A346C291AECC7005A27A9 /* PrivateSentrySDKOnly.h in Headers */, D88817DA26D72AB800BF2251 /* SentryTraceContext.h in Headers */, @@ -5198,6 +5204,7 @@ 7D65260E237F649E00113EA2 /* SentryScope.m in Sources */, D4EDF9842D0B2A210071E7B3 /* Data+SentryTracing.swift in Sources */, 84281C472A57905700EE88F2 /* SentrySample.m in Sources */, + D4291A612DD61A1400772088 /* SentryDispatchQueueProvider.m in Sources */, 84AC61D329F7541E009EEF61 /* SentryDispatchSourceWrapper.m in Sources */, 62A456E52B0370E0003F19A1 /* SentryUIEventTrackerTransactionMode.m in Sources */, D4E829D62D75E39B00D375AD /* SentryViewRenderer.swift in Sources */, diff --git a/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme b/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme index 53783b37f22..79c7c938423 100644 --- a/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme +++ b/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme @@ -60,6 +60,74 @@ BlueprintName = "SentryTests" ReferencedContainer = "container:Sentry.xcodeproj"> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -70,6 +138,11 @@ BlueprintName = "SentryProfilerTests" ReferencedContainer = "container:Sentry.xcodeproj"> + + + + diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m index b4834f35e87..3e716cfd09a 100644 --- a/Sources/Sentry/SentryDependencyContainer.m +++ b/Sources/Sentry/SentryDependencyContainer.m @@ -2,6 +2,7 @@ #import "SentryBinaryImageCache.h" #import "SentryDispatchFactory.h" +#import "SentryDispatchQueueProvider.h" #import "SentryDispatchQueueWrapper.h" #import "SentryDisplayLinkWrapper.h" #import "SentryExtraContextProvider.h" @@ -96,6 +97,7 @@ + (void)reset - (instancetype)init { if (self = [super init]) { + _dispatchQueueProvider = [[SentryDispatchQueueProvider alloc] init]; _dispatchQueueWrapper = [[SentryDispatchQueueWrapper alloc] init]; _random = [[SentryRandom alloc] init]; _threadWrapper = [[SentryThreadWrapper alloc] init]; diff --git a/Sources/Sentry/SentryDispatchQueueProvider.m b/Sources/Sentry/SentryDispatchQueueProvider.m new file mode 100644 index 00000000000..c75d35c9c05 --- /dev/null +++ b/Sources/Sentry/SentryDispatchQueueProvider.m @@ -0,0 +1,13 @@ +#import "SentryDispatchQueueProvider.h" + +@implementation SentryDispatchQueueProvider + +- (SentryDispatchQueueWrapper *)createBackgroundQueueWithName:(const char *)name + relativePriority:(int)relativePriority +{ + dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( + DISPATCH_QUEUE_SERIAL, QOS_CLASS_BACKGROUND, relativePriority); + return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; +} + +@end diff --git a/Sources/Sentry/SentryDispatchQueueWrapper.m b/Sources/Sentry/SentryDispatchQueueWrapper.m index aa208253a37..2663c12f49e 100644 --- a/Sources/Sentry/SentryDispatchQueueWrapper.m +++ b/Sources/Sentry/SentryDispatchQueueWrapper.m @@ -105,14 +105,6 @@ - (nullable dispatch_block_t)createDispatchBlock:(void (^)(void))block return dispatch_block_create(0, block); } -+ (SentryDispatchQueueWrapper *)createBackgroundDispatchQueueWithName:(const char *)name - relativePriority:(int)relativePriority - -{ - dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( - DISPATCH_QUEUE_SERIAL, QOS_CLASS_BACKGROUND, relativePriority); - return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; -} @end NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/SentrySessionReplayIntegration.m b/Sources/Sentry/SentrySessionReplayIntegration.m index 06bf101c109..b4e2a6c7896 100644 --- a/Sources/Sentry/SentrySessionReplayIntegration.m +++ b/Sources/Sentry/SentrySessionReplayIntegration.m @@ -5,6 +5,7 @@ # import "SentryClient+Private.h" # import "SentryCrashWrapper.h" # import "SentryDependencyContainer.h" +# import "SentryDispatchQueueProviderProtocol.h" # import "SentryDispatchQueueWrapper.h" # import "SentryDisplayLinkWrapper.h" # import "SentryEvent+Private.h" @@ -130,17 +131,22 @@ - (void)setupWith:(SentryReplayOptions *)replayOptions _notificationCenter = SentryDependencyContainer.sharedInstance.notificationCenterWrapper; _dateProvider = SentryDependencyContainer.sharedInstance.dateProvider; + // We use the dispatch queue provider as a factory to create the queues, but store the queues + // directly in this instance, so they get deallocated when the integration is deallocated. + id dispatchQueueProvider + = SentryDependencyContainer.sharedInstance.dispatchQueueProvider; + // The asset worker queue is used to work on video and frames data. // Use a relative priority of -1 to make it lower than the default background priority. - _replayAssetWorkerQueue = [SentryDispatchQueueWrapper - createBackgroundDispatchQueueWithName:"io.sentry.session-replay.asset-worker" - relativePriority:-1]; + _replayAssetWorkerQueue = + [dispatchQueueProvider createBackgroundQueueWithName:"io.sentry.session-replay.asset-worker" + relativePriority:-1]; // The dispatch queue is used to asynchronously wait for the asset worker queue to finish its // work. To avoid a deadlock, the priority of the processing queue must be lower than the asset // worker queue. Use a relative priority of -2 to make it lower than the asset worker queue. - _replayProcessingQueue = [SentryDispatchQueueWrapper - createBackgroundDispatchQueueWithName:"io.sentry.session-replay.processing" - relativePriority:-2]; + _replayProcessingQueue = + [dispatchQueueProvider createBackgroundQueueWithName:"io.sentry.session-replay.processing" + relativePriority:-2]; // The asset worker queue is used to work on video and frames data. @@ -243,13 +249,15 @@ - (void)resumePreviousSessionReplay:(SentryEvent *)event // Either error or videos should be set. if (error != nil) { - SENTRY_LOG_ERROR(@"Could not create replay video, reason: %@", error); + SENTRY_LOG_ERROR(@"[Session Replay] Could not create replay video, reason: %@", error); return; } if (videos == nil) { - SENTRY_LOG_ERROR(@"Could not create replay video, reason: no videos available"); + SENTRY_LOG_ERROR( + @"[Session Replay] Could not create replay video, reason: no videos available"); return; } + SENTRY_LOG_DEBUG(@"[Session Replay] Created replay with %lu video segments", videos.count); // For each segment we need to create a new event with the video. int _segmentId = segmentId; diff --git a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h index d92fe16a723..fb719369930 100644 --- a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h +++ b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h @@ -26,6 +26,7 @@ @protocol SentryRandom; @protocol SentryCurrentDateProvider; @protocol SentryRateLimits; +@protocol SentryDispatchQueueProviderProtocol; #if SENTRY_HAS_METRIC_KIT @class SentryMXManager; @@ -66,6 +67,7 @@ SENTRY_NO_INIT @property (nonatomic, strong) SentryThreadWrapper *threadWrapper; @property (nonatomic, strong) id random; @property (nonatomic, strong) SentrySwizzleWrapper *swizzleWrapper; +@property (nonatomic, strong) id dispatchQueueProvider; @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueueWrapper; @property (nonatomic, strong) SentryNSNotificationCenterWrapper *notificationCenterWrapper; @property (nonatomic, strong) SentryDebugImageProvider *debugImageProvider; diff --git a/Sources/Sentry/include/SentryDispatchQueueProvider.h b/Sources/Sentry/include/SentryDispatchQueueProvider.h new file mode 100644 index 00000000000..4786df2bf18 --- /dev/null +++ b/Sources/Sentry/include/SentryDispatchQueueProvider.h @@ -0,0 +1,10 @@ +#import "SentryDispatchQueueProviderProtocol.h" +#import + +NS_ASSUME_NONNULL_BEGIN + +@interface SentryDispatchQueueProvider : NSObject + +@end + +NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h b/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h new file mode 100644 index 00000000000..7944f8eca9c --- /dev/null +++ b/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h @@ -0,0 +1,22 @@ +#import "SentryDispatchQueueWrapper.h" +#import + +NS_ASSUME_NONNULL_BEGIN + +@protocol SentryDispatchQueueProviderProtocol + +/** + * Creates a background queue with the given name and relative priority. + * + * @note This method is only a factory method and does not keep a reference to the created queue. + * + * @param name The name of the queue. + * @param relativePriority The priority of the queue relative to the background QoS. + * @return Unretained reference to the created queue. + */ +- (SentryDispatchQueueWrapper *)createBackgroundQueueWithName:(const char *)name + relativePriority:(int)relativePriority; + +@end + +NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentryDispatchQueueWrapper.h b/Sources/Sentry/include/SentryDispatchQueueWrapper.h index 56748389cde..7859c74ac55 100644 --- a/Sources/Sentry/include/SentryDispatchQueueWrapper.h +++ b/Sources/Sentry/include/SentryDispatchQueueWrapper.h @@ -32,8 +32,6 @@ NS_ASSUME_NONNULL_BEGIN - (nullable dispatch_block_t)createDispatchBlock:(void (^)(void))block; -+ (SentryDispatchQueueWrapper *)createBackgroundDispatchQueueWithName:(const char *)name - relativePriority:(int)relativePriority; @end NS_ASSUME_NONNULL_END diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 508e9180ea2..c200b2349ab 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -64,8 +64,8 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { /// /// - Parameter path: The path to the directory containing the frames. private func loadFrames(fromPath path: String) { + SentryLog.debug("[Session Replay] Loading frames from path: \(path)") do { - SentryLog.debug("[Session Replay] Loading frames from path: \(path)") let content = try FileManager.default.contentsOfDirectory(atPath: path) _frames = content.compactMap { frameFilePath -> SentryReplayFrame? in guard frameFilePath.hasSuffix(".png") else { return nil } @@ -91,7 +91,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { private func addFrame(image: UIImage, forScreen: String?) { SentryLog.debug("[Session Replay] Adding frame for screen: \(forScreen ?? "nil")") guard let data = rescaleImage(image)?.pngData() else { - SentryLog.warning("[Session Replay] Could not rescale image to PNG data, ignoring frame") + SentryLog.error("[Session Replay] Could not rescale image, dropping frame") return } @@ -99,7 +99,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { let imagePath = (_outputPath as NSString).appendingPathComponent("\(date.timeIntervalSinceReferenceDate).png") do { let url = URL(fileURLWithPath: imagePath) - SentryLog.debug("[Session Replay] Saving replay frame to: \(url.path)") + SentryLog.debug("[Session Replay] Saving frame to file URL: \(url)") try data.write(to: url) } catch { SentryLog.error("[Session Replay] Could not save replay frame, reason: \(error)") @@ -111,7 +111,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { while _frames.count > cacheMaxSize { let first = _frames.removeFirst() let url = URL(fileURLWithPath: first.imagePath) - SentryLog.debug("[Session Replay] Removing frame at url: \(url.path)") + SentryLog.debug("[Session Replay] Removing oldest frame at file URL: \(url.path)") try? FileManager.default.removeItem(at: url) } _totalFrames += 1 @@ -133,8 +133,8 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } func releaseFramesUntil(_ date: Date) { - SentryLog.debug("[Session Replay] Releasing frames until date: \(date)") processingQueue.dispatchAsync { + SentryLog.debug("[Session Replay] Releasing frames until date: \(date)") while let first = self._frames.first, first.time < date { self._frames.removeFirst() let fileUrl = URL(fileURLWithPath: first.imagePath) @@ -145,6 +145,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { SentryLog.error("[Session Replay] Failed to remove frame at: \(fileUrl.path), reason: \(error), ignoring error") } } + SentryLog.debug("[Session Replay] Frames released, remaining frames count: \(self._frames.count)") } } @@ -211,15 +212,15 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // It is imporant that the renderVideo completion block signals the group. // The queue used by render video must have a higher priority than the processing queue to reduce thread inversion. // Otherwise, it could lead to queue starvation and a deadlock/timeout. - guard group.wait(timeout: .now() + 120) == .success else { - SentryLog.error("[Session Replay] Timeout while waiting for video rendering to finish.") - throw SentryOnDemandReplayError.errorRenderingVideo + guard group.wait(timeout: .now() + 10) == .success else { + SentryLog.error("[Session Replay] Timeout while waiting for video rendering to finish, returning \(videos.count) videos") + return videos } // If there was an error, throw it to exit the loop. if let error = currentError { - SentryLog.error("[Session Replay] Error while rendering video: \(error), cancelling video segment creation") - throw error + SentryLog.error("[Session Replay] Error while rendering video: \(error), returning \(videos.count) videos") + return videos } SentryLog.debug("[Session Replay] Finished rendering video, frame count moved to: \(frameCount)") @@ -244,6 +245,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { let videoHeight = image.size.height * CGFloat(videoScale) let pixelSize = CGSize(width: videoWidth, height: videoHeight) + SentryLog.debug("[Session Replay] Creating video writer with output file URL: \(outputFileURL)") let videoWriter: AVAssetWriter do { videoWriter = try AVAssetWriter(url: outputFileURL, fileType: .mp4) @@ -314,6 +316,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { let frame = videoFrames[frameIndex] if let image = UIImage(contentsOfFile: frame.imagePath) { + SentryLog.debug("[Session Replay] Image is ready, size: \(image.size)") guard lastImageSize == image.size else { SentryLog.debug("[Session Replay] Image size changed, finishing the video") return strongSelf.finishVideo( @@ -355,7 +358,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { onCompletion completion: @escaping (Result) -> Void ) { // Note: This method is expected to be called from the asset worker queue and *not* the processing queue. - SentryLog.debug("[Session Replay] Finishing video with output file URL: \(outputFileURL.path)") + SentryLog.info("[Session Replay] Finishing video with output file URL: \(outputFileURL), used frames count: \(usedFrames.count), video height: \(videoHeight), video width: \(videoWidth)") videoWriter.inputs.forEach { $0.markAsFinished() } videoWriter.finishWriting { [weak self] in SentryLog.debug("[Session Replay] Finished video writing, status: \(videoWriter.status)") diff --git a/Sources/Swift/Integrations/SessionReplay/SentryReplayType.swift b/Sources/Swift/Integrations/SessionReplay/SentryReplayType.swift index c6df26ffa79..a933d8220c7 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryReplayType.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryReplayType.swift @@ -6,14 +6,6 @@ enum SentryReplayType: Int { case buffer } -// Implementing the CustomStringConvertible protocol to provide a string representation of the enum values. -// This method will be called by the Swift runtime when converting the enum to a string, i.e. in String interpolations. -extension SentryReplayType: CustomStringConvertible { - var description: String { - return toString() - } -} - extension SentryReplayType { func toString() -> String { switch self { @@ -23,6 +15,8 @@ extension SentryReplayType { } } +// Implementing the CustomStringConvertible protocol to provide a string representation of the enum values. +// This method will be called by the Swift runtime when converting the enum to a string, i.e. in String interpolations. extension SentryReplayType: CustomStringConvertible { var description: String { return toString() diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift index aab15d06371..d22674f55be 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift @@ -250,7 +250,7 @@ class SentrySessionReplay: NSObject { // Creating a video is computationally expensive, therefore perform it on a background queue. self.replayMaker.createVideoInBackgroundWith(beginning: startedAt, end: self.dateProvider.date()) { videos, error in if let error = error { - SentryLog.error("[Session Replay] Could not create replay video - \(error.localizedDescription)") + SentryLog.error("[Session Replay] Could not create replay video, reason: \(error)") return } guard let videos = videos else { From a065663380f57d21312f8a51d7a40f5d33c68876 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 16 May 2025 10:07:05 +0200 Subject: [PATCH 21/26] add dispatch queue provider --- Sentry.xcodeproj/project.pbxproj | 24 ++-- Sources/Sentry/SentryDependencyContainer.m | 9 +- Sources/Sentry/SentryDispatchFactory.m | 11 ++ Sources/Sentry/SentryDispatchQueueProvider.m | 13 -- Sources/Sentry/SentryDispatchSourceWrapper.m | 12 ++ .../Sentry/SentrySessionReplayIntegration.m | 6 +- .../HybridPublic/SentryDependencyContainer.h | 3 +- .../Sentry/include/SentryDispatchFactory.h | 20 +-- .../include/SentryDispatchQueueProvider.h | 10 -- .../SentryDispatchQueueProviderProtocol.h | 17 ++- .../SentryDispatchSourceProviderProtocol.h | 19 +++ .../include/SentryDispatchSourceWrapper.h | 5 + .../SentrySessionReplayIntegrationTests.swift | 17 +++ .../Networking/SentryDispatchFactoryTests.m | 135 ++++++++++++++++++ .../SentryDispatchQueueWrapperTests.m | 82 +++++++++++ 15 files changed, 326 insertions(+), 57 deletions(-) delete mode 100644 Sources/Sentry/SentryDispatchQueueProvider.m delete mode 100644 Sources/Sentry/include/SentryDispatchQueueProvider.h create mode 100644 Sources/Sentry/include/SentryDispatchSourceProviderProtocol.h create mode 100644 Tests/SentryTests/Networking/SentryDispatchFactoryTests.m create mode 100644 Tests/SentryTests/Networking/SentryDispatchQueueWrapperTests.m diff --git a/Sentry.xcodeproj/project.pbxproj b/Sentry.xcodeproj/project.pbxproj index de84ef6d4ea..0f9fa56894f 100644 --- a/Sentry.xcodeproj/project.pbxproj +++ b/Sentry.xcodeproj/project.pbxproj @@ -818,9 +818,8 @@ A8F17B2E2901765900990B25 /* SentryRequest.m in Sources */ = {isa = PBXBuildFile; fileRef = A8F17B2D2901765900990B25 /* SentryRequest.m */; }; A8F17B342902870300990B25 /* SentryHttpStatusCodeRange.m in Sources */ = {isa = PBXBuildFile; fileRef = A8F17B332902870300990B25 /* SentryHttpStatusCodeRange.m */; }; D4009EB22D771BC20007AF30 /* SentryFileIOTrackerSwiftHelpersTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4009EB12D771BB90007AF30 /* SentryFileIOTrackerSwiftHelpersTests.swift */; }; - D4291A612DD61A1400772088 /* SentryDispatchQueueProvider.m in Sources */ = {isa = PBXBuildFile; fileRef = D4291A602DD61A1400772088 /* SentryDispatchQueueProvider.m */; }; - D4291A682DD61A3F00772088 /* SentryDispatchQueueProvider.h in Headers */ = {isa = PBXBuildFile; fileRef = D4291A662DD61A3F00772088 /* SentryDispatchQueueProvider.h */; }; D4291A692DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h in Headers */ = {isa = PBXBuildFile; fileRef = D4291A672DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h */; }; + D4291A6D2DD62ACE00772088 /* SentryDispatchFactoryTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D4291A6C2DD62AC800772088 /* SentryDispatchFactoryTests.m */; }; D42E48572D48DF1600D251BC /* SentryBuildAppStartSpansTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D42E48562D48DF1600D251BC /* SentryBuildAppStartSpansTests.swift */; }; D43647F12D5CFB71001468E0 /* SentrySpanKeyTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43647F02D5CFB71001468E0 /* SentrySpanKeyTests.swift */; }; D43647F32D5CFBC7001468E0 /* FileManagerTracingIntegrationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D43647F22D5CFBC2001468E0 /* FileManagerTracingIntegrationTests.swift */; }; @@ -862,6 +861,8 @@ D4E829DF2D75FCF000D375AD /* SentryGraphicsImageRenderer.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4E829DE2D75FCE800D375AD /* SentryGraphicsImageRenderer.swift */; }; D4EDF9842D0B2A210071E7B3 /* Data+SentryTracing.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4EDF9832D0B2A1D0071E7B3 /* Data+SentryTracing.swift */; }; D4F2B5352D0C69D500649E42 /* SentryCrashCTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D4F2B5342D0C69D100649E42 /* SentryCrashCTests.swift */; }; + D4FC68172DD632E7001B74FF /* SentryDispatchSourceProviderProtocol.h in Headers */ = {isa = PBXBuildFile; fileRef = D4FC68162DD632E7001B74FF /* SentryDispatchSourceProviderProtocol.h */; }; + D4FC681A2DD63465001B74FF /* SentryDispatchQueueWrapperTests.m in Sources */ = {isa = PBXBuildFile; fileRef = D4FC68192DD63465001B74FF /* SentryDispatchQueueWrapperTests.m */; }; D8019910286B089000C277F0 /* SentryCrashReportSinkTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D801990F286B089000C277F0 /* SentryCrashReportSinkTests.swift */; }; D802994E2BA836EF000F0081 /* SentryOnDemandReplay.swift in Sources */ = {isa = PBXBuildFile; fileRef = D802994D2BA836EF000F0081 /* SentryOnDemandReplay.swift */; }; D80299502BA83A88000F0081 /* SentryPixelBuffer.swift in Sources */ = {isa = PBXBuildFile; fileRef = D802994F2BA83A88000F0081 /* SentryPixelBuffer.swift */; }; @@ -1996,9 +1997,8 @@ D4009EB12D771BB90007AF30 /* SentryFileIOTrackerSwiftHelpersTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryFileIOTrackerSwiftHelpersTests.swift; sourceTree = ""; }; D41909922D48FFF6002B83D0 /* SentryNSDictionarySanitize+Tests.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "SentryNSDictionarySanitize+Tests.h"; sourceTree = ""; }; D41909942D490006002B83D0 /* SentryNSDictionarySanitize+Tests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "SentryNSDictionarySanitize+Tests.m"; sourceTree = ""; }; - D4291A602DD61A1400772088 /* SentryDispatchQueueProvider.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryDispatchQueueProvider.m; sourceTree = ""; }; - D4291A662DD61A3F00772088 /* SentryDispatchQueueProvider.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDispatchQueueProvider.h; path = include/SentryDispatchQueueProvider.h; sourceTree = ""; }; D4291A672DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDispatchQueueProviderProtocol.h; path = include/SentryDispatchQueueProviderProtocol.h; sourceTree = ""; }; + D4291A6C2DD62AC800772088 /* SentryDispatchFactoryTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryDispatchFactoryTests.m; sourceTree = ""; }; D42E48562D48DF1600D251BC /* SentryBuildAppStartSpansTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryBuildAppStartSpansTests.swift; sourceTree = ""; }; D42E48582D48FC8F00D251BC /* SentryNSDictionarySanitizeTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryNSDictionarySanitizeTests.swift; sourceTree = ""; }; D43647F02D5CFB71001468E0 /* SentrySpanKeyTests.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SentrySpanKeyTests.swift; sourceTree = ""; }; @@ -2043,6 +2043,8 @@ D4E829DE2D75FCE800D375AD /* SentryGraphicsImageRenderer.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryGraphicsImageRenderer.swift; sourceTree = ""; }; D4EDF9832D0B2A1D0071E7B3 /* Data+SentryTracing.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Data+SentryTracing.swift"; sourceTree = ""; }; D4F2B5342D0C69D100649E42 /* SentryCrashCTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryCrashCTests.swift; sourceTree = ""; }; + D4FC68162DD632E7001B74FF /* SentryDispatchSourceProviderProtocol.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; name = SentryDispatchSourceProviderProtocol.h; path = include/SentryDispatchSourceProviderProtocol.h; sourceTree = ""; }; + D4FC68192DD63465001B74FF /* SentryDispatchQueueWrapperTests.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = SentryDispatchQueueWrapperTests.m; sourceTree = ""; }; D800942628F82F3A005D3943 /* SwiftDescriptor.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SwiftDescriptor.swift; sourceTree = ""; }; D801990F286B089000C277F0 /* SentryCrashReportSinkTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryCrashReportSinkTests.swift; sourceTree = ""; }; D802994D2BA836EF000F0081 /* SentryOnDemandReplay.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SentryOnDemandReplay.swift; sourceTree = ""; }; @@ -2425,15 +2427,14 @@ 631E6D321EBC679C00712345 /* SentryQueueableRequestManager.m */, 638DC99E1EBC6B6400A66E41 /* SentryRequestOperation.h */, 638DC99F1EBC6B6400A66E41 /* SentryRequestOperation.m */, + D4291A672DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h */, 7BDB03B6251364F800BAE198 /* SentryDispatchQueueWrapper.h */, 7BDB03BA2513652900BAE198 /* SentryDispatchQueueWrapper.m */, - D4291A672DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h */, - D4291A662DD61A3F00772088 /* SentryDispatchQueueProvider.h */, - D4291A602DD61A1400772088 /* SentryDispatchQueueProvider.m */, + D4FC68162DD632E7001B74FF /* SentryDispatchSourceProviderProtocol.h */, + 84AC61D129F7541E009EEF61 /* SentryDispatchSourceWrapper.m */, 84AC61D029F7541E009EEF61 /* SentryDispatchSourceWrapper.h */, 84AC61D429F75A98009EEF61 /* SentryDispatchFactory.h */, 84AC61D529F75A98009EEF61 /* SentryDispatchFactory.m */, - 84AC61D129F7541E009EEF61 /* SentryDispatchSourceWrapper.m */, 0AAE202028ED9BCC00D0CD80 /* SentryReachability.h */, 0AAE201D28ED9B9400D0CD80 /* SentryReachability.m */, ); @@ -3264,6 +3265,7 @@ 7BBD18AF24517E5D00427C76 /* Networking */ = { isa = PBXGroup; children = ( + D4291A6C2DD62AC800772088 /* SentryDispatchFactoryTests.m */, 92136D662C9D765D002A9FB8 /* SentryNSURLRequestBuilderTests.swift */, 7BBD18B124517FE800427C76 /* RateLimits */, 7BC8523A2458849E005A70F0 /* SentryDataCategoryMapperTests.swift */, @@ -3286,6 +3288,7 @@ 0AE455AC28F584D2006680E5 /* SentryReachabilityTests.m */, 629690522AD3E060000185FA /* SentryReachabilitySwiftTests.swift */, 0A9415B928F96CAC006A5DD1 /* TestSentryReachability.swift */, + D4FC68192DD63465001B74FF /* SentryDispatchQueueWrapperTests.m */, ); path = Networking; sourceTree = ""; @@ -4383,7 +4386,6 @@ 84DEE86B2B686BD400A7BC17 /* SentrySamplerDecision.h in Headers */, 7B4E23BE251A2BD500060D68 /* SentryCrashIntegrationSessionHandler.h in Headers */, 7B88F2FE24BC5A4C00ADF90A /* SentrySdkInfo.h in Headers */, - D4291A682DD61A3F00772088 /* SentryDispatchQueueProvider.h in Headers */, D4291A692DD61A3F00772088 /* SentryDispatchQueueProviderProtocol.h in Headers */, 7BCFBD672681C95000BC27D8 /* SentryScopeObserver.h in Headers */, D81A346C291AECC7005A27A9 /* PrivateSentrySDKOnly.h in Headers */, @@ -4659,6 +4661,7 @@ 848A451E2BBF9504006AAAEC /* SentryProfilerTestHelpers.h in Headers */, 639FCF9C1EBC7F9500778193 /* SentryThread.h in Headers */, 63FE716B20DA4C1100CDBAE8 /* SentryCrashJSONCodecObjC.h in Headers */, + D4FC68172DD632E7001B74FF /* SentryDispatchSourceProviderProtocol.h in Headers */, 63AA76A51EB9CBC200D153DE /* SentryDsn.h in Headers */, 844EDD6C2949387000C86F34 /* SentryMetricProfiler.h in Headers */, 636085131ED47BE600E8599E /* SentryFileManager.h in Headers */, @@ -5204,7 +5207,6 @@ 7D65260E237F649E00113EA2 /* SentryScope.m in Sources */, D4EDF9842D0B2A210071E7B3 /* Data+SentryTracing.swift in Sources */, 84281C472A57905700EE88F2 /* SentrySample.m in Sources */, - D4291A612DD61A1400772088 /* SentryDispatchQueueProvider.m in Sources */, 84AC61D329F7541E009EEF61 /* SentryDispatchSourceWrapper.m in Sources */, 62A456E52B0370E0003F19A1 /* SentryUIEventTrackerTransactionMode.m in Sources */, D4E829D62D75E39B00D375AD /* SentryViewRenderer.swift in Sources */, @@ -5450,6 +5452,7 @@ 7B6D98ED24C703F8005502FA /* Async.swift in Sources */, 7BA0C04C28056556003E0326 /* SentryTransportAdapterTests.swift in Sources */, 7BE0DC29272A9E1C004FA8B7 /* SentryBreadcrumbTrackerTests.swift in Sources */, + D4FC681A2DD63465001B74FF /* SentryDispatchQueueWrapperTests.m in Sources */, 63FE722520DA66EC00CDBAE8 /* SentryCrashFileUtils_Tests.m in Sources */, D86130122BB563FD004C0F5E /* SentrySessionReplayIntegrationTests.swift in Sources */, 7BFC16BA2524D4AF00FF6266 /* SentryMessage+Equality.m in Sources */, @@ -5481,6 +5484,7 @@ 7BB6550D253EEB3900887E87 /* SentryUserFeedbackTests.swift in Sources */, 7BBD18B7245180FF00427C76 /* SentryDsnTests.m in Sources */, D8DBE0D22C0EFFC300FAB1FD /* SentryReplayOptionsTests.swift in Sources */, + D4291A6D2DD62ACE00772088 /* SentryDispatchFactoryTests.m in Sources */, 62950F1029E7FE0100A42624 /* SentryTransactionContextTests.swift in Sources */, 0A2D7BBA29152CBF008727AF /* SentryWatchdogTerminationsScopeObserverTests.swift in Sources */, 7BD4BD4B27EB2DC20071F4FF /* SentryDiscardedEventTests.swift in Sources */, diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m index 3e716cfd09a..4ea0db57c4a 100644 --- a/Sources/Sentry/SentryDependencyContainer.m +++ b/Sources/Sentry/SentryDependencyContainer.m @@ -2,7 +2,6 @@ #import "SentryBinaryImageCache.h" #import "SentryDispatchFactory.h" -#import "SentryDispatchQueueProvider.h" #import "SentryDispatchQueueWrapper.h" #import "SentryDisplayLinkWrapper.h" #import "SentryExtraContextProvider.h" @@ -97,7 +96,6 @@ + (void)reset - (instancetype)init { if (self = [super init]) { - _dispatchQueueProvider = [[SentryDispatchQueueProvider alloc] init]; _dispatchQueueWrapper = [[SentryDispatchQueueWrapper alloc] init]; _random = [[SentryRandom alloc] init]; _threadWrapper = [[SentryThreadWrapper alloc] init]; @@ -491,6 +489,13 @@ - (SentryDispatchFactory *)dispatchFactory SENTRY_DISABLE_THREAD_SANITIZER( return _dispatchFactory; } +- (id)dispatchQueueProvider SENTRY_DISABLE_THREAD_SANITIZER( + "double-checked lock produce false alarms") +{ + // Reusing the existing dispatch factory as the dispatch queue provider. + return [self dispatchFactory]; +} + - (SentryNSTimerFactory *)timerFactory SENTRY_DISABLE_THREAD_SANITIZER( "double-checked lock produce false alarms") { diff --git a/Sources/Sentry/SentryDispatchFactory.m b/Sources/Sentry/SentryDispatchFactory.m index c3a0f94c9d3..7ec41369f8e 100644 --- a/Sources/Sentry/SentryDispatchFactory.m +++ b/Sources/Sentry/SentryDispatchFactory.m @@ -1,6 +1,7 @@ #import "SentryDispatchFactory.h" #import "SentryDispatchQueueWrapper.h" #import "SentryDispatchSourceWrapper.h" +#import "SentryInternalDefines.h" @implementation SentryDispatchFactory @@ -10,6 +11,16 @@ - (SentryDispatchQueueWrapper *)queueWithName:(const char *)name return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; } +- (SentryDispatchQueueWrapper *)createBackgroundQueueWithName:(const char *)name + relativePriority:(int)relativePriority +{ + SENTRY_CASSERT(relativePriority <= 0 && relativePriority >= QOS_MIN_RELATIVE_PRIORITY, + @"Relative priority must be between 0 and %d", QOS_MIN_RELATIVE_PRIORITY); + dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( + DISPATCH_QUEUE_SERIAL, QOS_CLASS_BACKGROUND, relativePriority); + return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; +} + - (SentryDispatchSourceWrapper *)sourceWithInterval:(uint64_t)interval leeway:(uint64_t)leeway queueName:(const char *)queueName diff --git a/Sources/Sentry/SentryDispatchQueueProvider.m b/Sources/Sentry/SentryDispatchQueueProvider.m deleted file mode 100644 index c75d35c9c05..00000000000 --- a/Sources/Sentry/SentryDispatchQueueProvider.m +++ /dev/null @@ -1,13 +0,0 @@ -#import "SentryDispatchQueueProvider.h" - -@implementation SentryDispatchQueueProvider - -- (SentryDispatchQueueWrapper *)createBackgroundQueueWithName:(const char *)name - relativePriority:(int)relativePriority -{ - dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( - DISPATCH_QUEUE_SERIAL, QOS_CLASS_BACKGROUND, relativePriority); - return [[SentryDispatchQueueWrapper alloc] initWithName:name attributes:attributes]; -} - -@end diff --git a/Sources/Sentry/SentryDispatchSourceWrapper.m b/Sources/Sentry/SentryDispatchSourceWrapper.m index 980911917da..63fc8a7a7d5 100644 --- a/Sources/Sentry/SentryDispatchSourceWrapper.m +++ b/Sources/Sentry/SentryDispatchSourceWrapper.m @@ -26,4 +26,16 @@ - (void)cancel dispatch_cancel(_source); } +#if SENTRY_TEST || SENTRY_TEST_CI +- (dispatch_source_t)source +{ + return _source; +} + +- (SentryDispatchQueueWrapper *)queue +{ + return _queueWrapper; +} +#endif // SENTRY_TEST || SENTRY_TEST_CI + @end diff --git a/Sources/Sentry/SentrySessionReplayIntegration.m b/Sources/Sentry/SentrySessionReplayIntegration.m index b4e2a6c7896..c3055fd2b17 100644 --- a/Sources/Sentry/SentrySessionReplayIntegration.m +++ b/Sources/Sentry/SentrySessionReplayIntegration.m @@ -41,6 +41,10 @@ static SentryTouchTracker *_touchTracker; @interface SentrySessionReplayIntegration () + +@property (nonatomic, strong) SentryDispatchQueueWrapper *replayProcessingQueue; +@property (nonatomic, strong) SentryDispatchQueueWrapper *replayAssetWorkerQueue; + - (void)newSceneActivate; @end @@ -59,8 +63,6 @@ @implementation SentrySessionReplayIntegration { // replay absolutely needs segment 0 to make replay work. BOOL _rateLimited; id _dateProvider; - SentryDispatchQueueWrapper *_replayProcessingQueue; - SentryDispatchQueueWrapper *_replayAssetWorkerQueue; } - (instancetype)init diff --git a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h index fb719369930..63adde158d0 100644 --- a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h +++ b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h @@ -67,7 +67,8 @@ SENTRY_NO_INIT @property (nonatomic, strong) SentryThreadWrapper *threadWrapper; @property (nonatomic, strong) id random; @property (nonatomic, strong) SentrySwizzleWrapper *swizzleWrapper; -@property (nonatomic, strong) id dispatchQueueProvider; +@property (nonatomic, strong, readonly) id + dispatchQueueProvider; @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueueWrapper; @property (nonatomic, strong) SentryNSNotificationCenterWrapper *notificationCenterWrapper; @property (nonatomic, strong) SentryDebugImageProvider *debugImageProvider; diff --git a/Sources/Sentry/include/SentryDispatchFactory.h b/Sources/Sentry/include/SentryDispatchFactory.h index 9dd2b815afb..74d2afde6af 100644 --- a/Sources/Sentry/include/SentryDispatchFactory.h +++ b/Sources/Sentry/include/SentryDispatchFactory.h @@ -1,3 +1,5 @@ +#import "SentryDispatchQueueProviderProtocol.h" +#import "SentryDispatchSourceProviderProtocol.h" #import @class SentryDispatchQueueWrapper; @@ -9,22 +11,8 @@ NS_ASSUME_NONNULL_BEGIN * A type of object that vends wrappers for dispatch queues and sources, which can be subclassed to * vend their mocked test subclasses. */ -@interface SentryDispatchFactory : NSObject - -/** - * Generate a new @c SentryDispatchQueueWrapper . - */ -- (SentryDispatchQueueWrapper *)queueWithName:(const char *)name - attributes:(dispatch_queue_attr_t)attributes; - -/** - * Generate a @c dispatch_source_t by internally vending the required @c SentryDispatchQueueWrapper. - */ -- (SentryDispatchSourceWrapper *)sourceWithInterval:(uint64_t)interval - leeway:(uint64_t)leeway - queueName:(const char *)queueName - attributes:(dispatch_queue_attr_t)attributes - eventHandler:(void (^)(void))eventHandler; +@interface SentryDispatchFactory + : NSObject @end diff --git a/Sources/Sentry/include/SentryDispatchQueueProvider.h b/Sources/Sentry/include/SentryDispatchQueueProvider.h deleted file mode 100644 index 4786df2bf18..00000000000 --- a/Sources/Sentry/include/SentryDispatchQueueProvider.h +++ /dev/null @@ -1,10 +0,0 @@ -#import "SentryDispatchQueueProviderProtocol.h" -#import - -NS_ASSUME_NONNULL_BEGIN - -@interface SentryDispatchQueueProvider : NSObject - -@end - -NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h b/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h index 7944f8eca9c..7b8bf017241 100644 --- a/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h +++ b/Sources/Sentry/include/SentryDispatchQueueProviderProtocol.h @@ -1,17 +1,28 @@ -#import "SentryDispatchQueueWrapper.h" #import +@class SentryDispatchQueueWrapper; + NS_ASSUME_NONNULL_BEGIN @protocol SentryDispatchQueueProviderProtocol /** - * Creates a background queue with the given name and relative priority. + * Generate a new @c SentryDispatchQueueWrapper . + */ +- (SentryDispatchQueueWrapper *)queueWithName:(const char *)name + attributes:(dispatch_queue_attr_t)attributes; + +/** + * Creates a background queue with the given name and relative priority, wrapped in a @c + * SentryDispatchQueueWrapper. * * @note This method is only a factory method and does not keep a reference to the created queue. * * @param name The name of the queue. - * @param relativePriority The priority of the queue relative to the background QoS. + * @param relativePriority A negative offset from the maximum supported scheduler priority for the + * given quality-of-service class. This value must be less than 0 and greater than or equal to @c + * QOS_MIN_RELATIVE_PRIORITY, otherwise throws an assertion and returns an unspecified + * quality-of-service. * @return Unretained reference to the created queue. */ - (SentryDispatchQueueWrapper *)createBackgroundQueueWithName:(const char *)name diff --git a/Sources/Sentry/include/SentryDispatchSourceProviderProtocol.h b/Sources/Sentry/include/SentryDispatchSourceProviderProtocol.h new file mode 100644 index 00000000000..d55924c3444 --- /dev/null +++ b/Sources/Sentry/include/SentryDispatchSourceProviderProtocol.h @@ -0,0 +1,19 @@ +#import + +@class SentryDispatchSourceWrapper; + +NS_ASSUME_NONNULL_BEGIN + +@protocol SentryDispatchSourceProviderProtocol + +/** + * Generate a @c dispatch_source_t by internally vending the required @c SentryDispatchQueueWrapper. + */ +- (SentryDispatchSourceWrapper *)sourceWithInterval:(uint64_t)interval + leeway:(uint64_t)leeway + queueName:(const char *)queueName + attributes:(dispatch_queue_attr_t)attributes + eventHandler:(void (^)(void))eventHandler; +@end + +NS_ASSUME_NONNULL_END diff --git a/Sources/Sentry/include/SentryDispatchSourceWrapper.h b/Sources/Sentry/include/SentryDispatchSourceWrapper.h index 2f817d09d63..ec70bd316f2 100644 --- a/Sources/Sentry/include/SentryDispatchSourceWrapper.h +++ b/Sources/Sentry/include/SentryDispatchSourceWrapper.h @@ -16,6 +16,11 @@ NS_ASSUME_NONNULL_BEGIN - (void)cancel; +#if SENTRY_TEST || SENTRY_TEST_CI +- (dispatch_source_t)source; +- (SentryDispatchQueueWrapper *)queue; +#endif // SENTRY_TEST || SENTRY_TEST_CI + @end NS_ASSUME_NONNULL_END diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift index 9c0e3b0c890..9a9573e5768 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift @@ -638,6 +638,23 @@ class SentrySessionReplayIntegrationTests: XCTestCase { XCTAssertEqual(writtenLastData, currentData) } + func testQueuePriorities_processingQueueShouldHaveLowerPriorityThanWorkerQueue() throws { + // -- Arrange -- + startSDK(sessionSampleRate: 1, errorSampleRate: 1) + let sut = try getSut() + let dynamicSut = Dynamic(sut) + + // -- Act -- + let processingQueue = try XCTUnwrap(dynamicSut.replayProcessingQueue.asObject as? SentryDispatchQueueWrapper) + let assetWorkerQueue = try XCTUnwrap(dynamicSut.replayAssetWorkerQueue.asObject as? SentryDispatchQueueWrapper) + + // -- Assert -- + XCTAssertEqual(assetWorkerQueue.queue.qos.qosClass, .background) + XCTAssertEqual(processingQueue.queue.qos.qosClass, .background) + + XCTAssertLessThan(processingQueue.queue.qos.relativePriority, assetWorkerQueue.queue.qos.relativePriority) + } + private func createLastSessionReplay(writeSessionInfo: Bool = true, errorSampleRate: Double = 1) throws { let replayFolder = replayFolder() let jsonPath = replayFolder + "/replay.current" diff --git a/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m b/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m new file mode 100644 index 00000000000..083846f0791 --- /dev/null +++ b/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m @@ -0,0 +1,135 @@ +#import "SentryDispatchFactory.h" +#import "SentryDispatchQueueWrapper.h" +#import "SentryDispatchSourceWrapper.h" +#import + +@interface SentryDispatchFactoryTests : XCTestCase +@property (nonatomic, strong) SentryDispatchFactory *sut; +@end + +@implementation SentryDispatchFactoryTests + +- (void)setUp +{ + [super setUp]; + + self.sut = [[SentryDispatchFactory alloc] init]; +} + +- (void)testQueueWithNameAndAttributes_shouldReturnQueueWithNameAndAttributesSet +{ + // Note: We are not testing the functionality of the queue itself, just the creation of it, + // making sure the factory sets the name and attributes correctly. + + // -- Arrange -- + const char *queueName = "sentry-dispatch-factory.test"; + int relativePriority = -5; + dispatch_queue_attr_t attributes = dispatch_queue_attr_make_with_qos_class( + DISPATCH_QUEUE_SERIAL, QOS_CLASS_UTILITY, relativePriority); + + // -- Act -- + SentryDispatchQueueWrapper *wrappedQueue = [self.sut queueWithName:queueName + attributes:attributes]; + + // -- Assert -- + const char *actualName = dispatch_queue_get_label(wrappedQueue.queue); + XCTAssertEqual(strcmp(actualName, queueName), 0); + + int actualRelativePriority; + dispatch_qos_class_t actualQoSClass + = dispatch_queue_get_qos_class(wrappedQueue.queue, &actualRelativePriority); + XCTAssertEqual(actualQoSClass, QOS_CLASS_BACKGROUND); + XCTAssertEqual(actualRelativePriority, relativePriority); +} + +- (void) + testCreateBackgroundQueueWithNameAndRelativePriority_shouldReturnQueueWithNameAndRelativePrioritySet +{ + // Note: We are not testing the functionality of the queue itself, just the creation of it, + // making sure the factory sets the name and attributes correctly. + + // -- Arrange -- + const char *queueName = "sentry-dispatch-factory.test"; + int relativePriority = -5; + + // -- Act -- + SentryDispatchQueueWrapper *wrappedQueue = + [self.sut createBackgroundQueueWithName:queueName relativePriority:relativePriority]; + + // -- Assert -- + const char *actualName = dispatch_queue_get_label(wrappedQueue.queue); + XCTAssertEqual(strcmp(actualName, queueName), 0); + + int actualRelativePriority; + dispatch_qos_class_t actualQoSClass + = dispatch_queue_get_qos_class(wrappedQueue.queue, &actualRelativePriority); + XCTAssertEqual(actualQoSClass, QOS_CLASS_BACKGROUND); + XCTAssertEqual(actualRelativePriority, relativePriority); +} + +- (void)testSourceWithInterval_providedEventHandlerIsCalled +{ + // Note: We are not testing the functionality of the source itself, just the creation of it, + // making sure the factory sets the name and attributes correctly. + + // -- Arrange -- + XCTestExpectation *expectation = [self expectationWithDescription:@"Timer fired"]; + uint64_t interval = 100 * NSEC_PER_MSEC; // 100ms + uint64_t leeway = 10 * NSEC_PER_MSEC; + const char *queueName = "sentry-dispatch-factory.timer"; + dispatch_queue_attr_t attributes + = dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_UTILITY, 0); + + __block int fireCount = 0; + void (^eventHandler)(void) = ^{ + fireCount++; + if (fireCount >= 1) { + [expectation fulfill]; + } + }; + + // -- Act -- + SentryDispatchSourceWrapper *source = [self.sut sourceWithInterval:interval + leeway:leeway + queueName:queueName + attributes:attributes + eventHandler:eventHandler]; + + // -- Assert -- + [self waitForExpectationsWithTimeout:1 handler:nil]; + XCTAssertGreaterThanOrEqual(fireCount, 1); + [source cancel]; +} + +- (void)testSource_queueUsesCorrectQoSAndPriority +{ + // Note: We are not testing the functionality of the source itself, just the creation of it, + // making sure the factory sets the name and attributes correctly. + + // -- Arrange -- + uint64_t interval = 1000; + uint64_t leeway = 0; + const char *queueName = "sentry-dispatch-factory.qos-check"; + dispatch_queue_attr_t attributes + = dispatch_queue_attr_make_with_qos_class(DISPATCH_QUEUE_SERIAL, QOS_CLASS_BACKGROUND, -10); + void (^eventHandler)(void) = ^{ }; + + // -- Act -- + SentryDispatchSourceWrapper *source = [self.sut sourceWithInterval:interval + leeway:leeway + queueName:queueName + attributes:attributes + eventHandler:eventHandler]; + SentryDispatchQueueWrapper *queueWrapper = source.queue; + + // -- Assert -- + const char *actualLabel = dispatch_queue_get_label(queueWrapper.queue); + XCTAssertEqual(strcmp(actualLabel, queueName), 0); + + int relativePriority; + dispatch_qos_class_t qos = dispatch_queue_get_qos_class(queueWrapper.queue, &relativePriority); + XCTAssertEqual(qos, QOS_CLASS_BACKGROUND); + XCTAssertEqual(relativePriority, -10); +} + +@end diff --git a/Tests/SentryTests/Networking/SentryDispatchQueueWrapperTests.m b/Tests/SentryTests/Networking/SentryDispatchQueueWrapperTests.m new file mode 100644 index 00000000000..085986040a9 --- /dev/null +++ b/Tests/SentryTests/Networking/SentryDispatchQueueWrapperTests.m @@ -0,0 +1,82 @@ +#import "SentryDispatchQueueWrapper.h" +#import + +NS_ASSUME_NONNULL_BEGIN + +@interface SentryDispatchQueueWrapperTests : XCTestCase + +@end + +@implementation SentryDispatchQueueWrapperTests + +- (void)testInitWithNameAndAttributes_shouldCreateQueueWithName +{ + // -- Arrange -- + const char *queueName = "sentry-dispatch-factory.test"; + + // -- Act -- + SentryDispatchQueueWrapper *wrappedQueue = + [[SentryDispatchQueueWrapper alloc] initWithName:queueName + attributes:DISPATCH_QUEUE_SERIAL]; + + // -- Assert -- + const char *actualName = dispatch_queue_get_label(wrappedQueue.queue); + XCTAssertEqual(strcmp(actualName, queueName), 0); +} + +- (void)testInitWithNameAndAttributes_customAttributes_shouldCreateQueueWithGivenAttributes +{ + // -- Arrange -- + const char *queueName = "sentry-dispatch-factory.test"; + int relativePriority = -5; + qos_class_t qosClass = QOS_CLASS_BACKGROUND; + dispatch_queue_attr_t _Nullable attr = DISPATCH_QUEUE_SERIAL; + + dispatch_queue_attr_t _Nullable attributes + = dispatch_queue_attr_make_with_qos_class(attr, qosClass, relativePriority); + + // -- Act -- + SentryDispatchQueueWrapper *wrappedQueue = + [[SentryDispatchQueueWrapper alloc] initWithName:queueName attributes:attributes]; + + // -- Assert -- + int actualRelativePriority; + dispatch_qos_class_t actualQoSClass + = dispatch_queue_get_qos_class(wrappedQueue.queue, &actualRelativePriority); + XCTAssertEqual(actualQoSClass, QOS_CLASS_BACKGROUND); + XCTAssertEqual(actualRelativePriority, -5); +} + +- (void) + testInitWithNameAndAttributes_customAttributesWithPositiveRelativePriority_shouldCreateQueueWithUnspecifiedQoS +{ + // The relative priority must be a negative offset from the maximum supported scheduler priority + // for the given quality-of-service class. This value must be less than 0 and greater than or + // equal to QOS_MIN_RELATIVE_PRIORITY, or else the attributes are NULL. If the attributes are + // NULL, the queue will be created with default unspecified QoS class and relative priority. + + // -- Arrange -- + const char *queueName = "sentry-dispatch-factory.test"; + int relativePriority = 5; + qos_class_t qosClass = QOS_CLASS_BACKGROUND; + dispatch_queue_attr_t _Nullable attr = DISPATCH_QUEUE_SERIAL; + + dispatch_queue_attr_t _Nullable attributes + = dispatch_queue_attr_make_with_qos_class(attr, qosClass, relativePriority); + + // -- Act -- + SentryDispatchQueueWrapper *wrappedQueue = + [[SentryDispatchQueueWrapper alloc] initWithName:queueName attributes:attributes]; + + // -- Assert -- + XCTAssertNil(attributes); + + int actualRelativePriority; + dispatch_qos_class_t actualQoSClass + = dispatch_queue_get_qos_class(wrappedQueue.queue, &actualRelativePriority); + XCTAssertEqual(actualQoSClass, QOS_CLASS_UNSPECIFIED); + XCTAssertEqual(actualRelativePriority, 0); +} +@end + +NS_ASSUME_NONNULL_END From 93479d04576002359d70ec866977c005f55659ad Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 16 May 2025 10:10:28 +0200 Subject: [PATCH 22/26] revert outdated changes --- .../xcshareddata/xcschemes/Sentry.xcscheme | 73 ------------------- 1 file changed, 73 deletions(-) diff --git a/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme b/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme index 79c7c938423..53783b37f22 100644 --- a/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme +++ b/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme @@ -60,74 +60,6 @@ BlueprintName = "SentryTests" ReferencedContainer = "container:Sentry.xcodeproj"> - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -138,11 +70,6 @@ BlueprintName = "SentryProfilerTests" ReferencedContainer = "container:Sentry.xcodeproj"> - - - - From ffb04e181db7c488bb94cea270aeeade1cc801b9 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 16 May 2025 10:35:32 +0200 Subject: [PATCH 23/26] fixes and reverts --- .../xcshareddata/xcschemes/Sentry.xcscheme | 73 +++++++++++++++++++ Sources/Sentry/SentryDependencyContainer.m | 10 ++- .../HybridPublic/SentryDependencyContainer.h | 3 +- .../SessionReplay/SentryOnDemandReplay.swift | 46 +++++++----- .../SentrySessionReplayIntegrationTests.swift | 6 ++ .../Networking/SentryDispatchFactoryTests.m | 2 +- 6 files changed, 116 insertions(+), 24 deletions(-) diff --git a/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme b/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme index 53783b37f22..79c7c938423 100644 --- a/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme +++ b/Sentry.xcodeproj/xcshareddata/xcschemes/Sentry.xcscheme @@ -60,6 +60,74 @@ BlueprintName = "SentryTests" ReferencedContainer = "container:Sentry.xcodeproj"> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + @@ -70,6 +138,11 @@ BlueprintName = "SentryProfilerTests" ReferencedContainer = "container:Sentry.xcodeproj"> + + + + diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m index f2121d82ec1..178b57876b1 100644 --- a/Sources/Sentry/SentryDependencyContainer.m +++ b/Sources/Sentry/SentryDependencyContainer.m @@ -492,8 +492,14 @@ - (SentryDispatchFactory *)dispatchFactory SENTRY_DISABLE_THREAD_SANITIZER( - (id)dispatchQueueProvider SENTRY_DISABLE_THREAD_SANITIZER( "double-checked lock produce false alarms") { - // Reusing the existing dispatch factory as the dispatch queue provider. - return [self dispatchFactory]; + if (_dispatchQueueProvider == nil) { + @synchronized(sentryDependencyContainerLock) { + if (_dispatchQueueProvider == nil) { + _dispatchQueueProvider = [[SentryDispatchFactory alloc] init]; + } + } + } + return _dispatchQueueProvider; } - (SentryNSTimerFactory *)timerFactory SENTRY_DISABLE_THREAD_SANITIZER( diff --git a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h index 63adde158d0..fb719369930 100644 --- a/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h +++ b/Sources/Sentry/include/HybridPublic/SentryDependencyContainer.h @@ -67,8 +67,7 @@ SENTRY_NO_INIT @property (nonatomic, strong) SentryThreadWrapper *threadWrapper; @property (nonatomic, strong) id random; @property (nonatomic, strong) SentrySwizzleWrapper *swizzleWrapper; -@property (nonatomic, strong, readonly) id - dispatchQueueProvider; +@property (nonatomic, strong) id dispatchQueueProvider; @property (nonatomic, strong) SentryDispatchQueueWrapper *dispatchQueueWrapper; @property (nonatomic, strong) SentryNSNotificationCenterWrapper *notificationCenterWrapper; @property (nonatomic, strong) SentryDebugImageProvider *debugImageProvider; diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index c200b2349ab..8457eb70a26 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -89,7 +89,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { } private func addFrame(image: UIImage, forScreen: String?) { - SentryLog.debug("[Session Replay] Adding frame for screen: \(forScreen ?? "nil")") + SentryLog.debug("[Session Replay] Adding frame to replay, screen: \(forScreen ?? "nil")") guard let data = rescaleImage(image)?.pngData() else { SentryLog.error("[Session Replay] Could not rescale image, dropping frame") return @@ -160,10 +160,10 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { processingQueue.dispatchAsync { do { let videos = try self.createVideoWith(beginning: beginning, end: end) - SentryLog.debug("[Session Replay] Finished creating video in backgroundwith \(videos.count) segments") + SentryLog.debug("[Session Replay] Finished creating video in background with \(videos.count) segments") completion(videos, nil) } catch { - SentryLog.error("[Session Replay] Failed to create video in background with error: \(error)") + SentryLog.error("[Session Replay] Failed to create video in background, reason: \(error)") completion(nil, error) } } @@ -233,8 +233,15 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // swiftlint:disable function_body_length cyclomatic_complexity private func renderVideo(with videoFrames: [SentryReplayFrame], from: Int, at outputFileURL: URL, completion: @escaping (Result) -> Void) { SentryLog.debug("[Session Replay] Rendering video with \(videoFrames.count) frames, from index: \(from), to output url: \(outputFileURL)") - guard from < videoFrames.count, let image = UIImage(contentsOfFile: videoFrames[from].imagePath) else { - SentryLog.error("[Session Replay] Failed to render video, reason: index out of bounds or can't read image at path: \(videoFrames[from].imagePath)") + guard from < videoFrames.count else { + SentryLog.error("[Session Replay] Failed to render video, reason: index out of bounds") + return completion(.success(SentryRenderVideoResult( + info: nil, + finalFrameIndex: from + ))) + } + guard let image = UIImage(contentsOfFile: videoFrames[from].imagePath) else { + SentryLog.error("[Session Replay] Failed to render video, reason: can't read image at path: \(videoFrames[from].imagePath)") return completion(.success(SentryRenderVideoResult( info: nil, finalFrameIndex: from @@ -257,7 +264,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { SentryLog.debug("[Session Replay] Creating pixel buffer based video writer input") let videoWriterInput = AVAssetWriterInput(mediaType: .video, outputSettings: createVideoSettings(width: videoWidth, height: videoHeight)) guard let currentPixelBuffer = SentryPixelBuffer(size: pixelSize, videoWriterInput: videoWriterInput) else { - SentryLog.error("[Session Replay] Failed to create pixel buffer, reason: \(SentryOnDemandReplayError.cantCreatePixelBuffer)") + SentryLog.error("[Session Replay] Failed to render video, reason: pixel buffer creation failed") return completion(.failure(SentryOnDemandReplayError.cantCreatePixelBuffer)) } videoWriter.add(videoWriterInput) @@ -293,12 +300,13 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // By setting the queue to the asset worker queue, we ensure that the callback is invoked on the asset worker queue. // This is important to avoid a deadlock, as this method is called on the processing queue. videoWriterInput.requestMediaDataWhenReady(on: assetWorkerQueue.queue) { [weak self] in + SentryLog.debug("[Session Replay] Video writer input is ready, status: \(videoWriter.status)") guard let strongSelf = self else { SentryLog.warning("[Session Replay] On-demand replay is deallocated, completing writing session without output video info") return deferredCompletionCallback(.success(nil)) } guard videoWriter.status == .writing else { - SentryLog.warning("[Session Replay] Video writer is not writing anymore, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + SentryLog.error("[Session Replay] Video writer is not writing anymore, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") videoWriter.cancelWriting() return deferredCompletionCallback(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) } @@ -316,9 +324,9 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { let frame = videoFrames[frameIndex] if let image = UIImage(contentsOfFile: frame.imagePath) { - SentryLog.debug("[Session Replay] Image is ready, size: \(image.size)") + SentryLog.debug("[Session Replay] Image at index \(frameIndex) is ready, size: \(image.size)") guard lastImageSize == image.size else { - SentryLog.debug("[Session Replay] Image size changed, finishing the video") + SentryLog.debug("[Session Replay] Image size has changed, finishing video") return strongSelf.finishVideo( outputFileURL: outputFileURL, usedFrames: usedFrames, @@ -334,8 +342,8 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { forFrameAtIndex: frameIndex, frameRate: strongSelf.frameRate ).timeValue - guard currentPixelBuffer.append(image: image, presentationTime: presentTime) == true else { - SentryLog.error("[Session Replay] Failed to append image to pixel buffer, cancelling the writing session, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + guard currentPixelBuffer.append(image: image, presentationTime: presentTime) else { + SentryLog.error("[Session Replay] Failed to append image to pixel buffer, cancelling the writing session, reason: \(String(describing: videoWriter.error))") videoWriter.cancelWriting() return deferredCompletionCallback(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) } @@ -369,13 +377,13 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { switch videoWriter.status { case .writing: - // noop - break + SentryLog.error("[Session Replay] Finish writing video was called with status writing, this is unexpected! Completing with no video info") + completion(.success(nil)) case .cancelled: - SentryLog.debug("[Session Replay] Finish writing video was cancelled, completing with no video info") + SentryLog.warning("[Session Replay] Finish writing video was cancelled, completing with no video info.") completion(.success(nil)) case .completed: - SentryLog.debug("[Session Replay] Finish writing video was completed, creating video info from file attributes") + SentryLog.debug("[Session Replay] Finish writing video was completed, creating video info from file attributes.") do { let result = try strongSelf.getVideoInfo( from: outputFileURL, @@ -385,17 +393,17 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { ) completion(.success(result)) } catch { - SentryLog.warning("[Session Replay] Failed to create video info from file attributes, reason: \(error.localizedDescription)") + SentryLog.warning("[Session Replay] Failed to create video info from file attributes, reason: \(error)") completion(.failure(error)) } case .failed: - SentryLog.warning("[Session Replay] Finish writing video failed, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + SentryLog.warning("[Session Replay] Finish writing video failed, reason: \(String(describing: videoWriter.error))") completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) case .unknown: - SentryLog.warning("[Session Replay] Finish writing video with unknown status, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + SentryLog.warning("[Session Replay] Finish writing video with unknown status, reason: \(String(describing: videoWriter.error))") completion(.failure(videoWriter.error ?? SentryOnDemandReplayError.errorRenderingVideo)) @unknown default: - SentryLog.warning("[Session Replay] Finish writing video failed, reason: \(videoWriter.error?.localizedDescription ?? "Unknown error")") + SentryLog.warning("[Session Replay] Finish writing video in unknown state, reason: \(String(describing: videoWriter.error))") completion(.failure(SentryOnDemandReplayError.errorRenderingVideo)) } } diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift index 5d5de8bf17a..89e92f54f4f 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayIntegrationTests.swift @@ -649,9 +649,15 @@ class SentrySessionReplayIntegrationTests: XCTestCase { let assetWorkerQueue = try XCTUnwrap(dynamicSut.replayAssetWorkerQueue.asObject as? SentryDispatchQueueWrapper) // -- Assert -- + XCTAssertEqual(assetWorkerQueue.queue.label, "io.sentry.session-replay.asset-worker") XCTAssertEqual(assetWorkerQueue.queue.qos.qosClass, .background) + + XCTAssertEqual(processingQueue.queue.label, "io.sentry.session-replay.processing") XCTAssertEqual(processingQueue.queue.qos.qosClass, .background) + // The actual priorities are not relevant, we just need to check that the processing queue has a lower priority + // than the asset worker queue and that both are lower than the default priority. + XCTAssertLessThan(processingQueue.queue.qos.relativePriority, 0) XCTAssertLessThan(processingQueue.queue.qos.relativePriority, assetWorkerQueue.queue.qos.relativePriority) } diff --git a/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m b/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m index 083846f0791..9f791910c1e 100644 --- a/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m +++ b/Tests/SentryTests/Networking/SentryDispatchFactoryTests.m @@ -38,7 +38,7 @@ - (void)testQueueWithNameAndAttributes_shouldReturnQueueWithNameAndAttributesSet int actualRelativePriority; dispatch_qos_class_t actualQoSClass = dispatch_queue_get_qos_class(wrappedQueue.queue, &actualRelativePriority); - XCTAssertEqual(actualQoSClass, QOS_CLASS_BACKGROUND); + XCTAssertEqual(actualQoSClass, QOS_CLASS_UTILITY); XCTAssertEqual(actualRelativePriority, relativePriority); } From 58ea59982f829d5fba0faaa97b1efab77220849b Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Fri, 16 May 2025 11:29:44 +0200 Subject: [PATCH 24/26] remove unnecessary error propagation --- .../Sentry/SentrySessionReplayIntegration.m | 10 +--------- .../SessionReplay/SentryOnDemandReplay.swift | 19 ++++++++----------- .../SentryReplayVideoMaker.swift | 4 ++-- .../SessionReplay/SentrySessionReplay.swift | 17 +++++------------ .../SentryOnDemandReplayTests.swift | 13 +++++++------ .../SentrySessionReplayTests.swift | 12 ++++-------- 6 files changed, 27 insertions(+), 48 deletions(-) diff --git a/Sources/Sentry/SentrySessionReplayIntegration.m b/Sources/Sentry/SentrySessionReplayIntegration.m index 103166a718c..df67ad4cd4c 100644 --- a/Sources/Sentry/SentrySessionReplayIntegration.m +++ b/Sources/Sentry/SentrySessionReplayIntegration.m @@ -246,16 +246,8 @@ - (void)resumePreviousSessionReplay:(SentryEvent *)event } NSDate *end = [beginning dateByAddingTimeInterval:duration]; - NSError *error; NSArray *videos = [resumeReplayMaker createVideoWithBeginning:beginning - end:end - error:&error]; - - // Either error or videos should be set. - if (error != nil) { - SENTRY_LOG_ERROR(@"[Session Replay] Could not create replay video, reason: %@", error); - return; - } + end:end]; if (videos == nil) { SENTRY_LOG_ERROR( @"[Session Replay] Could not create replay video, reason: no videos available"); diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index 8457eb70a26..afe7c2840bf 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -153,23 +153,18 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { return _frames.first?.time } - func createVideoInBackgroundWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) { + func createVideoInBackgroundWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]) -> Void) { // Note: In Swift it is best practice to use `Result` instead of `(Value?, Error?)` // Due to interoperability with Objective-C and @objc, we can not use Result for the completion callback. SentryLog.debug("[Session Replay] Creating video in background with beginning: \(beginning), end: \(end)") processingQueue.dispatchAsync { - do { - let videos = try self.createVideoWith(beginning: beginning, end: end) - SentryLog.debug("[Session Replay] Finished creating video in background with \(videos.count) segments") - completion(videos, nil) - } catch { - SentryLog.error("[Session Replay] Failed to create video in background, reason: \(error)") - completion(nil, error) - } + let videos = self.createVideoWith(beginning: beginning, end: end) + SentryLog.debug("[Session Replay] Finished creating video in background with \(videos.count) segments") + completion(videos) } } - func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] { + func createVideoWith(beginning: Date, end: Date) -> [SentryVideoInfo] { SentryLog.debug("[Session Replay] Creating video with beginning: \(beginning), end: \(end)") // Note: In previous implementations this method was wrapped by a sync call to the processing queue. // As this method is already called from the processing queue, we must remove the sync call. @@ -217,8 +212,10 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { return videos } - // If there was an error, throw it to exit the loop. + // If there was an error, log it and exit the loop. if let error = currentError { + // In previous implementations the error was propagated to the completion block, discarding any generated video. + // Instead this will "silently" fail by only logging the error and returning the successfully generated videos. SentryLog.error("[Session Replay] Error while rendering video: \(error), returning \(videos.count) videos") return videos } diff --git a/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift b/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift index 66a19bab4ef..1bb202b7512 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryReplayVideoMaker.swift @@ -6,8 +6,8 @@ import UIKit protocol SentryReplayVideoMaker: NSObjectProtocol { func addFrameAsync(image: UIImage, forScreen: String?) func releaseFramesUntil(_ date: Date) - func createVideoInBackgroundWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]?, Error?) -> Void) - func createVideoWith(beginning: Date, end: Date) throws -> [SentryVideoInfo] + func createVideoInBackgroundWith(beginning: Date, end: Date, completion: @escaping ([SentryVideoInfo]) -> Void) + func createVideoWith(beginning: Date, end: Date) -> [SentryVideoInfo] } extension SentryReplayVideoMaker { diff --git a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift index d22674f55be..a244f623a42 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentrySessionReplay.swift @@ -248,25 +248,17 @@ class SentrySessionReplay: NSObject { private func createAndCaptureInBackground(startedAt: Date, replayType: SentryReplayType) { SentryLog.debug("[Session Replay] Creating replay video started at date: \(startedAt), replayType: \(replayType)") // Creating a video is computationally expensive, therefore perform it on a background queue. - self.replayMaker.createVideoInBackgroundWith(beginning: startedAt, end: self.dateProvider.date()) { videos, error in - if let error = error { - SentryLog.error("[Session Replay] Could not create replay video, reason: \(error)") - return - } - guard let videos = videos else { - SentryLog.warning("[Session Replay] Finished replay video creation without any segments") - return - } + self.replayMaker.createVideoInBackgroundWith(beginning: startedAt, end: self.dateProvider.date()) { videos in SentryLog.debug("[Session Replay] Created replay video with \(videos.count) segments") for video in videos { - self.newSegmentAvailable(videoInfo: video, replayType: replayType) + self.processNewlyAvailableSegment(videoInfo: video, replayType: replayType) } SentryLog.debug("[Session Replay] Finished processing replay video with \(videos.count) segments") } } - private func newSegmentAvailable(videoInfo: SentryVideoInfo, replayType: SentryReplayType) { - SentryLog.debug("[Session Replay] New segment available for replayType: \(replayType), videoInfo: \(videoInfo)") + private func processNewlyAvailableSegment(videoInfo: SentryVideoInfo, replayType: SentryReplayType) { + SentryLog.debug("[Session Replay] Processing new segment available for replayType: \(replayType), videoInfo: \(videoInfo)") guard let sessionReplayId = sessionReplayId else { SentryLog.warning("[Session Replay] No session replay ID available, ignoring segment.") return @@ -275,6 +267,7 @@ class SentrySessionReplay: NSObject { replayMaker.releaseFramesUntil(videoInfo.end) videoSegmentStart = videoInfo.end currentSegmentId++ + SentryLog.debug("[Session Replay] Processed segment, incrementing currentSegmentId to: \(currentSegmentId)") } private func captureSegment(segment: Int, video: SentryVideoInfo, replayId: SentryId, replayType: SentryReplayType) { diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift index dd3e68c13fb..dbfb9cafbdc 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentryOnDemandReplayTests.swift @@ -94,7 +94,7 @@ class SentryOnDemandReplayTests: XCTestCase { let videoExpectation = expectation(description: "Wait for video render") - let videos = try sut.createVideoWith(beginning: Date(timeIntervalSinceReferenceDate: 0), end: Date(timeIntervalSinceReferenceDate: 10)) + let videos = sut.createVideoWith(beginning: Date(timeIntervalSinceReferenceDate: 0), end: Date(timeIntervalSinceReferenceDate: 10)) XCTAssertEqual(videos.count, 1) let info = try XCTUnwrap(videos.first) @@ -166,7 +166,7 @@ class SentryOnDemandReplayTests: XCTestCase { XCTAssertEqual(sut.frames.count, 0) } - func testInvalidWriter() throws { + func testCreateVideo_outputFileAlreadyExists_assetWriterErrorShouldNotThrowError() throws { // -- Arrange -- let processingQueue = SentryDispatchQueueWrapper() let workerQueue = SentryDispatchQueueWrapper() @@ -189,7 +189,8 @@ class SentryOnDemandReplayTests: XCTestCase { try Data("tempFile".utf8).write(to: outputPath.appendingPathComponent("0.0.mp4")) // -- Act & Assert -- - XCTAssertThrowsError(try sut.createVideoWith(beginning: start, end: end)) + let result = sut.createVideoWith(beginning: start, end: end) + XCTAssertEqual(result.count, 0) } func testGenerateVideoForEachSize() throws { @@ -204,7 +205,7 @@ class SentryOnDemandReplayTests: XCTestCase { sut.addFrameAsync(image: i < 5 ? image1 : image2) } - let videos = try sut.createVideoWith(beginning: Date(timeIntervalSinceReferenceDate: 0), end: Date(timeIntervalSinceReferenceDate: 10)) + let videos = sut.createVideoWith(beginning: Date(timeIntervalSinceReferenceDate: 0), end: Date(timeIntervalSinceReferenceDate: 10)) XCTAssertEqual(videos.count, 2) @@ -234,13 +235,13 @@ class SentryOnDemandReplayTests: XCTestCase { dateProvider.driftTimeInterval = 1 // -- Act -- - let videos = try sut.createVideoWith( + let videos = sut.createVideoWith( beginning: Date(timeIntervalSinceReferenceDate: 0), end: Date(timeIntervalSinceReferenceDate: 10) ) // -- Assert -- - XCTAssertNil(videos.first) + XCTAssertEqual(videos.count, 0) } func testCalculatePresentationTime_withOneFPS_shouldReturnTiming() { diff --git a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift index 63ae9f1da96..25895848f6d 100644 --- a/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift +++ b/Tests/SentryTests/Integrations/SessionReplay/SentrySessionReplayTests.swift @@ -38,19 +38,15 @@ class SentrySessionReplayTests: XCTestCase { func createVideoInBackgroundWith( beginning: Date, end: Date, - completion: @escaping ([Sentry.SentryVideoInfo]?, (any Error)?) -> Void + completion: @escaping ([Sentry.SentryVideoInfo]) -> Void ) { // Note: This implementation is just to satisfy the protocol. // If possible, keep the tests logic the synchronous version `createVideoWith` - do { - let videos = try createVideoWith(beginning: beginning, end: end) - completion(videos, nil) - } catch { - completion(nil, error) - } + let videos = createVideoWith(beginning: beginning, end: end) + completion(videos) } - func createVideoWith(beginning: Date, end: Date) throws -> [Sentry.SentryVideoInfo] { + func createVideoWith(beginning: Date, end: Date) -> [Sentry.SentryVideoInfo] { lastCallToCreateVideo = CreateVideoCall(beginning: beginning, end: end) let outputFileURL = FileManager.default.temporaryDirectory.appendingPathComponent("tempvideo.mp4") From 073fab858e2b8229de50fde346c717bfa8314d3d Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Mon, 19 May 2025 16:33:16 +0200 Subject: [PATCH 25/26] update docs and changelog --- CHANGELOG.md | 8 +++----- .../Integrations/SessionReplay/SentryOnDemandReplay.swift | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8b2a20f9ba2..7805ae92cc5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,18 +4,16 @@ ### Fixes -- Fix thread inversion warning in session replay (#5018) > [!Important] > This version creates new issue groups for your unhandled C++ exceptions because it now again reports the message of unhandled C++ exceptions, which we use for grouping. +- Reporting unhandled C++ exception message (#5190) +- Improved internal multi-threading of session replay to fix thread inversion warning and reduce chance of queue starvation (#5018) + ### Features - Apps can now manually show and hide the included feedback widget button (#5236) -### Fixes - -- Reporting unhandled C++ exception message (#5190) - ### Improvements - Add `itemCount` to `SentryEnvelopeItemHeader` ([#5230](https://github.com/getsentry/sentry-cocoa/pull/5230)) diff --git a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift index afe7c2840bf..a22a6482820 100644 --- a/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift +++ b/Sources/Swift/Integrations/SessionReplay/SentryOnDemandReplay.swift @@ -214,7 +214,7 @@ class SentryOnDemandReplay: NSObject, SentryReplayVideoMaker { // If there was an error, log it and exit the loop. if let error = currentError { - // In previous implementations the error was propagated to the completion block, discarding any generated video. + // Until v8.50.2 the error was propagated to the completion block, discarding any generated video. // Instead this will "silently" fail by only logging the error and returning the successfully generated videos. SentryLog.error("[Session Replay] Error while rendering video: \(error), returning \(videos.count) videos") return videos From fa9a3b75f95cbbaa122c6368c97a42eb7abca909 Mon Sep 17 00:00:00 2001 From: Philip Niedertscheider Date: Mon, 19 May 2025 16:34:25 +0200 Subject: [PATCH 26/26] fix merge conflict in changelog --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7805ae92cc5..f5ff436cc51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,18 +2,18 @@ ## Unreleased -### Fixes - > [!Important] > This version creates new issue groups for your unhandled C++ exceptions because it now again reports the message of unhandled C++ exceptions, which we use for grouping. -- Reporting unhandled C++ exception message (#5190) -- Improved internal multi-threading of session replay to fix thread inversion warning and reduce chance of queue starvation (#5018) - ### Features - Apps can now manually show and hide the included feedback widget button (#5236) +### Fixes + +- Reporting unhandled C++ exception message (#5190) +- Improved internal multi-threading of session replay to fix thread inversion warning and reduce chance of queue starvation (#5018) + ### Improvements - Add `itemCount` to `SentryEnvelopeItemHeader` ([#5230](https://github.com/getsentry/sentry-cocoa/pull/5230))