Skip to content

Conversation

noahsmartin
Copy link
Contributor

@noahsmartin noahsmartin commented Sep 17, 2025

Converts one more deserialization function to Swift, this one was a bit long but pretty straightforward and very thoroughly unit tested

#skip-changelog

Closes #6190

Copy link

codecov bot commented Sep 17, 2025

Codecov Report

❌ Patch coverage is 97.29730% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.718%. Comparing base (fccc4e5) to head (e147e18).
⚠️ Report is 7 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...ources/Swift/Helper/SentrySerializationSwift.swift 97.740% 4 Missing ⚠️
Sources/Sentry/PrivateSentrySDKOnly.m 0.000% 1 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #6186       +/-   ##
=============================================
+ Coverage   86.699%   86.718%   +0.019%     
=============================================
  Files          436       436               
  Lines        37089     37090        +1     
  Branches     17399     17363       -36     
=============================================
+ Hits         32156     32164        +8     
+ Misses        4889      4653      -236     
- Partials        44       273      +229     
Files with missing lines Coverage Δ
Sources/Sentry/SentryAttachment.m 98.214% <ø> (ø)
Sources/Sentry/SentryFileManager.m 91.931% <100.000%> (+0.264%) ⬆️
Sources/Sentry/SentryHttpTransport.m 98.245% <100.000%> (ø)
Sources/Sentry/SentryMigrateSessionInit.m 88.659% <100.000%> (ø)
Sources/Sentry/SentrySerialization.m 100.000% <ø> (+1.345%) ⬆️
Sources/Sentry/PrivateSentrySDKOnly.m 23.786% <0.000%> (ø)
...ources/Swift/Helper/SentrySerializationSwift.swift 97.740% <97.740%> (ø)

... and 41 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 fccc4e5...e147e18. Read the comment docs.

Copy link
Contributor

github-actions bot commented Sep 17, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1207.48 ms 1235.27 ms 27.79 ms
Size 23.75 KiB 977.30 KiB 953.56 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
7c7ac56 1225.90 ms 1250.22 ms 24.33 ms
8ea5293 1242.70 ms 1262.25 ms 19.55 ms
35c962f 1207.61 ms 1235.90 ms 28.29 ms
aa98fe8 1227.69 ms 1253.18 ms 25.50 ms
339539a 1219.58 ms 1254.63 ms 35.05 ms
0ac4c65 1236.85 ms 1267.84 ms 30.98 ms
6ee4973 1228.42 ms 1252.26 ms 23.84 ms
b13e93a 1236.24 ms 1247.33 ms 11.08 ms
2e79f5f 1220.53 ms 1249.35 ms 28.82 ms
5ae9ff1 1222.31 ms 1250.96 ms 28.65 ms

App size

Revision Plain With Sentry Diff
7c7ac56 23.75 KiB 902.49 KiB 878.74 KiB
8ea5293 23.75 KiB 852.24 KiB 828.49 KiB
35c962f 23.75 KiB 854.77 KiB 831.02 KiB
aa98fe8 23.75 KiB 891.01 KiB 867.26 KiB
339539a 23.75 KiB 968.24 KiB 944.50 KiB
0ac4c65 23.75 KiB 968.24 KiB 944.50 KiB
6ee4973 23.75 KiB 896.53 KiB 872.79 KiB
b13e93a 23.75 KiB 855.37 KiB 831.62 KiB
2e79f5f 23.75 KiB 928.87 KiB 905.12 KiB
5ae9ff1 23.74 KiB 971.82 KiB 948.08 KiB

Previous results on branch: envelopeDeserializeSwift

Startup times

Revision Plain With Sentry Diff
7710a76 1228.43 ms 1244.53 ms 16.10 ms
4f59363 1222.23 ms 1250.30 ms 28.07 ms
b27584f 1213.15 ms 1225.34 ms 12.19 ms
dd0291a 1200.69 ms 1235.64 ms 34.94 ms

App size

Revision Plain With Sentry Diff
7710a76 23.75 KiB 976.94 KiB 953.19 KiB
4f59363 23.75 KiB 977.33 KiB 953.58 KiB
b27584f 23.75 KiB 977.31 KiB 953.57 KiB
dd0291a 23.75 KiB 977.30 KiB 953.56 KiB

@noahsmartin noahsmartin force-pushed the envelopeDeserializeSwift branch 3 times, most recently from 956cca3 to 62e2308 Compare September 17, 2025 18:59
@noahsmartin noahsmartin force-pushed the envelopeDeserializeSwift branch from 62e2308 to dce6537 Compare September 17, 2025 20:02
@noahsmartin noahsmartin marked this pull request as ready for review September 18, 2025 02:30
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Woah, this code is complex 🤯 . Thanks for converting it to Swift. It looks way better now.

Copy link
Contributor

github-actions bot commented Sep 18, 2025

🚨 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:

  • Sources/Sentry/SentryFileManager.m
  • Sources/Sentry/SentrySerialization.m

@noahsmartin noahsmartin merged commit a4c5ddc into main Sep 18, 2025
179 checks passed
@noahsmartin noahsmartin deleted the envelopeDeserializeSwift branch September 18, 2025 14:30
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.

ref: Envelope deserialization to Swift
2 participants