From 008799df4d36558cf31a02072301cc9a6dee9c14 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Mon, 29 Jan 2024 08:42:45 +0100 Subject: [PATCH 01/19] fix: Move header reference out of `extern C` (#3538) Removing header references from extern C because they are causing compile errors with Android Studio and flutter. --- CHANGELOG.md | 7 ++++++- .../Recording/Tools/SentryCrashMachineContext.h | 8 ++++---- .../SentryCrash/Recording/Tools/SentryCrashStackCursor.h | 9 ++++----- Sources/SentryCrash/Recording/Tools/SentryCrashThread.h | 6 +++--- 4 files changed, 17 insertions(+), 13 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a53469418c..62f2af5baa4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Move header reference out of "extern C" (#3538) + ## 8.19.0 ### Features @@ -30,7 +36,6 @@ - Fix a race condition in SentryTracer (#3523) - App start ends when first frame is drawn when performanceV2 is enabled (#3530) - Use correct rendered frames timestamp for TTID/TTFD and app start (#3531) - - Missing transactions when not calling `reportFullyDisplayed` (#3477) ## 8.17.2 diff --git a/Sources/SentryCrash/Recording/Tools/SentryCrashMachineContext.h b/Sources/SentryCrash/Recording/Tools/SentryCrashMachineContext.h index 6e3cb9e1f98..5b97e04d2b2 100644 --- a/Sources/SentryCrash/Recording/Tools/SentryCrashMachineContext.h +++ b/Sources/SentryCrash/Recording/Tools/SentryCrashMachineContext.h @@ -28,14 +28,14 @@ #ifndef HDR_SentryCrashMachineContext_h #define HDR_SentryCrashMachineContext_h -#ifdef __cplusplus -extern "C" { -#endif - #include "SentryCrashThread.h" #include #include +#ifdef __cplusplus +extern "C" { +#endif + /** Suspend the runtime environment. */ void sentrycrashmc_suspendEnvironment( diff --git a/Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor.h b/Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor.h index 9edb737c43f..0805ddaf2cd 100644 --- a/Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor.h +++ b/Sources/SentryCrash/Recording/Tools/SentryCrashStackCursor.h @@ -26,15 +26,14 @@ #ifndef SentryCrashStackCursor_h #define SentryCrashStackCursor_h -#ifdef __cplusplus -extern "C" { -#endif - #include "SentryCrashMachineContext.h" - #include #include +#ifdef __cplusplus +extern "C" { +#endif + #define SentryCrashSC_CONTEXT_SIZE 100 /** Point at which to give up walking a stack and consider it a stack overflow. diff --git a/Sources/SentryCrash/Recording/Tools/SentryCrashThread.h b/Sources/SentryCrash/Recording/Tools/SentryCrashThread.h index 7244aeb297c..e061f227133 100644 --- a/Sources/SentryCrash/Recording/Tools/SentryCrashThread.h +++ b/Sources/SentryCrash/Recording/Tools/SentryCrashThread.h @@ -28,13 +28,13 @@ #ifndef HDR_SentryCrashThread_h #define HDR_SentryCrashThread_h +#include +#include + #ifdef __cplusplus extern "C" { #endif -#include -#include - typedef uintptr_t SentryCrashThread; /** Get a thread's name. Internally, a thread name will From 8e5919b7aba462e377b44adc30b2f110d62f1b62 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 29 Jan 2024 09:19:10 +0100 Subject: [PATCH 02/19] Add VisionOS Support for Carthage (#3565) Fix Sources/Configuration/SDK.xcconfig by removing the workaround added with #491 so CI can build Carthage XCFrameworks to include VisionOS. The workaround seems to be fixed with Carthage/Carthage#3001 in Carthage 0.35.0 released in Jun 2020. Therefore, we can remove the SDKROOT__CARTHAGE settings in Sources/Configuration/SDK.xcconfig, which caused problems when building the SDK for visionOS see #3410 (comment). We never removed the workaround #491, as it didn't cause any problems. Furthermore, bump pre-commit action https://github.com/python-jsonschema/check-jsonschema to 0.27.3, so it allows macos-13-xlarge as a valid runner. --- .github/workflows/build.yml | 10 ++++++---- .pre-commit-config.yaml | 2 +- CHANGELOG.md | 1 + .../Configuration/DeploymentTargets.xcconfig | 1 + Sources/Configuration/SDK.xcconfig | 19 ------------------- 5 files changed, 9 insertions(+), 24 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index e95a91e2734..8e70c86be5b 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -91,10 +91,12 @@ jobs: build-xcframework: name: Build XCFramework - runs-on: macos-13 + # The macos-13 uses an Intel processor and doesn't compile the XCFramework for visionOS. + # The large image compiles on arm64 and successfully creates the XCFramework for visionOS. + runs-on: macos-13-xlarge steps: - uses: actions/checkout@v4 - - run: ./scripts/ci-select-xcode.sh 15.0.1 + - run: ./scripts/ci-select-xcode.sh 15.2 - run: make build-xcframework shell: sh @@ -116,14 +118,14 @@ jobs: validate-xcframework: name: Validate XCFramework - runs-on: macos-13 + runs-on: macos-13-xlarge needs: build-xcframework steps: - uses: actions/checkout@v4 - uses: actions/download-artifact@v4 with: name: ${{ github.sha }} - - run: ./scripts/ci-select-xcode.sh 15.0.1 + - run: ./scripts/ci-select-xcode.sh 15.2 - run: make build-xcframework-sample shell: sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 90fe2b02560..384f9fcb076 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -14,7 +14,7 @@ repos: - id: no-commit-to-branch - repo: https://github.com/python-jsonschema/check-jsonschema - rev: 0.23.1 + rev: 0.27.3 hooks: - id: check-github-actions - id: check-github-workflows diff --git a/CHANGELOG.md b/CHANGELOG.md index 62f2af5baa4..b46e4d6dd8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ ### Features - Send debug meta for app start transactions (#3543) +- Add VisionOS Support for Carthage (#3565) ### Fixes diff --git a/Sources/Configuration/DeploymentTargets.xcconfig b/Sources/Configuration/DeploymentTargets.xcconfig index 1945e658194..d8bdbc678e4 100644 --- a/Sources/Configuration/DeploymentTargets.xcconfig +++ b/Sources/Configuration/DeploymentTargets.xcconfig @@ -2,3 +2,4 @@ MACOSX_DEPLOYMENT_TARGET = 10.13 IPHONEOS_DEPLOYMENT_TARGET = 11.0 WATCHOS_DEPLOYMENT_TARGET = 4.0 TVOS_DEPLOYMENT_TARGET = 11.0 +XROS_DEPLOYMENT_TARGET = 1.0 diff --git a/Sources/Configuration/SDK.xcconfig b/Sources/Configuration/SDK.xcconfig index b9d655e46f4..bf3f8cfb889 100644 --- a/Sources/Configuration/SDK.xcconfig +++ b/Sources/Configuration/SDK.xcconfig @@ -1,24 +1,5 @@ #include "DeploymentTargets.xcconfig" -SDKROOT = $(SDKROOT__CARTHAGE_$(CARTHAGE)) // basically, iphoneos (unless «CARTHAGE» == «YES») -// Carthage relies on this assumption, years standing — that SDKROOT is default or explicitly -// set to `macosx` (or equivalent) under the ‘single target, multiple platform’ paradigm -// because of a xcodebuild bug involving the sdk flag and implicit dependency: see «Carthage/Carthage#347». -SDKROOT__CARTHAGE_YES = macosx -// Importantly, the below two lines appease «Xcode.app», and get the UI to show Mac Catalyst destinations. -SDKROOT__CARTHAGE_NO = iphoneos -SDKROOT__CARTHAGE_ = iphoneos -// …in order for ‘single target, multiple platform’ extrapolations to hold true, -// all the above relies on the ability of Xcode GUI, xcodebuild, and Carthage via xcodebuild to -// override «SDKROOT» based on selected destination (particularly for appletv* and watchos* platforms.) -// …if the override behavior ever breaks, expect weird output and the probable need to migrate away from -// the ‘single target, multiple platform’ paradigm. - -// …`SUPPORTED_PLATFORMS`, in service of ‘single target, multiple platform’ extrapolation, must never -// engage in dollar-parentheses syntax — unless that dollar-parentheses basis is -// entirely non-platform–derived, e.g. based upon `XCODE_VERSION_MAJOR`. -// Note: Carthage, unfortunately, as current of v0.34.0 queries rather harshly on the platform values below -// ⋯ quite early in the process, queried values not compiled into Carthage will cause hard errors. SUPPORTED_PLATFORMS = macosx iphoneos iphonesimulator watchos watchsimulator appletvos appletvsimulator xros xrsimulator TARGETED_DEVICE_FAMILY = 1,2,3,4,7 SKIP_INSTALL = YES From 89491ad8edbda58b515e9d2ede12184aca132e01 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 29 Jan 2024 09:22:23 +0100 Subject: [PATCH 03/19] fix: Clarify FramesTracker log message (#3570) Change two log messages to state they detected a slow or frozen frame but not to capture them. --- CHANGELOG.md | 1 + Sources/Sentry/SentryFramesTracker.m | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b46e4d6dd8f..c3f67e081c1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixes - Move header reference out of "extern C" (#3538) +- Clarify FramesTracker log message (#3570) ## 8.19.0 diff --git a/Sources/Sentry/SentryFramesTracker.m b/Sources/Sentry/SentryFramesTracker.m index 038e4b52a9e..cf689be27e4 100644 --- a/Sources/Sentry/SentryFramesTracker.m +++ b/Sources/Sentry/SentryFramesTracker.m @@ -164,7 +164,7 @@ - (void)displayLinkCallback && frameDuration <= SentryFrozenFrameThreshold) { _slowFrames++; # if SENTRY_TARGET_PROFILING_SUPPORTED - SENTRY_LOG_DEBUG(@"Capturing slow frame starting at %llu (frame tracker: %@).", + SENTRY_LOG_DEBUG(@"Detected slow frame starting at %llu (frame tracker: %@).", thisFrameSystemTimestamp, self); [self recordTimestamp:thisFrameSystemTimestamp value:@(thisFrameSystemTimestamp - self.previousFrameSystemTimestamp) @@ -173,7 +173,7 @@ - (void)displayLinkCallback } else if (frameDuration > SentryFrozenFrameThreshold) { _frozenFrames++; # if SENTRY_TARGET_PROFILING_SUPPORTED - SENTRY_LOG_DEBUG(@"Capturing frozen frame starting at %llu.", thisFrameSystemTimestamp); + SENTRY_LOG_DEBUG(@"Detected frozen frame starting at %llu.", thisFrameSystemTimestamp); [self recordTimestamp:thisFrameSystemTimestamp value:@(thisFrameSystemTimestamp - self.previousFrameSystemTimestamp) array:self.frozenFrameTimestamps]; From 2c603bfb9082e6b39f6c64d56843cb9c4cdc158d Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 29 Jan 2024 13:13:27 +0100 Subject: [PATCH 04/19] Fix CHANGELOG.md (#3578) --- CHANGELOG.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c3f67e081c1..988c7994d57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## Unreleased +### Features + +- Add VisionOS Support for Carthage (#3565) + ### Fixes - Move header reference out of "extern C" (#3538) @@ -12,7 +16,6 @@ ### Features - Send debug meta for app start transactions (#3543) -- Add VisionOS Support for Carthage (#3565) ### Fixes From 10eb2883894132d6603a03954a7a7b6ae2bdcfc1 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 29 Jan 2024 13:13:47 +0100 Subject: [PATCH 05/19] chore: Bump CocoaPods from 1.14.3 to 1.15.0 (#3579) --- Gemfile.lock | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index f02a8dcec41..04287bdedba 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -3,7 +3,7 @@ GEM specs: CFPropertyList (3.0.6) rexml - activesupport (7.1.2) + activesupport (7.1.3) base64 bigdecimal concurrent-ruby (~> 1.0, >= 1.0.2) @@ -38,13 +38,13 @@ GEM aws-eventstream (~> 1, >= 1.0.2) babosa (1.0.4) base64 (0.2.0) - bigdecimal (3.1.4) + bigdecimal (3.1.6) claide (1.1.0) clamp (1.3.2) - cocoapods (1.14.3) + cocoapods (1.15.0) addressable (~> 2.8) claide (>= 1.0.2, < 2.0) - cocoapods-core (= 1.14.3) + cocoapods-core (= 1.15.0) cocoapods-deintegrate (>= 1.0.3, < 2.0) cocoapods-downloader (>= 2.1, < 3.0) cocoapods-plugins (>= 1.0.0, < 2.0) @@ -59,7 +59,7 @@ GEM nap (~> 1.0) ruby-macho (>= 2.3.0, < 3.0) xcodeproj (>= 1.23.0, < 2.0) - cocoapods-core (1.14.3) + cocoapods-core (1.15.0) activesupport (>= 5.0, < 8) addressable (~> 2.8) algoliasearch (~> 1.0) @@ -82,7 +82,7 @@ GEM colored2 (3.1.2) commander (4.6.0) highline (~> 2.0.0) - concurrent-ruby (1.2.2) + concurrent-ruby (1.2.3) connection_pool (2.4.1) declarative (0.0.20) digest-crc (0.6.5) @@ -225,7 +225,7 @@ GEM mini_magick (4.12.0) mini_mime (1.1.5) mini_portile2 (2.8.5) - minitest (5.20.0) + minitest (5.21.2) molinillo (0.8.0) multi_json (1.15.0) multipart-post (2.3.0) @@ -289,7 +289,7 @@ GEM unicode-display_width (2.5.0) webrick (1.8.1) word_wrap (1.0.0) - xcodeproj (1.23.0) + xcodeproj (1.24.0) CFPropertyList (>= 2.3.3, < 4.0) atomos (~> 0.1.3) claide (>= 1.0.2, < 2.0) From eef4553cc006286e675705eefef6444853dd5a0e Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 29 Jan 2024 13:28:20 +0100 Subject: [PATCH 06/19] fix: Synchronization issue in FramesTracker (#3571) * fix: Synchronization issue in FramesTracker Synchronize the call to SentryFramesTracker.resetProfilingTimestamps by dispatching it to the main thread cause there is no guarantee that the SDK calls this method on the main thread. Previously, this issue led to crashes in SentryFramesTracker. Fixes GH-3511 --- CHANGELOG.md | 1 + Sources/Sentry/SentryDependencyContainer.m | 1 + Sources/Sentry/SentryFramesTracker.m | 14 ++++++++++- Sources/Sentry/include/SentryFramesTracker.h | 2 ++ .../SentryProfilerSwiftTests.swift | 2 +- .../SentryAppStartTrackerTests.swift | 2 +- .../SentryFramesTrackerTests.swift | 25 +++++++++++++++++-- .../SentryTimeToDisplayTrackerTest.swift | 2 +- .../Transaction/SentrySpanTests.swift | 2 +- 9 files changed, 44 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 988c7994d57..23f82ea9352 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Move header reference out of "extern C" (#3538) - Clarify FramesTracker log message (#3570) +- Fix synchronization issue in FramesTracker (#3571) ## 8.19.0 diff --git a/Sources/Sentry/SentryDependencyContainer.m b/Sources/Sentry/SentryDependencyContainer.m index 0562824ac29..14063e80209 100644 --- a/Sources/Sentry/SentryDependencyContainer.m +++ b/Sources/Sentry/SentryDependencyContainer.m @@ -275,6 +275,7 @@ - (SentryFramesTracker *)framesTracker _framesTracker = [[SentryFramesTracker alloc] initWithDisplayLinkWrapper:[[SentryDisplayLinkWrapper alloc] init] dateProvider:self.dateProvider + dispatchQueueWrapper:self.dispatchQueueWrapper keepDelayedFramesDuration:SENTRY_AUTO_TRANSACTION_MAX_DURATION]; } } diff --git a/Sources/Sentry/SentryFramesTracker.m b/Sources/Sentry/SentryFramesTracker.m index cf689be27e4..9c009bb9395 100644 --- a/Sources/Sentry/SentryFramesTracker.m +++ b/Sources/Sentry/SentryFramesTracker.m @@ -6,6 +6,7 @@ # import "SentryCurrentDateProvider.h" # import "SentryDelayedFrame.h" # import "SentryDelayedFramesTracker.h" +# import "SentryDispatchQueueWrapper.h" # import "SentryDisplayLinkWrapper.h" # import "SentryLog.h" # import "SentryProfiler.h" @@ -28,6 +29,7 @@ @property (nonatomic, strong, readonly) SentryDisplayLinkWrapper *displayLinkWrapper; @property (nonatomic, strong, readonly) SentryCurrentDateProvider *dateProvider; +@property (nonatomic, strong, readonly) SentryDispatchQueueWrapper *dispatchQueueWrapper; @property (nonatomic, assign) CFTimeInterval previousFrameTimestamp; @property (nonatomic) uint64_t previousFrameSystemTimestamp; @property (nonatomic) uint64_t currentFrameRate; @@ -58,12 +60,14 @@ @implementation SentryFramesTracker { - (instancetype)initWithDisplayLinkWrapper:(SentryDisplayLinkWrapper *)displayLinkWrapper dateProvider:(SentryCurrentDateProvider *)dateProvider + dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper keepDelayedFramesDuration:(CFTimeInterval)keepDelayedFramesDuration { if (self = [super init]) { _isRunning = NO; _displayLinkWrapper = displayLinkWrapper; _dateProvider = dateProvider; + _dispatchQueueWrapper = dispatchQueueWrapper; _delayedFramesTracker = [[SentryDelayedFramesTracker alloc] initWithKeepDelayedFramesDuration:keepDelayedFramesDuration dateProvider:dateProvider]; @@ -91,7 +95,7 @@ - (void)resetFrames self.previousFrameTimestamp = SentryPreviousFrameInitialValue; # if SENTRY_TARGET_PROFILING_SUPPORTED - [self resetProfilingTimestamps]; + [self resetProfilingTimestampsInternal]; # endif // SENTRY_TARGET_PROFILING_SUPPORTED [self.delayedFramesTracker resetDelayedFramesTimeStamps]; @@ -99,11 +103,19 @@ - (void)resetFrames # if SENTRY_TARGET_PROFILING_SUPPORTED - (void)resetProfilingTimestamps +{ + // The DisplayLink callback always runs on the main thread. We dispatch this to the main thread + // instead to avoid using locks in the DisplayLink callback. + [self.dispatchQueueWrapper dispatchOnMainQueue:^{ [self resetProfilingTimestampsInternal]; }]; +} + +- (void)resetProfilingTimestampsInternal { self.frozenFrameTimestamps = [SentryMutableFrameInfoTimeSeries array]; self.slowFrameTimestamps = [SentryMutableFrameInfoTimeSeries array]; self.frameRateTimestamps = [SentryMutableFrameInfoTimeSeries array]; } + # endif // SENTRY_TARGET_PROFILING_SUPPORTED - (void)start diff --git a/Sources/Sentry/include/SentryFramesTracker.h b/Sources/Sentry/include/SentryFramesTracker.h index 56f139d55b3..daccbea5184 100644 --- a/Sources/Sentry/include/SentryFramesTracker.h +++ b/Sources/Sentry/include/SentryFramesTracker.h @@ -6,6 +6,7 @@ @class SentryOptions, SentryDisplayLinkWrapper, SentryScreenFrames; @class SentryCurrentDateProvider; +@class SentryDispatchQueueWrapper; NS_ASSUME_NONNULL_BEGIN @@ -24,6 +25,7 @@ NS_ASSUME_NONNULL_BEGIN - (instancetype)initWithDisplayLinkWrapper:(SentryDisplayLinkWrapper *)displayLinkWrapper dateProvider:(SentryCurrentDateProvider *)dateProvider + dispatchQueueWrapper:(SentryDispatchQueueWrapper *)dispatchQueueWrapper keepDelayedFramesDuration:(CFTimeInterval)keepDelayedFramesDuration; @property (nonatomic, assign, readonly) SentryScreenFrames *currentFrames; diff --git a/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift b/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift index 3c87f3c7655..7075e485735 100644 --- a/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift +++ b/Tests/SentryProfilerTests/SentryProfilerSwiftTests.swift @@ -36,7 +36,7 @@ class SentryProfilerSwiftTests: XCTestCase { #if !os(macOS) lazy var displayLinkWrapper = TestDisplayLinkWrapper(dateProvider: currentDateProvider) - lazy var framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: currentDateProvider, keepDelayedFramesDuration: 0) + lazy var framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: currentDateProvider, dispatchQueueWrapper: SentryDispatchQueueWrapper(), keepDelayedFramesDuration: 0) #endif init() { diff --git a/Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift index 18106d87d0d..ef8361ac3f8 100644 --- a/Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/AppStartTracking/SentryAppStartTrackerTests.swift @@ -46,7 +46,7 @@ class SentryAppStartTrackerTests: NotificationCenterTestCase { notificationCenterWrapper: SentryNSNotificationCenterWrapper() ) - framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: currentDate, keepDelayedFramesDuration: 0) + framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: currentDate, dispatchQueueWrapper: SentryDispatchQueueWrapper(), keepDelayedFramesDuration: 0) framesTracker.start() runtimeInitTimestamp = SentryDependencyContainer.sharedInstance().dateProvider.date().addingTimeInterval(0.2) diff --git a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift index b618ae16cf7..83692562a98 100644 --- a/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift +++ b/Tests/SentryTests/Integrations/Performance/FramesTracking/SentryFramesTrackerTests.swift @@ -23,7 +23,7 @@ class SentryFramesTrackerTests: XCTestCase { slowestSlowFrameDelay = (displayLinkWrapper.slowestSlowFrameDuration - slowFrameThreshold(displayLinkWrapper.currentFrameRate.rawValue)) } - lazy var sut: SentryFramesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, keepDelayedFramesDuration: keepDelayedFramesDuration) + lazy var sut: SentryFramesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, dispatchQueueWrapper: SentryDispatchQueueWrapper(), keepDelayedFramesDuration: keepDelayedFramesDuration) } private var fixture: Fixture! @@ -601,13 +601,34 @@ class SentryFramesTrackerTests: XCTestCase { func testDealloc_CallsStop() { func sutIsDeallocatedAfterCallingMe() { - _ = SentryFramesTracker(displayLinkWrapper: fixture.displayLinkWrapper, dateProvider: fixture.dateProvider, keepDelayedFramesDuration: 0) + _ = SentryFramesTracker(displayLinkWrapper: fixture.displayLinkWrapper, dateProvider: fixture.dateProvider, dispatchQueueWrapper: SentryDispatchQueueWrapper(), keepDelayedFramesDuration: 0) } sutIsDeallocatedAfterCallingMe() XCTAssertEqual(1, fixture.displayLinkWrapper.invalidateInvocations.count) } +#if os(iOS) || os(macOS) || targetEnvironment(macCatalyst) + func testResetProfilingTimestamps_FromBackgroundThread() { + let sut = fixture.sut + sut.start() + + let queue = DispatchQueue(label: "reset profiling timestamps", attributes: [.initiallyInactive, .concurrent]) + + for _ in 0..<10_000 { + queue.async { + sut.resetProfilingTimestamps() + } + } + + queue.activate() + + for _ in 0..<1_000 { + self.fixture.displayLinkWrapper.normalFrame() + } + } +#endif // os(iOS) || os(tvOS) || targetEnvironment(macCatalyst) + private func givenMoreDelayedFramesThanTransactionMaxDuration(_ framesTracker: SentryFramesTracker) -> (UInt64, UInt, Double) { let displayLink = fixture.displayLinkWrapper displayLink.call() diff --git a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift index 70c77257c45..9a16193649b 100644 --- a/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift +++ b/Tests/SentryTests/Integrations/Performance/UIViewController/SentryTimeToDisplayTrackerTest.swift @@ -16,7 +16,7 @@ class SentryTimeToDisplayTrackerTest: XCTestCase { var framesTracker: SentryFramesTracker init() { - framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, keepDelayedFramesDuration: 0) + framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: dateProvider, dispatchQueueWrapper: SentryDispatchQueueWrapper(), keepDelayedFramesDuration: 0) SentryDependencyContainer.sharedInstance().framesTracker = framesTracker framesTracker.start() } diff --git a/Tests/SentryTests/Transaction/SentrySpanTests.swift b/Tests/SentryTests/Transaction/SentrySpanTests.swift index 15b9ae320dd..a71d4b3c73d 100644 --- a/Tests/SentryTests/Transaction/SentrySpanTests.swift +++ b/Tests/SentryTests/Transaction/SentrySpanTests.swift @@ -541,7 +541,7 @@ class SentrySpanTests: XCTestCase { private func givenFramesTracker() -> (TestDisplayLinkWrapper, SentryFramesTracker) { let displayLinkWrapper = TestDisplayLinkWrapper(dateProvider: self.fixture.currentDateProvider) - let framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: self.fixture.currentDateProvider, keepDelayedFramesDuration: 10) + let framesTracker = SentryFramesTracker(displayLinkWrapper: displayLinkWrapper, dateProvider: self.fixture.currentDateProvider, dispatchQueueWrapper: SentryDispatchQueueWrapper(), keepDelayedFramesDuration: 10) framesTracker.start() displayLinkWrapper.call() From 6129be508e6d2f43ede3ec580e3c3dc77206242e Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 29 Jan 2024 15:35:34 +0100 Subject: [PATCH 07/19] test: Add Scope maxBreadcrumb edge case tests (#3581) Add two test cases for edge cases when adding breadcrumbs. --- Tests/SentryTests/SentryScopeSwiftTests.swift | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Tests/SentryTests/SentryScopeSwiftTests.swift b/Tests/SentryTests/SentryScopeSwiftTests.swift index 2366d5c507b..0f3984560a1 100644 --- a/Tests/SentryTests/SentryScopeSwiftTests.swift +++ b/Tests/SentryTests/SentryScopeSwiftTests.swift @@ -1,3 +1,4 @@ +import Nimble import SentryTestUtils import XCTest @@ -303,6 +304,24 @@ class SentryScopeSwiftTests: XCTestCase { wait(for: [expect], timeout: 0.1) } + func testMaxBreadcrumbs_IsZero() { + let scope = Scope(maxBreadcrumbs: 0) + + scope.addBreadcrumb(fixture.breadcrumb) + + let serialized = scope.serialize() + expect(serialized["breadcrumbs"]) == nil + } + + func testMaxBreadcrumbs_IsNegative() { + let scope = Scope(maxBreadcrumbs: Int.min) + + scope.addBreadcrumb(fixture.breadcrumb) + + let serialized = scope.serialize() + expect(serialized["breadcrumbs"]) == nil + } + func testUseSpanForClear() { fixture.scope.span = fixture.transaction fixture.scope.useSpan { (_) in From f90bec5116f18e455bf64d542723065ed0bd5a93 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 29 Jan 2024 15:44:06 +0100 Subject: [PATCH 08/19] chore: Bump saucectl from 0.107.2 to 0.171.0 (#3580) 0.107.2 was released in Sep 2022. Let's bump to the latest version. --- .github/workflows/benchmarking.yml | 2 +- .github/workflows/saucelabs-UI-tests.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/benchmarking.yml b/.github/workflows/benchmarking.yml index 6a048b3c6f0..c9aae76deb1 100644 --- a/.github/workflows/benchmarking.yml +++ b/.github/workflows/benchmarking.yml @@ -93,7 +93,7 @@ jobs: - uses: actions/download-artifact@v4 with: name: DerivedData-Xcode - - run: npm install -g saucectl@0.107.2 + - run: npm install -g saucectl@0.171.0 - name: Run Benchmarks in SauceLab env: SAUCE_USERNAME: ${{ secrets.SAUCE_USERNAME }} diff --git a/.github/workflows/saucelabs-UI-tests.yml b/.github/workflows/saucelabs-UI-tests.yml index 056537f58b3..927ccfbe8f8 100644 --- a/.github/workflows/saucelabs-UI-tests.yml +++ b/.github/workflows/saucelabs-UI-tests.yml @@ -122,7 +122,7 @@ jobs: with: name: DerivedData-Xcode-${{ matrix.xcode }} - - run: npm install -g saucectl@0.107.2 + - run: npm install -g saucectl@0.171.0 # As Sauce Labs is a bit flaky we retry 5 times - name: Run Tests in SauceLab From 30f88719192689aa16af26a608618b857f59b9c5 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Mon, 29 Jan 2024 16:01:13 +0100 Subject: [PATCH 09/19] fix: Rare battery breadcrumbs crash (#3582) We see rare crashes in getBatteryStatus of SentrySystemEventBreadcrumbs. The object of an NSNotification can be nil, and we didn't check for that edge case. This PR fixes that problem and uses dictionaryWithCapacity to slightly reduce the memory footprint for getBatteryStatus. This change is not guaranteed to fix these rare crashes, but it won't do any harm. --- CHANGELOG.md | 1 + Sources/Sentry/SentrySystemEventBreadcrumbs.m | 18 ++++++++++++++---- .../SentrySystemEventBreadcrumbsTest.swift | 12 +++++++++++- 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 23f82ea9352..09e31a014d9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,7 @@ - Move header reference out of "extern C" (#3538) - Clarify FramesTracker log message (#3570) +- Fix rare battery breadcrumbs crash (#3582) - Fix synchronization issue in FramesTracker (#3571) ## 8.19.0 diff --git a/Sources/Sentry/SentrySystemEventBreadcrumbs.m b/Sources/Sentry/SentrySystemEventBreadcrumbs.m index dcdb672e562..520a14be4b1 100644 --- a/Sources/Sentry/SentrySystemEventBreadcrumbs.m +++ b/Sources/Sentry/SentrySystemEventBreadcrumbs.m @@ -102,8 +102,15 @@ - (void)initBatteryObserver:(UIDevice *)currentDevice - (void)batteryStateChanged:(NSNotification *)notification { // Notifications for battery level change are sent no more frequently than once per minute - NSMutableDictionary *batteryData = [self getBatteryStatus:notification.object]; - batteryData[@"action"] = @"BATTERY_STATE_CHANGE"; + UIDevice *currentDevice = notification.object; + // The object of an NSNotification may be nil. + if (currentDevice == nil) { + SENTRY_LOG_DEBUG( + @"UIDevice of NSNotification was nil. Won't create battery changed breadcrumb."); + return; + } + + NSDictionary *batteryData = [self getBatteryStatus:notification.object]; SentryBreadcrumb *crumb = [[SentryBreadcrumb alloc] initWithLevel:kSentryLevelInfo category:@"device.event"]; @@ -112,7 +119,7 @@ - (void)batteryStateChanged:(NSNotification *)notification [_delegate addBreadcrumb:crumb]; } -- (NSMutableDictionary *)getBatteryStatus:(UIDevice *)currentDevice +- (NSDictionary *)getBatteryStatus:(UIDevice *)currentDevice { // borrowed and adapted from // https://github.com/apache/cordova-plugin-battery-status/blob/master/src/ios/CDVBattery.m @@ -124,7 +131,8 @@ - (void)batteryStateChanged:(NSNotification *)notification isPlugged = YES; } float currentLevel = [currentDevice batteryLevel]; - NSMutableDictionary *batteryData = [NSMutableDictionary new]; + NSMutableDictionary *batteryData = + [NSMutableDictionary dictionaryWithCapacity:3]; // W3C spec says level must be null if it is unknown if ((currentState != UIDeviceBatteryStateUnknown) && (currentLevel != -1.0)) { @@ -135,6 +143,8 @@ - (void)batteryStateChanged:(NSNotification *)notification } batteryData[@"plugged"] = @(isPlugged); + batteryData[@"action"] = @"BATTERY_STATE_CHANGE"; + return batteryData; } diff --git a/Tests/SentryTests/Integrations/Breadcrumbs/SentrySystemEventBreadcrumbsTest.swift b/Tests/SentryTests/Integrations/Breadcrumbs/SentrySystemEventBreadcrumbsTest.swift index ffc8ee4cdee..e89c22fbecb 100644 --- a/Tests/SentryTests/Integrations/Breadcrumbs/SentrySystemEventBreadcrumbsTest.swift +++ b/Tests/SentryTests/Integrations/Breadcrumbs/SentrySystemEventBreadcrumbsTest.swift @@ -121,6 +121,16 @@ class SentrySystemEventBreadcrumbsTest: XCTestCase { assertBatteryBreadcrumb(charging: false, level: 100) } + func testBatteryUIDeviceNilNotification() { + let currentDevice = MyUIDevice() + + sut = fixture.getSut(currentDevice: currentDevice) + + postBatteryLevelNotification(uiDevice: nil) + + expect(self.fixture.delegate.addCrumbInvocations.count) == 0 + } + private func assertBatteryBreadcrumb(charging: Bool, level: Float) { XCTAssertEqual(1, fixture.delegate.addCrumbInvocations.count) @@ -273,7 +283,7 @@ class SentrySystemEventBreadcrumbsTest: XCTestCase { XCTAssertEqual(fixture.notificationCenterWrapper.removeObserverWithNameInvocations.count, 7) } - private func postBatteryLevelNotification(uiDevice: UIDevice) { + private func postBatteryLevelNotification(uiDevice: UIDevice?) { Dynamic(sut).batteryStateChanged(Notification(name: UIDevice.batteryLevelDidChangeNotification, object: uiDevice)) } From 720779678ec34ec3e7773ca28019a8d7a0ef2c27 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Mon, 29 Jan 2024 16:21:35 +0100 Subject: [PATCH 10/19] feat: Add visionOS as device family (#3548) Added visionOS as device family type. This will be used to filter events --- CHANGELOG.md | 1 + .../SentryCrash/Recording/Monitors/SentryCrashMonitor_System.m | 2 ++ Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h | 3 ++- 3 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 09e31a014d9..8d7ed09ca57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- Add visionOS as device family (#3548) - Add VisionOS Support for Carthage (#3565) ### Fixes diff --git a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_System.m b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_System.m index c223b777624..7920ae04183 100644 --- a/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_System.m +++ b/Sources/SentryCrash/Recording/Monitors/SentryCrashMonitor_System.m @@ -541,6 +541,8 @@ g_systemData.systemName = "macOS"; #elif SentryCrashCRASH_HOST_WATCH g_systemData.systemName = "watchOS"; +#elif SentryCrashCRASH_HOST_VISION + g_systemData.systemName = "visionOS"; #else g_systemData.systemName = "unknown"; #endif diff --git a/Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h b/Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h index bc5d54cdfa6..ad90dcf3de8 100644 --- a/Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h +++ b/Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h @@ -38,9 +38,10 @@ #define SentryCrashCRASH_HOST_IOS (SentryCrashCRASH_HOST_APPLE && TARGET_OS_IOS) #define SentryCrashCRASH_HOST_TV (SentryCrashCRASH_HOST_APPLE && TARGET_OS_TV) #define SentryCrashCRASH_HOST_WATCH (SentryCrashCRASH_HOST_APPLE && TARGET_OS_WATCH) +#define SentryCrashCRASH_HOST_VISION (SentryCrashCRASH_HOST_APPLE && TARGET_OS_VISION) #define SentryCrashCRASH_HOST_MAC \ (SentryCrashCRASH_HOST_APPLE && TARGET_OS_MAC \ - && !(TARGET_OS_IOS || TARGET_OS_TV || TARGET_OS_WATCH)) + && !(TARGET_OS_IOS || TARGET_OS_TV || TARGET_OS_WATCH || TARGET_OS_VISION)) #if SentryCrashCRASH_HOST_APPLE # define SentryCrashCRASH_CAN_GET_MAC_ADDRESS 1 From 52e491237a32581d5dfebccd918d7a8b2f278566 Mon Sep 17 00:00:00 2001 From: Dhiogo Brustolin Date: Tue, 30 Jan 2024 09:02:20 +0100 Subject: [PATCH 11/19] fix: TARGET_OS_VISION not defined for macOS (#3585) fix compilation on CI for macOS --- Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h b/Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h index ad90dcf3de8..14442ad3304 100644 --- a/Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h +++ b/Sources/SentryCrash/Recording/SentryCrashSystemCapabilities.h @@ -35,6 +35,10 @@ # define SentryCrashCRASH_HOST_ANDROID 1 #endif +#ifndef TARGET_OS_VISION +# define TARGET_OS_VISION 0 +#endif + #define SentryCrashCRASH_HOST_IOS (SentryCrashCRASH_HOST_APPLE && TARGET_OS_IOS) #define SentryCrashCRASH_HOST_TV (SentryCrashCRASH_HOST_APPLE && TARGET_OS_TV) #define SentryCrashCRASH_HOST_WATCH (SentryCrashCRASH_HOST_APPLE && TARGET_OS_WATCH) From 72f0262c12a37d0997c0bdc760d186b756c2d2cb Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 30 Jan 2024 13:19:54 +0100 Subject: [PATCH 12/19] build(deps): bump codecov/codecov-action from 3.1.4 to 3.1.5 (#3587) Bumps [codecov/codecov-action](https://github.com/codecov/codecov-action) from 3.1.4 to 3.1.5. - [Release notes](https://github.com/codecov/codecov-action/releases) - [Changelog](https://github.com/codecov/codecov-action/blob/main/CHANGELOG.md) - [Commits](https://github.com/codecov/codecov-action/compare/eaaf4bedf32dbdc6b720b63067d99c4d77d6047d...4fe8c5f003fae66aa5ebb77cfd3e7bfbbda0b6b0) --- updated-dependencies: - dependency-name: codecov/codecov-action dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- .github/workflows/test.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 9a441dd9307..82608c7c45b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -235,7 +235,7 @@ jobs: # We don't upload codecov for scheduled runs as CodeCov only accepts a limited amount of uploads per commit. - name: Push code coverage to codecov id: codecov_1 - uses: codecov/codecov-action@eaaf4bedf32dbdc6b720b63067d99c4d77d6047d # pin@v3.1.4 + uses: codecov/codecov-action@4fe8c5f003fae66aa5ebb77cfd3e7bfbbda0b6b0 # pin@v3.1.5 if: ${{ contains(matrix.platform, 'iOS') && !contains(github.ref, 'release') && github.event.schedule == '' }} with: # Although public repos should not have to specify a token there seems to be a bug with the Codecov GH action, which can @@ -247,7 +247,7 @@ jobs: # Sometimes codecov uploads etc can fail. Retry one time to rule out e.g. intermittent network failures. - name: Push code coverage to codecov id: codecov_2 - uses: codecov/codecov-action@eaaf4bedf32dbdc6b720b63067d99c4d77d6047d # pin@v3.1.4 + uses: codecov/codecov-action@4fe8c5f003fae66aa5ebb77cfd3e7bfbbda0b6b0 # pin@v3.1.5 if: ${{ steps.codecov_1.outcome == 'failure' && contains(matrix.platform, 'iOS') && !contains(github.ref, 'release') && github.event.schedule == '' }} with: token: ${{ secrets.CODECOV_TOKEN }} From 6e8d7e1390ba06aad185d29b593850d6c0df107e Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 31 Jan 2024 08:55:57 +0100 Subject: [PATCH 13/19] ref: Remove redundant nonnull in FileManager (#3591) SentryFileManager has NS_ASSUME_NONNULL_BEGIN at the beginning. No need to specify nonnull for createPathsWithOptions. --- Sources/Sentry/SentryFileManager.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 387de2c5c9f..59773dcb2a7 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -639,7 +639,7 @@ + (BOOL)createDirectoryAtPath:(NSString *)path withError:(NSError **)error #pragma mark private methods -- (void)createPathsWithOptions:(SentryOptions *_Nonnull)options +- (void)createPathsWithOptions:(SentryOptions *)options { NSString *cachePath = options.cacheDirectoryPath; From cff2b8c23990951a14068c85bc3d24e877012f64 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 31 Jan 2024 08:56:11 +0100 Subject: [PATCH 14/19] ref: FileManager remove redundant NS_SWIFT_NAME (#3593) --- Sources/Sentry/include/SentryFileManager.h | 1 - 1 file changed, 1 deletion(-) diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index 3bd3a908dfd..02452a5110d 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -13,7 +13,6 @@ NS_ASSUME_NONNULL_BEGIN @class SentryOptions; @class SentrySession; -NS_SWIFT_NAME(SentryFileManager) @interface SentryFileManager : NSObject SENTRY_NO_INIT From 13cd5a8cb62dfbba5331393baa2494dba5b3bbc7 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 31 Jan 2024 08:56:29 +0100 Subject: [PATCH 15/19] ref: Make allFilesInFolder internal (#3592) SentryFileManager.allFilesInFolder is only used by the SentryFileManager and can be internal. Furthermore, we can safely remove the test testAllFilesInFolder as that functionality is covered with other tests. --- Sources/Sentry/include/SentryFileManager.h | 2 -- Tests/SentryTests/Helper/SentryFileManagerTests.swift | 5 ----- 2 files changed, 7 deletions(-) diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index 02452a5110d..dc3f917f6b7 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -68,8 +68,6 @@ SENTRY_NO_INIT - (void)removeFileAtPath:(NSString *)path; -- (NSArray *)allFilesInFolder:(NSString *)path; - - (void)storeAppState:(SentryAppState *)appState; - (void)moveAppStateToPreviousAppState; - (SentryAppState *_Nullable)readAppState; diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 34b7739e33f..6bb724bd8b4 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -206,11 +206,6 @@ class SentryFileManagerTests: XCTestCase { try SentryFileManager.createDirectory(atPath: "a") } - func testAllFilesInFolder() { - let files = sut.allFiles(inFolder: "x") - XCTAssertTrue(files.isEmpty) - } - func testDeleteFileNotExists() { let logOutput = TestLogOutput() SentryLog.setLogOutput(logOutput) From e00303038646fa1870f491242187dc6afa1795ef Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 31 Jan 2024 08:56:45 +0100 Subject: [PATCH 16/19] test: Improve for CrashInstallationReporterTests (#3595) Use the same options when installing the SentryCrashInstallationReporter and starting the SDK so SentryCrashInstallationReporter.install doesn't have to retrieve the cacheDirectoryPath from the PrivateSentrySDKOnly. Furthermore, validate that the TestClient has a captured crash event. --- ...SentryCrashInstallationReporterTests.swift | 49 ++++++++----------- 1 file changed, 21 insertions(+), 28 deletions(-) diff --git a/Tests/SentryTests/SentryCrash/SentryCrashInstallationReporterTests.swift b/Tests/SentryTests/SentryCrash/SentryCrashInstallationReporterTests.swift index 5596e095eb3..adf6ebef499 100644 --- a/Tests/SentryTests/SentryCrash/SentryCrashInstallationReporterTests.swift +++ b/Tests/SentryTests/SentryCrash/SentryCrashInstallationReporterTests.swift @@ -1,21 +1,12 @@ +import Nimble @testable import Sentry import SentryTestUtils import XCTest class SentryCrashInstallationReporterTests: XCTestCase { - - private static let dsnAsString = TestConstants.dsnAsString(username: "SentryCrashInstallationReporterTests") - - private var testClient: TestClient! + private var sut: SentryCrashInstallationReporter! - - override func setUp() { - super.setUp() - sut = SentryCrashInstallationReporter(inAppLogic: SentryInAppLogic(inAppIncludes: [], inAppExcludes: []), crashWrapper: TestSentryCrashWrapper.sharedInstance(), dispatchQueue: TestSentryDispatchQueueWrapper()) - sut.install(PrivateSentrySDKOnly.options.cacheDirectoryPath) - // Works only if SentryCrash is installed - sentrycrash_deleteAllReports() - } + private var testClient: TestClient! override func tearDown() { super.tearDown() @@ -25,42 +16,44 @@ class SentryCrashInstallationReporterTests: XCTestCase { } func testReportIsSentAndDeleted() throws { - sdkStarted() + givenSutWithStartedSDK() try givenStoredSentryCrashReport(resource: "Resources/crash-report-1") sut.sendAllReports { filteredReports, _, _ in - XCTAssertEqual(1, filteredReports?.count) + expect(filteredReports?.count) == 1 } - assertNoReportsStored() + expect(self.testClient.captureCrashEventInvocations.count) == 1 + expect(sentrycrash_getReportCount()) == 0 } func testFaultyReportIsNotSentAndDeleted() throws { - sdkStarted() + givenSutWithStartedSDK() try givenStoredSentryCrashReport(resource: "Resources/Crash-faulty-report") sut.sendAllReports { filteredReports, _, _ in - XCTAssertEqual(0, filteredReports?.count) + expect(filteredReports?.count) == 0 } - assertNoReportsStored() + expect(self.testClient.captureCrashEventInvocations.count) == 0 + expect(sentrycrash_getReportCount()) == 0 } - private func sdkStarted() { - SentrySDK.start { options in - options.dsn = SentryCrashInstallationReporterTests.dsnAsString - options.setIntegrations([SentryCrashIntegration.self]) - } + private func givenSutWithStartedSDK() { let options = Options() - options.dsn = SentryCrashInstallationReporterTests.dsnAsString + options.dsn = TestConstants.dsnAsString(username: "SentryCrashInstallationReporterTests") + options.setIntegrations([SentryCrashIntegration.self]) + SentrySDK.start(options: options) + testClient = TestClient(options: options) let hub = SentryHub(client: testClient, andScope: nil) SentrySDK.setCurrentHub(hub) - } - - private func assertNoReportsStored() { - XCTAssertEqual(0, sentrycrash_getReportCount()) + + sut = SentryCrashInstallationReporter(inAppLogic: SentryInAppLogic(inAppIncludes: [], inAppExcludes: []), crashWrapper: TestSentryCrashWrapper.sharedInstance(), dispatchQueue: TestSentryDispatchQueueWrapper()) + sut.install(options.cacheDirectoryPath) + // Works only if SentryCrash is installed + sentrycrash_deleteAllReports() } } From 86f72498d8acd8128233833896f2b5307b4f02b7 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 31 Jan 2024 08:57:51 +0100 Subject: [PATCH 17/19] fix: FileManager log info when path doesn't exist (#3594) Logging an error when getting all files for a folder that doesn't exist confuses users; see GH-3577. Instead, log an info message that the SentryFileManager tried getting a list of files for a folder that doesn't exist. We chose info because it's still an edge case that shouldn't occur often, but it doesn't break any functionality. If a path was deleted manually by the user that the SDK requires, the SDK will create the path on next launch. We don't want to add checks if all the necessary paths exist every time the SDK stores an envelope cause this adds some overhead. If users manually remove the required paths, we accept that they have to wait until the next time the SDK launches. --- CHANGELOG.md | 1 + Sources/Sentry/SentryFileManager.m | 9 +++++++-- .../Helper/SentryFileManagerTests.swift | 17 +++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8d7ed09ca57..a6da588e30b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Clarify FramesTracker log message (#3570) - Fix rare battery breadcrumbs crash (#3582) - Fix synchronization issue in FramesTracker (#3571) +- Fix FileManager logs info instead of error when a path doesn't exist (#3594) ## 8.19.0 diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 59773dcb2a7..5cd5b2f3650 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -225,11 +225,16 @@ - (void)deleteAllEnvelopes - (NSArray *)allFilesInFolder:(NSString *)path { NSFileManager *fileManager = [NSFileManager defaultManager]; + if (![fileManager fileExistsAtPath:path]) { + SENTRY_LOG_INFO(@"Returning empty files list, as folder doesn't exist at path: %@", path); + return @[]; + } + NSError *error = nil; NSArray *storedFiles = [fileManager contentsOfDirectoryAtPath:path error:&error]; - if (nil != error) { + if (error != nil) { SENTRY_LOG_ERROR(@"Couldn't load files in folder %@: %@", path, error); - return [NSArray new]; + return @[]; } return [storedFiles sortedArrayUsingSelector:@selector(localizedCaseInsensitiveCompare:)]; } diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 6bb724bd8b4..94d5c1f44bb 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -1,3 +1,4 @@ +import Nimble import Sentry import SentryTestUtils import XCTest @@ -473,6 +474,22 @@ class SentryFileManagerTests: XCTestCase { XCTAssertEqual(0, sut.getAllEnvelopes().count) } + func testGetAllEnvelopesWhenNoEnvelopesPath_LogsInfoMessage() { + let logOutput = TestLogOutput() + SentryLog.setLogOutput(logOutput) + SentryLog.configure(true, diagnosticLevel: .debug) + + sut.deleteAllFolders() + sut.getAllEnvelopes() + + let debugLogMessages = logOutput.loggedMessages.filter { $0.contains("[Sentry] [info]") && $0.contains("Returning empty files list, as folder doesn't exist at path:") } + expect(debugLogMessages.count) == 1 + + let errorMessages = logOutput.loggedMessages.filter { $0.contains("[Sentry] [error]") } + + expect(errorMessages.count) == 0 + } + func testReadStoreDeleteAppState() { sut.store(TestData.appState) From f2daa680e1552830fd789d47831965cb614eb472 Mon Sep 17 00:00:00 2001 From: Philipp Hofmann Date: Wed, 31 Jan 2024 09:51:35 +0100 Subject: [PATCH 18/19] fix: Fix FileManager logs warning for .DS_Store (#3584) Don't log a warning in SentryFileManager for .DS_Store files. --- CHANGELOG.md | 1 + Sources/Sentry/SentryFileManager.m | 56 ++++++++++++----- Sources/Sentry/include/SentryFileManager.h | 6 ++ .../Helper/SentryFileManagerTests.swift | 63 +++++++++++++++++++ 4 files changed, 109 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a6da588e30b..13ab65918bf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,7 @@ - Clarify FramesTracker log message (#3570) - Fix rare battery breadcrumbs crash (#3582) - Fix synchronization issue in FramesTracker (#3571) +- Fix SentryFileManager logs warning for .DS_Files (#3584) - Fix FileManager logs info instead of error when a path doesn't exist (#3594) ## 8.19.0 diff --git a/Sources/Sentry/SentryFileManager.m b/Sources/Sentry/SentryFileManager.m index 5cd5b2f3650..30e9993aff0 100644 --- a/Sources/Sentry/SentryFileManager.m +++ b/Sources/Sentry/SentryFileManager.m @@ -167,29 +167,51 @@ - (SentryFileContents *_Nullable)getFileContents:(NSString *)folderPath - (void)deleteOldEnvelopesFromAllSentryPaths { // First we find all directories in the base path, these are all the various hashed DSN paths - for (NSString *path in [self allFilesInFolder:self.basePath]) { - NSString *fullPath = [self.basePath stringByAppendingPathComponent:path]; - NSDictionary *dict = [[NSFileManager defaultManager] attributesOfItemAtPath:fullPath - error:nil]; - if (!dict || dict[NSFileType] != NSFileTypeDirectory) { - SENTRY_LOG_WARN(@"Could not get NSFileTypeDirectory from %@", fullPath); - continue; - } - - // If the options don't have a DSN the sentry path doesn't contain a hash and the envelopes - // folder is stored in the base path. - NSString *envelopesPath; - if ([fullPath hasSuffix:EnvelopesPathComponent]) { - envelopesPath = fullPath; - } else { - envelopesPath = [fullPath stringByAppendingPathComponent:EnvelopesPathComponent]; - } + for (NSString *filePath in [self allFilesInFolder:self.basePath]) { + NSString *envelopesPath = [self getEnvelopesPath:filePath]; // Then we will remove all old items from the envelopes subdirectory [self deleteOldEnvelopesFromPath:envelopesPath]; } } +- (nullable NSString *)getEnvelopesPath:(NSString *)filePath +{ + NSString *fullPath = [self.basePath stringByAppendingPathComponent:filePath]; + + if ([fullPath hasSuffix:@".DS_Store"]) { + SENTRY_LOG_DEBUG( + @"Ignoring .DS_Store file when building envelopes path at path: %@", fullPath); + return nil; + } + + NSError *error = nil; + NSDictionary *dict = [[NSFileManager defaultManager] attributesOfItemAtPath:fullPath + error:&error]; + if (error != nil) { + SENTRY_LOG_WARN( + @"Could not get attributes of item at path: %@. Error: %@", fullPath, error); + return nil; + } + + if (dict[NSFileType] != NSFileTypeDirectory) { + SENTRY_LOG_DEBUG( + @"Ignoring non directory when deleting old envelopes at path: %@", fullPath); + return nil; + } + + // If the options don't have a DSN the sentry path doesn't contain a hash and the envelopes + // folder is stored in the base path. + NSString *envelopesPath; + if ([fullPath hasSuffix:EnvelopesPathComponent]) { + envelopesPath = fullPath; + } else { + envelopesPath = [fullPath stringByAppendingPathComponent:EnvelopesPathComponent]; + } + + return envelopesPath; +} + - (void)deleteOldEnvelopesFromPath:(NSString *)envelopesPath { NSTimeInterval now = diff --git a/Sources/Sentry/include/SentryFileManager.h b/Sources/Sentry/include/SentryFileManager.h index dc3f917f6b7..d4b36615c8f 100644 --- a/Sources/Sentry/include/SentryFileManager.h +++ b/Sources/Sentry/include/SentryFileManager.h @@ -16,6 +16,7 @@ NS_ASSUME_NONNULL_BEGIN @interface SentryFileManager : NSObject SENTRY_NO_INIT +@property (nonatomic, readonly) NSString *basePath; @property (nonatomic, readonly) NSString *sentryPath; @property (nonatomic, readonly) NSString *breadcrumbsFilePathOne; @property (nonatomic, readonly) NSString *breadcrumbsFilePathTwo; @@ -54,6 +55,11 @@ SENTRY_NO_INIT - (void)deleteOldEnvelopeItems; +/** + * Only used for testing. + */ +- (nullable NSString *)getEnvelopesPath:(NSString *)filePath; + /** * Get all envelopes sorted ascending by the @c timeIntervalSince1970 the envelope was stored and if * two envelopes are stored at the same time sorted by the order they were stored. diff --git a/Tests/SentryTests/Helper/SentryFileManagerTests.swift b/Tests/SentryTests/Helper/SentryFileManagerTests.swift index 94d5c1f44bb..39fb13664a3 100644 --- a/Tests/SentryTests/Helper/SentryFileManagerTests.swift +++ b/Tests/SentryTests/Helper/SentryFileManagerTests.swift @@ -145,6 +145,69 @@ class SentryFileManagerTests: XCTestCase { XCTAssertEqual(sut.getAllEnvelopes().count, 0) } + func testDeleteOldEnvelopes_LogsIgnoreDSStoreFiles() throws { + let logOutput = TestLogOutput() + SentryLog.setLogOutput(logOutput) + SentryLog.configure(true, diagnosticLevel: .debug) + + let dsStoreFile = "\(sut.basePath)/.DS_Store" + + let result = FileManager.default.createFile(atPath: dsStoreFile, contents: "some data".data(using: .utf8)) + expect(result) == true + + sut.deleteOldEnvelopeItems() + + let logMessages = logOutput.loggedMessages.filter { + $0.contains("[Sentry] [debug]") && + $0.contains("Ignoring .DS_Store file when building envelopes path at path: \(dsStoreFile)") + } + expect(logMessages.count) == 1 + + try FileManager.default.removeItem(atPath: dsStoreFile) + } + + func testDeleteOldEnvelopes_LogsDebugForTextFiles() throws { + let logOutput = TestLogOutput() + SentryLog.setLogOutput(logOutput) + SentryLog.configure(true, diagnosticLevel: .debug) + + let sut = fixture.getSut() + + let textFilePath = "\(sut.basePath)/something.txt" + + let result = FileManager.default.createFile(atPath: textFilePath, contents: "some data".data(using: .utf8)) + expect(result) == true + + sut.deleteOldEnvelopeItems() + + let logMessages = logOutput.loggedMessages.filter { + $0.contains("[Sentry] [debug]") && + $0.contains("Ignoring non directory when deleting old envelopes at path: \(textFilePath)") + } + expect(logMessages.count) == 1 + + try FileManager.default.removeItem(atPath: textFilePath) + } + + func testGetEnvelopesPath_ForNonExistentPath_LogsWarning() throws { + let logOutput = TestLogOutput() + SentryLog.setLogOutput(logOutput) + SentryLog.configure(true, diagnosticLevel: .debug) + + let sut = fixture.getSut() + + let nonExistentFile = "nonExistentFile.txt" + let nonExistentFileFullPath = "\(sut.basePath)/\(nonExistentFile)" + + expect(sut.getEnvelopesPath(nonExistentFile)) == nil + + let logMessages = logOutput.loggedMessages.filter { + $0.contains("[Sentry] [warning]") && + $0.contains("Could not get attributes of item at path: \(nonExistentFileFullPath)") + } + expect(logMessages.count) == 1 + } + func testDeleteOldEnvelopes_WithEmptyDSN() throws { fixture.options.dsn = nil sut = fixture.getSut() From b847a202a517a90763e8fd0656d8028aeee7b78d Mon Sep 17 00:00:00 2001 From: getsentry-bot Date: Thu, 1 Feb 2024 07:21:20 +0000 Subject: [PATCH 19/19] release: 8.20.0 --- CHANGELOG.md | 2 +- Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj | 8 ++++---- Sentry.podspec | 4 ++-- SentryPrivate.podspec | 2 +- SentrySwiftUI.podspec | 4 ++-- Sources/Configuration/Sentry.xcconfig | 2 +- Sources/Configuration/SentryPrivate.xcconfig | 2 +- Sources/Sentry/SentryMeta.m | 2 +- Tests/HybridSDKTest/HybridPod.podspec | 2 +- 9 files changed, 14 insertions(+), 14 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 13ab65918bf..c0160ae7816 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,6 @@ # Changelog -## Unreleased +## 8.20.0 ### Features diff --git a/Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj b/Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj index 4f50f3ca2a3..b722db68955 100644 --- a/Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj +++ b/Samples/iOS-Swift/iOS-Swift.xcodeproj/project.pbxproj @@ -1252,7 +1252,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 8.19.0; + MARKETING_VERSION = 8.20.0; PRODUCT_BUNDLE_IDENTIFIER = "io.sentry.sample.iOS-Swift"; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = "match Development io.sentry.sample.iOS-Swift"; @@ -1281,7 +1281,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 8.19.0; + MARKETING_VERSION = 8.20.0; PRODUCT_BUNDLE_IDENTIFIER = "io.sentry.sample.iOS-Swift"; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = "match AppStore io.sentry.sample.iOS-Swift"; @@ -1928,7 +1928,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 8.19.0; + MARKETING_VERSION = 8.20.0; PRODUCT_BUNDLE_IDENTIFIER = "io.sentry.sample.iOS-Swift.Clip"; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = "match Development io.sentry.sample.iOS-Swift.Clip"; @@ -1963,7 +1963,7 @@ "$(inherited)", "@executable_path/Frameworks", ); - MARKETING_VERSION = 8.19.0; + MARKETING_VERSION = 8.20.0; PRODUCT_BUNDLE_IDENTIFIER = "io.sentry.sample.iOS-Swift.Clip"; PRODUCT_NAME = "$(TARGET_NAME)"; PROVISIONING_PROFILE_SPECIFIER = "match AppStore io.sentry.sample.iOS-Swift.Clip"; diff --git a/Sentry.podspec b/Sentry.podspec index 21b0a5f3b8e..32d8ea835ad 100644 --- a/Sentry.podspec +++ b/Sentry.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = "Sentry" - s.version = "8.19.0" + s.version = "8.20.0" s.summary = "Sentry client for cocoa" s.homepage = "https://github.com/getsentry/sentry-cocoa" s.license = "mit" @@ -27,7 +27,7 @@ Pod::Spec.new do |s| } s.default_subspecs = ['Core'] - s.dependency "SentryPrivate", "8.19.0" + s.dependency "SentryPrivate", "8.20.0" s.subspec 'Core' do |sp| sp.source_files = "Sources/Sentry/**/*.{h,hpp,m,mm,c,cpp}", diff --git a/SentryPrivate.podspec b/SentryPrivate.podspec index 5d7d91acd43..73d82f6dda4 100644 --- a/SentryPrivate.podspec +++ b/SentryPrivate.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = "SentryPrivate" - s.version = "8.19.0" + s.version = "8.20.0" s.summary = "Sentry Private Library." s.homepage = "https://github.com/getsentry/sentry-cocoa" s.license = "mit" diff --git a/SentrySwiftUI.podspec b/SentrySwiftUI.podspec index 7fbff47cee4..4a9fb631c97 100644 --- a/SentrySwiftUI.podspec +++ b/SentrySwiftUI.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = "SentrySwiftUI" - s.version = "8.19.0" + s.version = "8.20.0" s.summary = "Sentry client for SwiftUI" s.homepage = "https://github.com/getsentry/sentry-cocoa" s.license = "mit" @@ -19,5 +19,5 @@ Pod::Spec.new do |s| s.watchos.framework = 'WatchKit' s.source_files = "Sources/SentrySwiftUI/**/*.{swift,h,m}" - s.dependency 'Sentry', "8.19.0" + s.dependency 'Sentry', "8.20.0" end diff --git a/Sources/Configuration/Sentry.xcconfig b/Sources/Configuration/Sentry.xcconfig index d4f0a90bbea..c69033f306c 100644 --- a/Sources/Configuration/Sentry.xcconfig +++ b/Sources/Configuration/Sentry.xcconfig @@ -2,6 +2,6 @@ PRODUCT_NAME = Sentry INFOPLIST_FILE = Sources/Resources/Info.plist PRODUCT_BUNDLE_IDENTIFIER = io.sentry.Sentry -CURRENT_PROJECT_VERSION = 8.19.0 +CURRENT_PROJECT_VERSION = 8.20.0 MODULEMAP_FILE = $(SRCROOT)/Sources/Resources/Sentry.modulemap diff --git a/Sources/Configuration/SentryPrivate.xcconfig b/Sources/Configuration/SentryPrivate.xcconfig index f718731ea75..199668b7f1c 100644 --- a/Sources/Configuration/SentryPrivate.xcconfig +++ b/Sources/Configuration/SentryPrivate.xcconfig @@ -1,3 +1,3 @@ PRODUCT_NAME = SentryPrivate MACH_O_TYPE = staticlib -CURRENT_PROJECT_VERSION = 8.19.0 +CURRENT_PROJECT_VERSION = 8.20.0 diff --git a/Sources/Sentry/SentryMeta.m b/Sources/Sentry/SentryMeta.m index d4a931ab9dc..9f7fc455186 100644 --- a/Sources/Sentry/SentryMeta.m +++ b/Sources/Sentry/SentryMeta.m @@ -5,7 +5,7 @@ @implementation SentryMeta // Don't remove the static keyword. If you do the compiler adds the constant name to the global // symbol table and it might clash with other constants. When keeping the static keyword the // compiler replaces all occurrences with the value. -static NSString *versionString = @"8.19.0"; +static NSString *versionString = @"8.20.0"; static NSString *sdkName = @"sentry.cocoa"; + (NSString *)versionString diff --git a/Tests/HybridSDKTest/HybridPod.podspec b/Tests/HybridSDKTest/HybridPod.podspec index f9d37b838bc..5eb2179d942 100644 --- a/Tests/HybridSDKTest/HybridPod.podspec +++ b/Tests/HybridSDKTest/HybridPod.podspec @@ -13,6 +13,6 @@ Pod::Spec.new do |s| s.requires_arc = true s.frameworks = 'Foundation' s.swift_versions = "5.5" - s.dependency "Sentry/HybridSDK", "8.19.0" + s.dependency "Sentry/HybridSDK", "8.20.0" s.source_files = "HybridTest.swift" end