KSCrash: install crash handler - #8469
Conversation
a053c2e to
dbbb96c
Compare
📲 Install BuildsiOS
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6beeea8 | 1220.50 ms | 1253.93 ms | 33.43 ms |
| 786f763 | 1217.17 ms | 1238.37 ms | 21.20 ms |
| 4c437ea | 1241.63 ms | 1266.20 ms | 24.57 ms |
| 4b5a1d7 | 1226.86 ms | 1257.86 ms | 31.01 ms |
| 1ec4cb5 | 1218.90 ms | 1255.53 ms | 36.63 ms |
| b32b02f | 1227.11 ms | 1249.68 ms | 22.57 ms |
| 6844d69 | 1228.25 ms | 1265.20 ms | 36.95 ms |
| 50fe369 | 1226.48 ms | 1268.96 ms | 42.48 ms |
| 5df460d | 1230.49 ms | 1260.69 ms | 30.20 ms |
| 5804f33 | 1218.24 ms | 1247.67 ms | 29.42 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6beeea8 | 24.14 KiB | 1.23 MiB | 1.21 MiB |
| 786f763 | 24.14 KiB | 1.15 MiB | 1.13 MiB |
| 4c437ea | 24.14 KiB | 1.18 MiB | 1.15 MiB |
| 4b5a1d7 | 24.14 KiB | 1.22 MiB | 1.20 MiB |
| 1ec4cb5 | 24.14 KiB | 1.22 MiB | 1.20 MiB |
| b32b02f | 24.14 KiB | 1.22 MiB | 1.20 MiB |
| 6844d69 | 24.14 KiB | 1.20 MiB | 1.17 MiB |
| 50fe369 | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| 5df460d | 24.14 KiB | 1.22 MiB | 1.20 MiB |
| 5804f33 | 24.14 KiB | 1.18 MiB | 1.16 MiB |
Previous results on branch: feat/kscrash-install
Startup times
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 38dde6e | 1219.47 ms | 1245.47 ms | 26.00 ms |
| eb9df97 | 1229.12 ms | 1270.14 ms | 41.03 ms |
| 73b2aec | 1221.31 ms | 1255.80 ms | 34.48 ms |
| 1533d6b | 1221.05 ms | 1248.71 ms | 27.67 ms |
| de50b03 | 1208.60 ms | 1250.48 ms | 41.88 ms |
| dad417c | 1233.31 ms | 1265.30 ms | 31.99 ms |
| 5e2cca0 | 1221.53 ms | 1251.69 ms | 30.16 ms |
| 0b7f67b | 1213.28 ms | 1256.58 ms | 43.30 ms |
| 000de32 | 1226.57 ms | 1259.02 ms | 32.46 ms |
| 06ac64e | 1228.92 ms | 1265.04 ms | 36.12 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 38dde6e | 24.14 KiB | 1.24 MiB | 1.21 MiB |
| eb9df97 | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| 73b2aec | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| 1533d6b | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| de50b03 | 24.14 KiB | 1.25 MiB | 1.23 MiB |
| dad417c | 24.14 KiB | 1.24 MiB | 1.21 MiB |
| 5e2cca0 | 24.14 KiB | 1.24 MiB | 1.21 MiB |
| 0b7f67b | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| 000de32 | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| 06ac64e | 24.14 KiB | 1.24 MiB | 1.21 MiB |
3f75d62 to
f60c3b2
Compare
f60c3b2 to
01119c1
Compare
8694ae2 to
1a058e6
Compare
3e308ff to
5f68bc1
Compare
…ntegration Adds KSCrash crash handler installation to SentryKSCrashIntegration. - Defines KSCrashInstalling/KSCrashInstallerProvider protocols using only primitive types (String, UInt, Bool), so SentryTestUtils can conform without importing KSCrash. - Adds production KSCrashInstaller wrapping KSCrash.shared.install(with:). - Implements init in SentryKSCrashIntegration: installs with MonitorType.productionSafeMinimal monitors, respects cacheDirectoryPath and experimental.enableUnhandledCPPExceptionsV2, sets crashReporterInstalled and fatalDetected flags. - Wires SentryDependencyContainer to KSCrashInstallerProvider. - Adds TestKSCrashInstaller in SentryTestUtils and four integration tests.
Swift Testing relies on Swift concurrency primitives (Actor, isolation()) which require macOS 10.15+. The KSCrash integration tests were rewritten to use Swift Testing, causing the TestV10KSCrash macOS build to fail with: 'Actor' is only available in macOS 10.15 or newer 'isolation()' is only available in macOS 10.15 or newer SentryTests.xcconfig includes DeploymentTargets.xcconfig (which sets 10.14) and then overrides MACOSX_DEPLOYMENT_TARGET to 10.15. This means all test targets (SentryTests and SentryTests+KSCrash, via SentryTestsKSCrash.xcconfig) pick up 10.15 while the SDK framework targets are unaffected — they include DeploymentTargets.xcconfig directly without going through SentryTests.xcconfig.
Reverts two commits: - 'test: update to use swift testing because the future is now' (d40cf78) — rewrote KSCrash integration tests with Swift Testing framework (@test, #expect, struct suites, Tag extension) - 'build: bump macOS deployment target to 10.15 for test targets only' (46e7cef) — was required because Swift Testing's concurrency APIs (Actor, isolation()) don't compile below macOS 10.15 Restores XCTestCase subclass with XCTAssert* assertions, removes the MACOSX_DEPLOYMENT_TARGET = 10.15 override from SentryTests.xcconfig, restores @_spi(Private) public on MockKSCrashInstaller, and removes the Swift Testing ONLY_TESTING note from Tests/AGENTS.md.
Apply the test<Function>_when<Condition>_should<Expected>() pattern
from Tests/AGENTS.md to all four KSCrash integration tests:
testInstallCalledOnInit
→ testInstall_whenCrashHandlerEnabled_shouldCallInstallOnce
testCrashedLastLaunchSetsFlag
→ testInstall_whenCrashedLastLaunch_shouldSetFatalDetected
testInstallFailureReturnsNil
→ testInstall_whenInstallThrows_shouldReturnNil
testInitSkipsWhenCrashHandlerDisabled
→ testInstall_whenCrashHandlerDisabled_shouldSkipInstall
MonitorType comes from KSCrashInstallations, which is imported @_implementationOnly in both SentryKSCrash+Integration.swift and SentryKSCrash+Installer.swift. @_implementationOnly makes the type completely invisible outside the Sentry module — even to @testable importers — so the test could not resolve MonitorType when it tried to call .rawValue on productionSafeMonitors. Fix: declare productionSafeMonitors as UInt (the raw type), computing it once with MonitorType([...]).rawValue inside the module where KSCrashInstallations is visible. The internal call site drops the now- redundant .rawValue suffix, and the test assertion compares UInt to UInt directly.
The integration appends 'KSCrash/<bundleIdentifier>' to the SDK's cacheDirectoryPath before passing it to the installer. The existing 'shouldCallInstallOnce' test was asserting installPath == cacheDirectoryPath directly, which became stale once the path was modified. Fix the stale assertion and add a dedicated test that mirrors the same URL construction, asserting the installer receives the expected subdirectory path.
fatalDetected is also set by things like the watchdog integration
…internal imports are different than @_implementationOnly
7798bde to
207f6c1
Compare
philipphofmann
left a comment
There was a problem hiding this comment.
Thanks, I added a few comments.
| "SentryKSCrashIntegration" | ||
| } | ||
|
|
||
| func uninstall() {} |
There was a problem hiding this comment.
h: I guess we must set SentrySDKInternal.crashReporterInstalled = false here. BTW, the SentryCrashIntegration also doesn't call this on uninstall. Maybe worth double checking and fixing in another PR.
| if dependencies.kscrashInstaller.crashedLastLaunch { | ||
| SentrySDKInternal.fatalDetected = true | ||
| SentrySDKInternal.crashHandlerDetectedCrash = true | ||
| } |
There was a problem hiding this comment.
m: I don't really like that approach. We set these boolean flags here based on a flag during install. I think it would be better if these two properties fatalDetected and crashHandlerDetectedCrash would call kscrashInstaller.crashedLastLaunch instead. Then they always represent the current state, and they can't get out of sync due to race conditions or whatever. This approach already exists in the SentryCrashIntegration, so we would also have to fix it there.
There was a problem hiding this comment.
For SentryCrash this is quite easy as I can expose the g_installed bool via a new function. For KSCrash this will be a little more tricky as it's currently pure Swift and self-contained, so will need some additional abstraction to expose to ObjC nicely (it would be easiest to have a isInstalled computed variable on the dependency container - but then the dependency container does 'more' than just holding and providing dependencies. Will play around a little before deciding exact direction, am just dropping my musings on it
| return SentryLastRunStatusDidCrash; | ||
| } | ||
| return SentryLastRunStatusDidNotCrash; | ||
| return crashHandlerDetectedCrash ? SentryLastRunStatusDidCrash : SentryLastRunStatusDidNotCrash; |
There was a problem hiding this comment.
h: If I'm not mistaken, this is a behavioral change. If the crash integration doesn't install (e.g. enableCrashHandler = false), crashedLastRun returns false even if a crash file exists on disk — previously it read the reporter directly, if I'm not mistaken.
…lding logic add tests to cover the happy and unhappy paths
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 0c228c3. Configure here.
philipphofmann
left a comment
There was a problem hiding this comment.
Thanks, I added a few more comments.
| { | ||
| if (!crashReporterInstalled) { | ||
| if (![SentrySDKInternal crashReporterInstalled]) { | ||
| return SentryLastRunStatusUnknown; | ||
| } | ||
|
|
||
| #if ENABLE_KSCRASH | ||
| if (SentryDependencyContainer.sharedInstance.kscrashQuery.crashedLastLaunch) { | ||
| return SentryLastRunStatusDidCrash; | ||
| } | ||
| #else |
There was a problem hiding this comment.
Bug: When ENABLE_KSCRASH is active, crashedLastRun incorrectly reads from the disabled SentryCrash reporter, always returning false even after a crash.
Severity: MEDIUM
Suggested Fix
Use an #if ENABLE_KSCRASH preprocessor directive for the crashedLastRun property. When the flag is active, the property should get its value from SentryDependencyContainer.sharedInstance.kscrashQuery.crashedLastLaunch. Otherwise, it should use the existing SentryDependencyContainer.sharedInstance.crashReporter.crashedLastLaunch.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: Sources/Sentry/SentrySDKInternal.m#L510-L519
Potential issue: When the `ENABLE_KSCRASH` preprocessor flag is active, the
`crashedLastRun` property incorrectly continues to read from the `SentryCrash` reporter
via `crashReporter.crashedLastLaunch`. However, with `ENABLE_KSCRASH` enabled, the
`SentryCrashIntegration` is not installed, meaning the `crashReporter` is never set up.
Consequently, `crashedLastRun` will always return `false`, even if the app actually
crashed. This is inconsistent with other properties like `lastRunStatus`, which
correctly use a conditional compilation block to read from the appropriate KSCrash
source.
There was a problem hiding this comment.
i guess this is because +(BOOL) crashedLastRun is just above this, but it's compiled out in SDK_V10 mode (which ENABLE_KSCRASH includes).

📜 Description
This change installs the KSCrash handler with a configuration as close to SentryCrash as possible
💡 Motivation and Context
This just installs the handler, it doesn't attempt to catch crash reports, to sink them, or operate on them in anyway - it lays the ground work for the next set of PRs.
💚 How did you test it?
📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.#skip-changelog (v10 only)
Closes #8495