-
-
Notifications
You must be signed in to change notification settings - Fork 366
fix(session-replay): Add exemption for CameraUI traversal for iOS 26.0 #6045
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
Conversation
Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift
Outdated
Show resolved
Hide resolved
Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift
Outdated
Show resolved
Hide resolved
❌ 9 Tests Failed:
View the top 3 failed test(s) by shortest run time
To view more test analytics, go to the Test Analytics Dashboard |
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
9389467 | 1218.62 ms | 1244.86 ms | 26.24 ms |
5d238d3 | 1228.94 ms | 1253.04 ms | 24.10 ms |
bc0a04c | 1226.83 ms | 1255.04 ms | 28.21 ms |
67e8e3e | 1220.08 ms | 1229.23 ms | 9.15 ms |
3ec47ae | 1231.02 ms | 1256.67 ms | 25.65 ms |
f0e2579 | 1224.82 ms | 1245.49 ms | 20.67 ms |
8047b99 | 1226.37 ms | 1246.63 ms | 20.26 ms |
701b301 | 1226.10 ms | 1245.57 ms | 19.47 ms |
07d7e83 | 1211.71 ms | 1240.08 ms | 28.37 ms |
570f725 | 1206.00 ms | 1238.96 ms | 32.96 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
9389467 | 23.75 KiB | 866.51 KiB | 842.76 KiB |
5d238d3 | 23.75 KiB | 913.62 KiB | 889.88 KiB |
bc0a04c | 23.75 KiB | 933.32 KiB | 909.57 KiB |
67e8e3e | 23.75 KiB | 919.91 KiB | 896.16 KiB |
3ec47ae | 23.75 KiB | 919.88 KiB | 896.13 KiB |
f0e2579 | 23.75 KiB | 969.22 KiB | 945.47 KiB |
8047b99 | 23.75 KiB | 855.37 KiB | 831.62 KiB |
701b301 | 23.75 KiB | 867.16 KiB | 843.41 KiB |
07d7e83 | 23.75 KiB | 913.27 KiB | 889.52 KiB |
570f725 | 23.74 KiB | 913.38 KiB | 889.63 KiB |
Previous results on branch: issue-5647
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d1b6626 | 1242.37 ms | 1262.79 ms | 20.42 ms |
384aab3 | 1232.67 ms | 1260.67 ms | 28.00 ms |
697c280 | 1206.27 ms | 1231.76 ms | 25.50 ms |
f8f9ada | 1214.33 ms | 1249.85 ms | 35.52 ms |
9793ab3 | 1231.33 ms | 1248.90 ms | 17.57 ms |
29dc424 | 1233.29 ms | 1257.29 ms | 24.01 ms |
afcf7f2 | 1200.98 ms | 1234.06 ms | 33.08 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
d1b6626 | 23.75 KiB | 969.24 KiB | 945.49 KiB |
384aab3 | 23.75 KiB | 963.05 KiB | 939.30 KiB |
697c280 | 23.75 KiB | 969.24 KiB | 945.49 KiB |
f8f9ada | 23.75 KiB | 938.51 KiB | 914.76 KiB |
9793ab3 | 23.75 KiB | 963.10 KiB | 939.35 KiB |
29dc424 | 23.75 KiB | 969.23 KiB | 945.48 KiB |
afcf7f2 | 23.75 KiB | 933.31 KiB | 909.56 KiB |
Using |
@cursor review |
The mapRedactRegion method could create duplicate redaction regions for views that met both general redaction criteria and isViewSubtreeIgnored condition. This happened when clipsToBounds was false, allowing both code paths to add regions for the same view. Fixed by moving the isViewSubtreeIgnored check to the beginning with an early return, ensuring views are processed only once. Added comprehensive tests to verify deduplication works correctly while maintaining proper redaction behavior."
@philipphofmann we can wait with this PR until after #6075 so we have verification in CI that the tests work as expected. |
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.
Thanks a lot for tackling this complex crash. I have a few questions.
Sources/Swift/Core/Tools/ViewCapture/SentryUIRedactBuilder.swift
Outdated
Show resolved
Hide resolved
@philipphofmann is the new UI test failing on your machine too? It passes on my machine but fails in CI. |
It runs successfully on my machine. I tested it with Xcode 26 RC and iPhone 17 Pro iOS 26 simulator. |
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.
LGTM
📜 Description
This pull request introduces a targeted workaround in the
SentryUIRedactBuilder
to handle a crash related toCameraUI.ChromeSwiftUIView
on iOS 26 when built with Xcode 16. The workaround ensures that the redaction process skips subtrees of this specific internal camera view to prevent the crash, while maintaining normal redaction behavior for other views.SentryUIRedactBuilder
to detect and skip subtrees ofCameraUI.ChromeSwiftUIView
when running on iOS 26+ to prevent crashes from unimplemented initializers. (cameraSwiftUIViewClassObjectId
,isViewSubtreeIgnored
,redactRegionsFor(view:)
) [1] [2] [3]view.debugDescription
instead oflayer.name
for more consistent identification. [1] [2]ExtrasViewController.swift
to present the camera UI usingUIImagePickerController
for testing camera permission flows when pressing the buttonShow Camera UI
.💡 Motivation and Context
Fixes #5647
💚 How did you test it?
📝 Checklist
You have to check all boxes before merging:
sendDefaultPII
is enabled.Full Investigation
The issue could be reproduced when using Xcode 16.4 to run on iOS 26.0 Beta 6 or later, where accessing the
layer.subLayers
during the redaction geometry calculations while displaying the standard Camera UI caused a crash:During my investigation I found the class to be part of the private framework
CameraUI
, located at/Library/Developer/CoreSimulator/Volumes/iOS_23A5326a/Library/Developer/CoreSimulator/Profiles/Runtimes/iOS 26.0.simruntime/Contents/Resources/RuntimeRoot/System/Library/PrivateFrameworks/CameraUI.framework
when running on iOS 26.0 Beta 6.My approach to resolve the issue is excluding the view hierarchy of the camera UI from traversal and instead fully redact the UI, as it can be considered personal information anyways. I found the
CameraUI.ModeLoupeLayer
to be part of the view hierarchy of theCameraUI.ChromeSwiftUIView
, so I use it to decide if the subview hierarchy should be ignored from traversal.While working on the unit tests to verify the changes redaction algorithm, I encountered issues in creating an instance of the
CameraUI.ChromeSwiftUIView
as it is a private framework which can not be imported directly into the unit tests.Instead I tried to find the class using
NSClassFromString("CameraUI.ChromeSwiftUIView")
which also returnednil
because the framework was not yet loaded. I was able to resolve this by creating an instance ofUIImagePickerController
(the original cause of the crash) which then transitively imports the frameworkCameraUI
.To analyse the class, I created the following dump tool to access the available Objective-C runtime information:
This resulted in the following output:
This shows that the
initWithCoder
andinitWithFrame
exist as expected, but as shown in the crash message might not be implemented.My initial approach to get an instance of the view was using the instance of the
UIImagePickerController
and its subviews for the redaction testing. Unfortunately the image picker controller seems to be empty at the beginning and loads additional views after being displayed - most likely also due to permissions checking.❌ Instance of UIImagePickerController
My second attempt was using the
NSClassFromString
to create the reference of the class type to then call it's init method with or without a frame:I concluded that it’s a Swift class that likely has a Swift-only designated initializer (internal/private, not @objc) and the ObjC inits (-init, -initWithFrame:, -initWithCoder:) are intentionally trapped to fatalError.
❌ Invoking init methods of
CameraUI.ChromeSwiftUIView
via Objective-C runtimeMy next approach was circumventing the traps by swizzling the init methods of the
ChromeSwiftUIView
with the init methods of it's superclassUIView
:✅ Swizzling the type initializer
As the solution above is replacing class type methods which can have side effects, I also explored the option of raw object initialization using the Objective-C runtime
✅ Swizzling the instance initializer
This implementation still relies on Objective-C, but I wanted to check if I can directly call the private Swift initializer too. I started to look into the LLVM symbol table dumper of the
CameraUI
framework to see if I can find anyTo summarize this output, the available symbols only include the
_TtC8CameraUI17ChromeSwiftUIView
demangles toCameraUI.ChromeSwiftUIView
(Swift class bridged to ObjC) and class and metaclass exist in the ObjC runtime (soNSClassFromString
works), but symbol lookup by name ((lldb) image lookup -rn ChromeSwiftUIView
) finds nothing because the relevant symbols aren’t exported.❌ This means we can not
dlsym
a Swift initializer to invoke it directly and instance swizzling is the best approach.