Skip to content

Conversation

noahsmartin
Copy link
Contributor

@noahsmartin noahsmartin commented Jul 1, 2025

Fixes a crash when SentryFileManager was null. The initializer for SentryFileManager is declared to return a nullable instance, so SentryScopeContextPersistanceStore should be the same. The crash looked like this:

Screenshot 2025-07-01 at 2 14 19 PM

From what I can tell this started crashing since #5242 so hasn't been in production for too long

#skip-changelog

Copy link

codecov bot commented Jul 1, 2025

Codecov Report

Attention: Patch coverage is 85.71429% with 1 line in your changes missing coverage. Please review.

Project coverage is 86.279%. Comparing base (65f8d2e) to head (88d50a6).
Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
Sources/Sentry/SentrySessionReplayIntegration.m 66.666% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #5535       +/-   ##
=============================================
+ Coverage   86.207%   86.279%   +0.071%     
=============================================
  Files          407       407               
  Lines        35034     35042        +8     
  Branches     15018     15218      +200     
=============================================
+ Hits         30202     30234       +32     
+ Misses        4791      4764       -27     
- Partials        41        44        +3     
Files with missing lines Coverage Δ
SentryTestUtils/ClearTestState.swift 86.153% <100.000%> (ø)
Sources/Sentry/SentryDependencyContainer.m 88.372% <ø> (ø)
...ersistence/SentryScopeContextPersistentStore.swift 97.590% <100.000%> (+0.122%) ⬆️
Sources/Sentry/SentrySessionReplayIntegration.m 89.325% <66.666%> (-0.129%) ⬇️

... and 16 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 65f8d2e...88d50a6. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@armcknight armcknight left a comment

Choose a reason for hiding this comment

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

Looks good, just looks like you need to update some test swift that now can't compile.

Since this modifies a file in HybridPublic I don't know if this is a breaking change or not, will defer to @philipphofmann there.

@noahsmartin noahsmartin force-pushed the fixFileManagerCrash branch from c10ea7b to 88d50a6 Compare July 1, 2025 20:39
Copy link
Contributor

github-actions bot commented Jul 1, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1226.85 ms 1255.98 ms 29.12 ms
Size 23.74 KiB 872.75 KiB 849.00 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
db9572a 1212.61 ms 1237.73 ms 25.13 ms
a2a3bfb 1227.94 ms 1261.26 ms 33.32 ms
9389467 1218.62 ms 1244.86 ms 26.24 ms
fdea6f5 1216.08 ms 1241.82 ms 25.73 ms
db9572a 1223.13 ms 1241.60 ms 18.47 ms
701b301 1226.10 ms 1245.57 ms 19.47 ms
35c962f 1207.61 ms 1235.90 ms 28.29 ms
55f739c 1226.06 ms 1248.78 ms 22.71 ms
2609f7a 1218.17 ms 1241.34 ms 23.17 ms
f97a070 1218.88 ms 1253.12 ms 34.24 ms

App size

Revision Plain With Sentry Diff
db9572a 23.75 KiB 858.65 KiB 834.90 KiB
a2a3bfb 23.75 KiB 872.67 KiB 848.92 KiB
9389467 23.75 KiB 866.51 KiB 842.76 KiB
fdea6f5 23.75 KiB 867.15 KiB 843.40 KiB
db9572a 23.75 KiB 858.64 KiB 834.89 KiB
701b301 23.75 KiB 867.16 KiB 843.41 KiB
35c962f 23.75 KiB 854.77 KiB 831.02 KiB
55f739c 23.75 KiB 858.73 KiB 834.98 KiB
2609f7a 23.75 KiB 867.04 KiB 843.29 KiB
f97a070 23.75 KiB 858.68 KiB 834.93 KiB

@noahsmartin noahsmartin merged commit aa0b738 into main Jul 1, 2025
135 of 137 checks passed
@noahsmartin noahsmartin deleted the fixFileManagerCrash branch July 1, 2025 21:07
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.

3 participants