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: Handle no releaseName in WatchdogLogic #3919

Merged
merged 2 commits into from
Apr 30, 2024

Conversation

philipphofmann
Copy link
Member

@philipphofmann philipphofmann commented Apr 29, 2024

📜 Description

The release name passed to the app state from the options is nullable. The SentryAppState including the SentryWatchdogTerminationLogic couldn't handle this edge case correctly, which is fixed now.

💡 Motivation and Context

Unit tests started to fail suddenly on the main branch https://github.com/getsentry/sentry-cocoa/actions/runs/8879401702/job/24377229574.

💚 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

The release name passed to the app state from the options is nullable.
The SentryAppState including the SentryWatchDogTerminationLogic
couldn't handle this edge case correctly, which is fixed now.
@philipphofmann philipphofmann changed the title fix: Handle no releaseName in WatchDogLogic fix: Handle no releaseName in WatchdogLogic Apr 29, 2024
Copy link

codecov bot commented Apr 29, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.694%. Comparing base (aa45f36) to head (4305a9a).

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #3919       +/-   ##
=============================================
+ Coverage   90.690%   90.694%   +0.003%     
=============================================
  Files          581       582        +1     
  Lines        45418     45520      +102     
  Branches     16162     16221       +59     
=============================================
+ Hits         41190     41284       +94     
- Misses        4048      4166      +118     
+ Partials       180        70      -110     
Files Coverage Δ
Sources/Sentry/SentryAppState.m 98.648% <100.000%> (+0.018%) ⬆️
Sources/Sentry/SentryWatchdogTerminationLogic.m 78.378% <100.000%> (+6.156%) ⬆️
Tests/SentryTests/Helper/SentryAppStateTests.swift 100.000% <100.000%> (ø)
...s/SentryWatchdogTerminationsIntegrationTests.swift 96.923% <100.000%> (-0.047%) ⬇️
...tions/SentryWatchdogTerminationsTrackerTests.swift 100.000% <100.000%> (+1.310%) ⬆️

... and 44 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 aa45f36...4305a9a. Read the comment docs.

Copy link

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1212.77 ms 1237.29 ms 24.52 ms
Size 21.58 KiB 616.79 KiB 595.21 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
408f43e 1210.44 ms 1242.35 ms 31.91 ms
07b120c 1227.48 ms 1235.14 ms 7.66 ms
d9308fd 1228.04 ms 1243.35 ms 15.31 ms
60dd0f5 1238.98 ms 1254.48 ms 15.50 ms
51307b7 1223.08 ms 1240.76 ms 17.68 ms
ee3f02e 1236.53 ms 1263.54 ms 27.01 ms
eaa1002 1205.50 ms 1235.08 ms 29.58 ms
b9a9dca 1196.09 ms 1225.14 ms 29.05 ms
a6f8b18 1238.54 ms 1265.56 ms 27.02 ms
28c80b5 1198.63 ms 1214.08 ms 15.45 ms

App size

Revision Plain With Sentry Diff
408f43e 21.58 KiB 573.17 KiB 551.59 KiB
07b120c 21.58 KiB 614.90 KiB 593.32 KiB
d9308fd 21.58 KiB 612.83 KiB 591.25 KiB
60dd0f5 20.76 KiB 393.37 KiB 372.61 KiB
51307b7 22.85 KiB 407.63 KiB 384.78 KiB
ee3f02e 22.84 KiB 401.67 KiB 378.83 KiB
eaa1002 20.76 KiB 423.19 KiB 402.43 KiB
b9a9dca 21.58 KiB 414.59 KiB 393.01 KiB
a6f8b18 20.76 KiB 431.87 KiB 411.11 KiB
28c80b5 21.58 KiB 417.85 KiB 396.27 KiB

@philipphofmann philipphofmann merged commit f96d41a into main Apr 30, 2024
69 of 70 checks passed
@philipphofmann philipphofmann deleted the fix/release-name-nil-watchdog branch April 30, 2024 11:59
philipphofmann added a commit that referenced this pull request May 6, 2024
The release name passed to the app state from the options is nullable.
The SentryAppState including the SentryWatchDogTerminationLogic
couldn't handle this edge case correctly, which is fixed now.
dKasabwala pushed a commit to dKasabwala/sentry-cocoa that referenced this pull request May 6, 2024
The release name passed to the app state from the options is nullable.
The SentryAppState including the SentryWatchDogTerminationLogic
couldn't handle this edge case correctly, which is fixed now.
threema-matteo pushed a commit to threema-ch/sentry-cocoa that referenced this pull request May 21, 2024
The release name passed to the app state from the options is nullable.
The SentryAppState including the SentryWatchDogTerminationLogic
couldn't handle this edge case correctly, which is fixed now.
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