Skip to content

fix(profiling): fix macOS launch profiling issue #5144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
acc595d
updated fix
armcknight Apr 27, 2025
5dc9a9b
add back file manager test
armcknight Apr 28, 2025
d6f7854
also use the static caches path for in options for default cache path
armcknight Apr 28, 2025
133b131
final fixes
armcknight Apr 28, 2025
90bf67d
changelog
armcknight Apr 28, 2025
802b95e
change sandbox check and custom path
philprime Apr 28, 2025
102d66e
fixed broken test
philprime Apr 28, 2025
cb2577e
reverting unrelated changes
philprime Apr 28, 2025
fa440ba
more reverting of unrelated changes
philprime Apr 28, 2025
d81524c
add comment for usage of NSProcessInfo instead of wrapper
philprime Apr 28, 2025
b51e3c1
refactored to be unit testable
philprime Apr 28, 2025
138d9ff
Update CHANGELOG.md
philprime Apr 28, 2025
0a1b91e
Fixed formatting
philprime Apr 28, 2025
0398fda
handle additional edge cases, and test all combinations
armcknight Apr 28, 2025
78a5a6d
remove the ci run of mac app ui tests for later branch
armcknight Apr 28, 2025
c0bb4b3
Merge branch 'armcknight/profiling/test/mac-app-launch-profiling-conf…
armcknight Apr 28, 2025
274aeb1
update changelog
armcknight Apr 29, 2025
97e7cf2
correctly gate test runs on ios/macos
armcknight Apr 29, 2025
142a3b9
change name of variable from sandbox to scoped
philprime Apr 29, 2025
6e936d2
Update Tests/SentryTests/Helper/SentryFileManagerTests.swift
philprime Apr 29, 2025
76adccd
added important note to unreleased changelog
philprime Apr 29, 2025
cbeaea5
revert fallback to keep using previously used executable path name
philprime Apr 29, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 0 additions & 39 deletions .github/workflows/ui-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -157,45 +157,6 @@ jobs:
~/Library/Logs/scan/*.log
./fastlane/test_output/**

ui-tests-macos:
name: UI Tests for macOS-Swift
runs-on: macos-15
steps:
- uses: actions/checkout@v4
- uses: ruby/setup-ruby@v1
with:
bundler-cache: true

- run: ./scripts/ci-select-xcode.sh "16.3"

- name: Run Fastlane
run: bundle exec fastlane ui_tests_ios_swift6

- name: Publish Test Report
uses: mikepenz/action-junit-report@cf701569b05ccdd861a76b8607a66d76f6fd4857 # v5.5.1
if: always()
with:
report_paths: "build/reports/junit.xml"
fail_on_failure: true
fail_on_parse_error: true
detailed_summary: true

- name: Upload Result Bundle
uses: actions/upload-artifact@v4
if: ${{ failure() }}
with:
name: ui-tests-ios-swift6.xcresult
path: fastlane/test_results/ui-tests-ios-swift6.xcresult

- name: Archiving Raw Test Logs
uses: actions/upload-artifact@v4
if: ${{ failure() || cancelled() }}
with:
name: ui-tests-ios-swift6-raw-output
path: |
~/Library/Logs/scan/*.log
./fastlane/test_output/**

ui-tests-swift6:
name: UI Tests for iOS-Swift6 - V3 # Up the version with every change to keep track of flaky tests
runs-on: macos-15
Expand Down
14 changes: 13 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,14 @@
# Changelog

## Unreleased

> ![Important]
> Version 8.21.0 introduced an issue for macOS apps that run without a sandbox (i.e. distributed outside the Mac App Store). We recommend upgrading to at least this version.

### Fixes

- Non-sandboxed macOS app launch profile configuration are now respected (#5144)

## 8.49.1

### Fixes
Expand All @@ -17,7 +26,7 @@
- New continuous profiling configuration API (#4952 and #5063)

> [!Important]
> With the addition of the new profiling configuation API, the previous profiling API are deprecated and will be removed in the next major version of the SDK:
> With the addition of the new profiling configuration API, the previous profiling API are deprecated and will be removed in the next major version of the SDK:
>
> - `SentryOptions.enableProfiling`
> - `SentryOptions.isProfilingEnabled`
Expand Down Expand Up @@ -897,6 +906,9 @@ The following two features, disabled by default, were mistakenly added to the re

## 8.21.0

> ![Important]
> This version introduced an issue for macOS apps that run without a sandbox (i.e. distributed outside the Mac App Store). We recommend upgrading to at least version 8.49.2.

### Features

- Add support for Sentry [Spotlight](https://spotlightjs.com/) (#3642), which is basically Sentry
Expand Down
2 changes: 1 addition & 1 deletion Samples/macOS-Swift/Shared/ViewController.swift
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ class ViewController: NSViewController {
if cachesDirectory.contains(bundleIdentifier) {
sandboxedCachesDirectory = cachesDirectory
} else {
sandboxedCachesDirectory = cachesDirectory
sandboxedCachesDirectory = (cachesDirectory as NSString).appendingPathComponent(bundleIdentifier)
}
return (sandboxedCachesDirectory as NSString).appendingPathComponent("io.sentry")
}
Expand Down
3 changes: 3 additions & 0 deletions SentryTestUtils/SentryFileManager+Test.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ NS_ASSUME_NONNULL_BEGIN

BOOL isErrorPathTooLong(NSError *error);
BOOL createDirectoryIfNotExists(NSString *path, NSError **error);
NSString *_Nullable sentryGetScopedCachesDirectory(NSString *cachesDirectory);
NSString *_Nullable sentryBuildScopedCachesDirectoryPath(NSString *cachesDirectory,
BOOL isSandboxed, NSString *_Nullable bundleIdentifier, NSString *_Nullable lastPathComponent);

SENTRY_EXTERN NSURL *launchProfileConfigFileURL(void);
SENTRY_EXTERN NSURL *_Nullable sentryLaunchConfigFileURL;
Expand Down
110 changes: 105 additions & 5 deletions Sources/Sentry/SentryFileManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -634,23 +634,116 @@
return [NSFileManager.defaultManager fileExistsAtPath:path isDirectory:&isDir] && isDir;
}

/**
* @note This method must be statically accessible because it will be called during app launch,
* before any instance of ``SentryFileManager`` exists, and so wouldn't be able to access this path
* from an objc property on it like the other paths.
*/
NSString *_Nullable sentryStaticCachesPath(void)
{
static NSString *_Nullable sentryStaticCachesPath = nil;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
NSString *cachesDirectory
// We request the users cache directory from Foundation.
// For iOS apps and macOS apps with sandboxing, this path will be scoped for the current
// app. For macOS apps without sandboxing, this path is not scoped and will be shared
// between all apps.
NSString *_Nullable cachesDirectory
= NSSearchPathForDirectoriesInDomains(NSCachesDirectory, NSUserDomainMask, YES)
.firstObject;
if (cachesDirectory == nil) {
SENTRY_LOG_WARN(@"No caches directory location reported.");
return;
}
sentryStaticCachesPath = cachesDirectory;

// We need to ensure our own scoped directory so that this path is not shared between other
// apps on the same system.
NSString *_Nullable scopedCachesDirectory = sentryGetScopedCachesDirectory(cachesDirectory);
if (!scopedCachesDirectory) {
SENTRY_LOG_WARN(@"Failed to get scoped static caches directory.");
return;

Check warning on line 664 in Sources/Sentry/SentryFileManager.m

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryFileManager.m#L664

Added line #L664 was not covered by tests
}
sentryStaticCachesPath = scopedCachesDirectory;
SENTRY_LOG_DEBUG(@"Using static cache directory: %@", sentryStaticCachesPath);
});
return sentryStaticCachesPath;
}

NSString *_Nullable sentryGetScopedCachesDirectory(NSString *cachesDirectory)
{
#if !TARGET_OS_OSX
// iOS apps are always sandboxed, therefore we can just early-return with the provided caches
// directory.
return cachesDirectory;
#else

// For macOS apps, we need to ensure our own sandbox so that this path is not shared between
// all apps that ship the SDK.

// We can not use the SentryNSProcessInfoWrapper here because this method is called before
// the SentryDependencyContainer is initialized.
NSProcessInfo *processInfo = [NSProcessInfo processInfo];

// Only apps running in a sandboxed environment have the `APP_SANDBOX_CONTAINER_ID` set as a
// process environment variable. Reference implementation:
// https://github.com/realm/realm-js/blob/a03127726939f08f608edbdb2341605938f25708/packages/realm/binding/apple/platform.mm#L58-L74
BOOL isSandboxed = processInfo.environment[@"APP_SANDBOX_CONTAINER_ID"] != nil;

// The bundle identifier is used to create a unique cache directory for the app.
// If the bundle identifier is not available, we use the name of the executable.
// Note: `SentryCrash.getBundleName` is using `CFBundleName` to create a scoped directory.
// That value can be absent, therefore we use a more stable approach here.
NSString *bundleIdentifier = [[NSBundle mainBundle] bundleIdentifier];
NSString *lastPathComponent = [[[NSBundle mainBundle] executablePath] lastPathComponent];

// Due to `NSProcessInfo` and `NSBundle` not being mockable in unit tests, we extract only the
// logic to a separate function.
return sentryBuildScopedCachesDirectoryPath(
cachesDirectory, isSandboxed, bundleIdentifier, lastPathComponent);
#endif
}

NSString *_Nullable sentryBuildScopedCachesDirectoryPath(NSString *cachesDirectory,
BOOL isSandboxed, NSString *_Nullable bundleIdentifier, NSString *_Nullable lastPathComponent)
{
// If the app is sandboxed, we can just use the provided caches directory.
if (isSandboxed) {
return cachesDirectory;
}

// If the macOS app is not sandboxed, we need to manually create a scoped cache
// directory. The cache path must be unique an stable over app launches, therefore we
// can not use any changing identifier.
SENTRY_LOG_DEBUG(
@"App is not sandboxed, extending default cache directory with bundle identifier.");
NSString *_Nullable identifier = bundleIdentifier;
if (identifier == nil) {
SENTRY_LOG_WARN(@"No bundle identifier found, using main bundle executable name.");
identifier = lastPathComponent;
} else if (identifier.length == 0) {
SENTRY_LOG_WARN(@"Bundle identifier exists but is zero length, using main bundle "
@"executable name.");
identifier = lastPathComponent;
}

// If neither the bundle identifier nor the executable name are available, we can't
// create a unique and stable cache directory.
// We do not fall back to any default path, because it could be shared with other apps
// and cause leaks impacting other apps.
if (identifier == nil) {
SENTRY_LOG_ERROR(@"No bundle identifier found, cannot create cache directory.");
return nil;
}

// It's unlikely that the executable name will be zero length, but we'll cover this case anyways
if (identifier.length == 0) {
SENTRY_LOG_ERROR(@"Executable name was zero length.");
return nil;
}

return [cachesDirectory stringByAppendingPathComponent:identifier];
}

NSString *_Nullable sentryStaticBasePath(void)
{
static NSString *_Nullable sentryStaticBasePath = nil;
Expand Down Expand Up @@ -684,13 +777,20 @@
{
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
NSString *cachesPath = sentryStaticBasePath();
if (cachesPath == nil) {
NSString *basePath = sentryStaticBasePath();
if (basePath == nil) {
SENTRY_LOG_WARN(@"No location available to write a launch profiling config.");
return;
}
NSError *error;
if (!createDirectoryIfNotExists(basePath, &error)) {
SENTRY_LOG_ERROR(
@"Can't create base path to store launch profile config file: %@", error);
return;

Check warning on line 789 in Sources/Sentry/SentryFileManager.m

View check run for this annotation

Codecov / codecov/patch

Sources/Sentry/SentryFileManager.m#L788-L789

Added lines #L788 - L789 were not covered by tests
}
sentryLaunchConfigFileURL =
[NSURL fileURLWithPath:[cachesPath stringByAppendingPathComponent:@"profileLaunch"]];
[NSURL fileURLWithPath:[basePath stringByAppendingPathComponent:@"profileLaunch"]];
SENTRY_LOG_DEBUG(@"Launch profile config file URL: %@", sentryLaunchConfigFileURL);
});
return sentryLaunchConfigFileURL;
}
Expand Down
3 changes: 2 additions & 1 deletion Sources/Sentry/include/SentryFileManager.h
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,8 @@ BOOL createDirectoryIfNotExists(NSString *path, NSError **error);
/**
* Path for a default directory Sentry can use in the app sandbox' caches directory.
* @note This method must be statically accessible because it will be called during app launch,
* before any instance of @c SentryFileManager exists, and so wouldn't be able to access this path
* before any instance of @c SentryFileManager exists, and so wouldn't be able to access this path.
* @note For unsandboxed macOS apps, the path has the form @c ~/Library/Caches/<app-bundle-id> .
* from an objc property on it like the other paths. It also cannot use
* @c SentryOptions.cacheDirectoryPath since this can be called before
* @c SentrySDK.startWithOptions .
Expand Down
94 changes: 94 additions & 0 deletions Tests/SentryTests/Helper/SentryFileManagerTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -1209,6 +1209,100 @@ extension SentryFileManagerTests {
// set the original value back so other tests don't crash
sentryLaunchConfigFileURL = (originalURL as NSURL)
}

func testSentryGetScopedCachesDirectory_targetIsNotMacOS_shouldReturnSamePath() throws {
#if os(macOS)
throw XCTSkip("Test is disabled for macOS")
#else
// -- Arrange --
let cachesDirectoryPath = "some/path/to/caches"

// -- Act --
let result = sentryGetScopedCachesDirectory(cachesDirectoryPath)

// -- Assert
XCTAssertEqual(result, cachesDirectoryPath)
#endif // os(macOS)
}

func testSentryGetScopedCachesDirectory_targetIsMacOS_shouldReturnPath() throws {
#if !os(macOS)
throw XCTSkip("Test is disabled for non macOS")
#else
// -- Arrange --
let cachesDirectoryPath = "some/path/to/caches"

// -- Act --
let result = sentryGetScopedCachesDirectory(cachesDirectoryPath)

// -- Assert
// Xcode unit tests are not sandboxed, therefore we expect it to use the bundle identifier to unique the path
// The bundle identifier will then be the xctest bundle identifier
XCTAssertEqual(result, "some/path/to/caches/com.apple.dt.xctest.tool")
#endif // os(macOS)

}

func testSentryBuildScopedCachesDirectoryPath_isSandboxed_shouldReturnInputPath() {
// -- Arrange --
let cachesDirectoryPath = "some/path/to/caches"
let isSandboxed = true
let bundleIdentifier: String? = nil
let lastPathComponent: String? = nil

// -- Act --
let result = sentryBuildScopedCachesDirectoryPath(
cachesDirectoryPath,
isSandboxed,
bundleIdentifier,
lastPathComponent
)

// -- Assert --
XCTAssertEqual(result, cachesDirectoryPath)
}

func test_sentryBuildScopedCachesDirectoryPath_inputCombinations() {
// -- Arrange --
for testCase: (isSandboxed: Bool, bundleIdentifier: String?, lastPathComponent: String?, expected: String?) in [
// bundleIdentifier defined
(isSandboxed: false, bundleIdentifier: "com.example.app", lastPathComponent: "AppBinaryName", expected: "some/path/to/caches/com.example.app"),
(isSandboxed: false, bundleIdentifier: "com.example.app", lastPathComponent: "", expected: "some/path/to/caches/com.example.app"),
(isSandboxed: false, bundleIdentifier: "com.example.app", lastPathComponent: nil, expected: "some/path/to/caches/com.example.app"),

// bundleIdentifier zero length string
(isSandboxed: false, bundleIdentifier: "", lastPathComponent: "AppBinaryName", expected: "some/path/to/caches/AppBinaryName"),
(isSandboxed: false, bundleIdentifier: "", lastPathComponent: "", expected: nil),
(isSandboxed: false, bundleIdentifier: "", lastPathComponent: nil, expected: nil),

// bundleIdentifier nil
(isSandboxed: false, bundleIdentifier: nil, lastPathComponent: "AppBinaryName", expected: "some/path/to/caches/AppBinaryName"),
(isSandboxed: false, bundleIdentifier: nil, lastPathComponent: "", expected: nil),
(isSandboxed: false, bundleIdentifier: nil, lastPathComponent: nil, expected: nil),

// for sandboxed scenarios, always return the original path
(isSandboxed: true, bundleIdentifier: "com.example.app", lastPathComponent: "AppBinaryName", expected: "some/path/to/caches"),
(isSandboxed: true, bundleIdentifier: "", lastPathComponent: "AppBinaryName", expected: "some/path/to/caches"),
(isSandboxed: true, bundleIdentifier: nil, lastPathComponent: "AppBinaryName", expected: "some/path/to/caches"),
(isSandboxed: true, bundleIdentifier: "com.example.app", lastPathComponent: "", expected: "some/path/to/caches"),
(isSandboxed: true, bundleIdentifier: "", lastPathComponent: "", expected: "some/path/to/caches"),
(isSandboxed: true, bundleIdentifier: nil, lastPathComponent: "", expected: "some/path/to/caches"),
(isSandboxed: true, bundleIdentifier: "com.example.app", lastPathComponent: nil, expected: "some/path/to/caches"),
(isSandboxed: true, bundleIdentifier: "", lastPathComponent: nil, expected: "some/path/to/caches"),
(isSandboxed: true, bundleIdentifier: nil, lastPathComponent: nil, expected: "some/path/to/caches")
] {
// -- Act --
let result = sentryBuildScopedCachesDirectoryPath(
"some/path/to/caches",
testCase.isSandboxed,
testCase.bundleIdentifier,
testCase.lastPathComponent
)

// -- Assert --
XCTAssertEqual(result, testCase.expected, "Inputs: (isSandboxed: \(testCase.isSandboxed), bundleIdentifier: \(String(describing: testCase.bundleIdentifier)), lastPathComponent: \(String(describing: testCase.lastPathComponent)), expected: \(String(describing: testCase.expected))); Output: \(String(describing: result))")
}
}
}

// MARK: Private profiling tests
Expand Down
Loading