Skip to content
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

fix: Stop SessionReplay when closing SDK #3941

Merged
merged 1 commit into from
May 6, 2024
Merged

Conversation

philipphofmann
Copy link
Member

📜 Description

Call SessionReplay.stop when uninstalling the SDK and invalidate the DisplayLinkWrapper when SessionReplay gets deallocated.

💡 Motivation and Context

This came up when investigating a failing test on the main branch: https://github.com/getsentry/sentry-cocoa/actions/runs/8969460754/job/24630955316?pr=3940. The logs show SessionReplay running, although it shouldn't. We also have a problem with accessing UIWindow, but we'll fix this in another PR.

Test Case '-[SentryTests.SentryViewHierarchyTests test_appViewHierarchy_usesMainThread]' started.
2024-05-06 13:01:44.899951+0000 xctest[4660:15933] [Sentry] [debug] [SentryThreadWrapper:26] Already on main thread.
[SentryOnDemandReplay] Could not save replay frame. Error: Error Domain=NSCocoaErrorDomain Code=4 "The file “0.png” doesn’t exist." UserInfo={NSFilePath=/Users/runner/Library/Developer/CoreSimulator/Devices/9854D8B3-535A-430C-9E2C-91A75EB44494/data/Library/Caches/io.sentry/replay/B560D7AF-3250-41B7-A86B-321C1DE4BB2F/0.png, NSUnderlyingError=0x600004da9260 {Error Domain=NSPOSIXErrorDomain Code=2 "No such file or directory"}}
=================================================================
Main Thread Checker: UI API called on a background thread: -[UIWindow screen]
PID: 4660, TID: 20723, Thread name: (none), Queue name: com.apple.root.default-qos, QoS: 0
Backtrace:
4   Sentry                              0x000000010bf0da76 -[SentryCrashWrapper enrichScope:] + 2006
5   Sentry                              0x000000010bf4148a -[SentryHub initWithClient:andScope:] + 1210
6   Sentry                              0x000000010bf97227 +[SentrySDK currentHub] + 135
7   Sentry                              0x000000010bf8c18b -[SentrySessionReplay captureSegment:video:replayId:replayType:] + 683
8   Sentry                              0x000000010bf8be52 __59-[SentrySessionReplay createAndCapture:duration:startedAt:]_block_invoke + 466
9   Sentry                              0x000000010c0129d4 $s6Sentry0A9VideoInfoCSgSo7NSErrorCSgIeyByy_ADs5Error_pSgIeggg_TR + 116
10  Sentry                              0x000000010c012593 $s6Sentry0A14OnDemandReplayC15createVideoWith8duration9beginning13outputFileURL10completionySd_10Foundation4DateVAI0L0VyAA0aF4InfoCSg_s5Error_pSgtctKFyycfU_yycfU_ + 2419
11  Sentry                              0x000000010c0153bc $s6Sentry0A14OnDemandReplayC15createVideoWith8duration9beginning13outputFileURL10completionySd_10Foundation4DateVAI0L0VyAA0aF4InfoCSg_s5Error_pSgtctKFyycfU_yycfU_TA + 204
12  Sentry                              0x000000010c004928 $sIeg_IeyB_TR + 40
13  Foundation                          0x00007ff8016432c1 __111+[__NSOperationInternalObserver _observeValueForKeyPath:ofObject:changeKind:oldValue:newValue:indexes:context:]_block_invoke_2 + 17
14  libdispatch.dylib                   0x00007ff8001896a1 _dispatch_call_block_and_release + 12
15  libdispatch.dylib                   0x00007ff80018a8d9 _dispatch_client_callout + 8
16  libdispatch.dylib                   0x00007ff80019d709 _dispatch_root_queue_drain + 920
17  libdispatch.dylib                   0x00007ff80019dfde _dispatch_worker_thread2 + 236
18  libsystem_pthread.dylib             0x000000010993bf8a _pthread_wqthread + 256
19  libsystem_pthread.dylib             0x000000010993af57 start_wqthread + 15
2024-05-06 13:01:44.951396+0000 xctest[4660:20723] [reports] Main Thread Checker: UI API called on a background thread: -[UIWindow screen]
PID: 4660, TID: 20723, Thread name: (none), Queue name: com.apple.root.default-qos, QoS: 0
Backtrace:
4   Sentry                              0x000000010bf0da76 -[SentryCrashWrapper enrichScope:] + 2006
5   Sentry                              0x000000010bf4148a -[SentryHub initWithClient:andScope:] + 1210
6   Sentry                              0x000000010bf97227 +[SentrySDK currentHub] + 135
7   Sentry                              0x000000010bf8c18b -[SentrySessionReplay captureSegment:video:replayId:replayType:] + 683
8   Sentry                              0x000000010bf8be52 __59-[SentrySessionReplay createAndCapture:duration:startedAt:]_block_invoke + 466
9   Sentry                              0x000000010c0129d4 $s6Sentry0A9VideoInfoCSgSo7NSErrorCSgIeyByy_ADs5Error_pSgIeggg_TR + 116
10  Sentry                              0x000000010c012593 $s6Sentry0A14OnDemandReplayC15createVideoWith8duration9beginning13outputFileURL10completionySd_10Foundation4DateVAI0L0VyAA0aF4InfoCSg_s5Error_pSgtctKFyycfU_yycfU_ + 2419
11  Sentry                              0x000000010c0153bc $s6Sentry0A14OnDemandReplayC15createVideoWith8duration9beginning13outputFileURL10completionySd_10Foundation4DateVAI0L0VyAA0aF4InfoCSg_s5Error_pSgtctKFyycfU_yycfU_TA + 204
12  Sentry                              0x000000010c004928 $sIeg_IeyB_TR + 40
13  Foundation                          0x00007ff8016432c1 __111+[__NSOperationInternalObserver _observeValueForKeyPath:ofObject:changeKind:oldValue:newValue:indexes:context:]_block_invoke_2 + 17
14  libdispatch.dylib                   0x00007ff8001896a1 _dispatch_call_block_and_release + 12
15  libdispatch.dylib                   0x00007ff80018a8d9 _dispatch_client_callout + 8
16  libdispatch.dylib                   0x00007ff80019d709 _dispatch_root_queue_drain + 920
17  libdispatch.dylib                   0x00007ff80019dfde _dispatch_worker_thread2 + 236
18  libsystem_pthread.dylib             0x000000010993bf8a _pthread_wqthread + 256
19  libsystem_pthread.dylib             0x000000010993af57 start_wqthread + 15
2024-05-06 13:01:59.832174+0000 xctest[7166:21925] [Sentry] [debug] [SentryThreadWrapper:26] Already on main thread.
2024-05-06 13:01:59.832937+0000 xctest[7166:21925] [Sentry] [debug] [SentryThreadWrapper:26] Already on main thread.
2024-05-06 13:01:59.833530+0000 xctest[7166:21925] [Sentry] [debug] [SentryThreadWrapper:26] Already on main thread.
2024-05-06 13:01:59.834178+0000 xctest[7166:21925] [Sentry] [debug] [SentryThreadWrapper:26] Already on main thread.
2024-05-06 13:01:59.834749+0000 xctest[7166:21925] [Sentry] [debug] [SentryThreadWrapper:26] Already on main thread.
2024-05-06 13:01:59.835339+0000 xctest[7166:21925] [Sentry] [debug] [SentryThreadWrapper:26] Already on main thread.

Restarting after unexpected exit, crash, or test timeout in SentryViewHierarchyTests.test_appViewHierarchy_usesMainThread(); summary will include totals from previous launches.

💚 How did you test it?

Unit tests

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

Call SessionReplay.stop when uninstalling the SDK and invalidate the
DisplayLinkWrapper when SessionReplay gets deallocated.
Copy link

codecov bot commented May 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.767%. Comparing base (c379c5e) to head (c41e72d).
Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3941       +/-   ##
=============================================
+ Coverage   90.663%   90.767%   +0.104%     
=============================================
  Files          581       586        +5     
  Lines        45434     45785      +351     
  Branches     16169     16330      +161     
=============================================
+ Hits         41192     41558      +366     
+ Misses        4172      4157       -15     
  Partials        70        70               
Files Coverage Δ
Sources/Sentry/SentrySessionReplay.m 86.624% <100.000%> (-0.131%) ⬇️
Sources/Sentry/SentrySessionReplayIntegration.m 93.442% <ø> (+1.775%) ⬆️
...tions/SessionReplay/SentrySessionReplayTests.swift 98.412% <100.000%> (+0.052%) ⬆️

... and 26 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c379c5e...c41e72d. Read the comment docs.

Copy link
Contributor

@brustolin brustolin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this!!

@brustolin brustolin merged commit 9bc6a96 into main May 6, 2024
69 of 70 checks passed
@brustolin brustolin deleted the fix/stop-session-replay branch May 6, 2024 17:58
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this pull request May 21, 2024
Call SessionReplay.stop when uninstalling the SDK and invalidate the
DisplayLinkWrapper when SessionReplay gets deallocated.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants