-
-
Notifications
You must be signed in to change notification settings - Fork 372
fix(session-replay): Add options to ignore views from subtree traversal #7063
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
base: v8.x
Are you sure you want to change the base?
Conversation
- Introduced a new property `subtreeTraversalIgnoredViewTypes` to the `SentryRedactOptions` protocol and its implementations, allowing specific view types to be excluded from subtree traversal during redaction. - Default implementation includes `CameraUI.ChromeSwiftUIView` on iOS 26+ to prevent crashes related to accessing problematic layers. - Updated related classes and tests to support the new functionality, ensuring robust handling of view hierarchies during redaction.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v8.x #7063 +/- ##
=============================================
+ Coverage 85.959% 86.027% +0.068%
=============================================
Files 440 440
Lines 27506 27555 +49
Branches 11891 11899 +8
=============================================
+ Hits 23644 23705 +61
+ Misses 3817 3807 -10
+ Partials 45 43 -2
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Sources/Swift/Integrations/Screenshot/SentryScreenshotOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/Screenshot/SentryScreenshotOptions.swift
Outdated
Show resolved
Hide resolved
- Added options `options.sessionReplay.includeSubtreeTraversalForViewType` and `options.sessionReplay.excludeSubtreeTraversalForViewType` to allow ignoring specific views during subtree traversal. - Updated default handling for `CameraUI.ChromeSwiftUIView` to prevent crashes on iOS 26+.
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Integrations/SessionReplay/SentryReplayOptions.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift
Outdated
Show resolved
Hide resolved
philipphofmann
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are all the changes in sdk_api_V9.json and sdk_api.json intended? If yes, does it make make sense to open extra PRs for this?
The changes LGTM, but I have a few high-level questions.
|
@philipphofmann and I have discussed the pull request review, and the confusion I need to resolve is mainly around terminology and mental models exposed to the user. The core issue is that two conceptually different mechanisms are currently perceived as related because of naming:
The confusion arises because:
The action item on my side is therefore to:
|
…actOptions - Replaced `viewTypesIgnoredFromSubtreeTraversal` with `excludedViewClasses` and added `includedViewClasses` to allow more granular control over subtree traversal during redaction. - Updated documentation to clarify the behavior of these new properties, including partial string matching for view class names. - Adjusted related classes and tests to reflect these changes, ensuring consistent handling of view hierarchies.
|
That #7063 (comment) summarizes it pretty well @philprime what we discussed 👏 ⬆️ |
Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift
Outdated
Show resolved
Hide resolved
🚨 Detected changes in high risk code 🚨High-risk code can easily blow up and is hard to test. We had severe bugs in the past. Be extra careful when changing these files, and have an extra careful look at these:
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 5e3fb04 | 1239.84 ms | 1267.39 ms | 27.55 ms |
| b66be9b | 1218.22 ms | 1244.19 ms | 25.96 ms |
| f73c5c8 | 1211.10 ms | 1234.62 ms | 23.51 ms |
| 237dfb1 | 1214.90 ms | 1258.63 ms | 43.73 ms |
| c21a31f | 1216.02 ms | 1236.34 ms | 20.32 ms |
| 3af1ae9 | 1225.60 ms | 1252.65 ms | 27.05 ms |
| c21a31f | 1237.04 ms | 1256.65 ms | 19.61 ms |
| 653de7c | 1205.02 ms | 1222.20 ms | 17.18 ms |
| e537c90 | 1226.22 ms | 1256.64 ms | 30.41 ms |
| 48dc176 | 1204.96 ms | 1237.73 ms | 32.77 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 5e3fb04 | 23.74 KiB | 981.30 KiB | 957.56 KiB |
| b66be9b | 23.75 KiB | 996.03 KiB | 972.28 KiB |
| f73c5c8 | 24.14 KiB | 1001.32 KiB | 977.18 KiB |
| 237dfb1 | 23.75 KiB | 1000.79 KiB | 977.04 KiB |
| c21a31f | 23.75 KiB | 1000.80 KiB | 977.05 KiB |
| 3af1ae9 | 23.74 KiB | 981.29 KiB | 957.55 KiB |
| c21a31f | 23.75 KiB | 1000.77 KiB | 977.02 KiB |
| 653de7c | 23.75 KiB | 992.25 KiB | 968.50 KiB |
| e537c90 | 23.75 KiB | 992.03 KiB | 968.28 KiB |
| 48dc176 | 24.14 KiB | 1001.34 KiB | 977.20 KiB |
This PR should be merged after #7089
📜 Description
This PR adds configurable subtree traversal ignoring functionality to prevent crashes when traversing problematic view hierarchies during session replay and screenshot capture.
Key Changes:
viewTypesIgnoredFromSubtreeTraversal: Set<String>property toSentryRedactOptionsprotocolSentryReplayOptions- with helper methodsexcludeViewTypeFromSubtreeTraversal(_:)andincludeViewTypeInSubtreeTraversal(_:)SentryRedactDefaultOptions- computed property with iOS 26+ defaultSentryViewScreenshotOptions- mutable property with empty defaultPreviewRedactOptions- immutable property with default fromSentryReplayOptionsSentryUIRedactBuilderto check ignored view types before accessinglayer.sublayers, preventing crashesviewTypesIgnoredFromSubtreeTraversalinSentryReplayOptionsandSentryViewScreenshotOptionsDefault Behavior:
CameraUI.ChromeSwiftUIViewis automatically included in the ignore set by default forSentryReplayOptionsandSentryRedactDefaultOptionsto prevent crashes when accessingCameraUI.ModeLoupeLayerSentryViewScreenshotOptionshas an empty ignore set by defaultImplementation Details:
When a view type is in the ignore set,
SentryUIRedactBuilderwill:UISwitch)💡 Motivation and Context
Closes #7053
Some view hierarchies contain layers that crash when accessed during traversal. Specifically,
CameraUI.ChromeSwiftUIViewon iOS 26+ containsCameraUI.ModeLoupeLayerinstances that cause fatal errors when theirsublayersproperty is accessed, resulting in crashes during session replay or screenshot capture.This change provides a configurable way to exclude problematic view types from subtree traversal while still redacting the views themselves.
💚 How did you test it?
CameraUI.ChromeSwiftUIViewis included on iOS 26+excludeViewTypeFromSubtreeTraversalandincludeViewTypeInSubtreeTraversalProblematicLayer/ProblematicViewtest classes that tracksublayersaccesssublayersaccess) while non-ignored types traverse normally📝 Checklist
You have to check all boxes before merging:
sendDefaultPIIis enabled.