-
-
Notifications
You must be signed in to change notification settings - Fork 366
refactor: Migrate SentryMsgPackSerializer from Objective-C to Swift #6143
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
base: main
Are you sure you want to change the base?
Conversation
…nverted tests - Updated SentryMsgPackSerializer to log errors instead of debug messages for empty data and input stream issues. - Modified the `asInputStream` method in the SentryStreamable protocol to return nullable streams. - Removed outdated Objective-C tests and added comprehensive Swift tests for SentryMsgPackSerializer, covering various scenarios including nil input streams and invalid file paths. - Ensured proper cleanup of temporary files in tests.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #6143 +/- ##
========================================
Coverage ? 86.777%
========================================
Files ? 442
Lines ? 37263
Branches ? 17425
========================================
Hits ? 32336
Misses ? 4883
Partials ? 44
Continue to review full report in Codecov by Sentry.
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
be9107c | 1223.63 ms | 1242.82 ms | 19.19 ms |
b193842 | 1231.36 ms | 1247.69 ms | 16.33 ms |
884b224 | 1221.11 ms | 1255.88 ms | 34.77 ms |
3279d4e | 1215.76 ms | 1256.45 ms | 40.69 ms |
ccf1278 | 1226.84 ms | 1248.51 ms | 21.67 ms |
f2bfecd | 1234.92 ms | 1250.34 ms | 15.42 ms |
52603c5 | 1229.35 ms | 1251.82 ms | 22.47 ms |
8fd192f | 1202.10 ms | 1220.19 ms | 18.09 ms |
d06a4db | 1233.20 ms | 1243.18 ms | 9.98 ms |
f43a482 | 1219.00 ms | 1245.56 ms | 26.56 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
be9107c | 23.75 KiB | 975.19 KiB | 951.44 KiB |
b193842 | 23.75 KiB | 920.65 KiB | 896.90 KiB |
884b224 | 23.75 KiB | 879.55 KiB | 855.80 KiB |
3279d4e | 23.75 KiB | 938.32 KiB | 914.57 KiB |
ccf1278 | 23.75 KiB | 877.15 KiB | 853.40 KiB |
f2bfecd | 23.75 KiB | 919.68 KiB | 895.93 KiB |
52603c5 | 23.75 KiB | 969.66 KiB | 945.92 KiB |
8fd192f | 23.74 KiB | 872.75 KiB | 849.01 KiB |
d06a4db | 23.75 KiB | 913.18 KiB | 889.43 KiB |
f43a482 | 23.75 KiB | 928.16 KiB | 904.41 KiB |
Previous results on branch: philprime/msg-pack-serializer-null-handling
Startup times
Revision | Plain | With Sentry | Diff |
---|---|---|---|
43dc3b5 | 1236.69 ms | 1255.65 ms | 18.95 ms |
8215a0d | 1206.23 ms | 1237.04 ms | 30.81 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
43dc3b5 | 23.75 KiB | 988.55 KiB | 964.80 KiB |
8215a0d | 23.75 KiB | 969.21 KiB | 945.46 KiB |
…rializer-null-handling
…nal serialization tests - Added support for error streams in TestStreamableObject. - Introduced new test cases for serializing empty dictionaries, single elements, large dictionaries, long keys, and handling invalid paths. - Implemented a custom ErrorInputStream to simulate read errors during serialization.
…ntation - Deleted the Objective-C SentryMsgPackSerializer and its associated header files. - Introduced a new Swift implementation of SentryMsgPackSerializer with improved error handling. - Added SentryStreamable protocol and extensions for Data, NSData, NSURL, and URL to support serialization. - Updated tests to validate the new Swift serialization logic and error handling.
…r error propagation - Changed keyData.withUnsafeBytes to use try for improved error handling. - This ensures that any errors during buffer address retrieval are properly thrown.
@cursor review |
- Introduced TestStreamableObject to simulate various SentryStreamable behaviors, including handling nil and error streams. - Updated SentryMsgPackSerializerTests to utilize TestStreamableObject for improved test coverage on serialization scenarios. - Removed redundant TestStreamableObject implementation from SentryMsgPackSerializerTests to streamline code.
…rializer-null-handling
Summary
This PR migrates the SentryMsgPackSerializer from Objective-C to Swift while maintaining 100% behavioral compatibility.
Changes Made
Complete Migration
SentryMsgPackSerializer.m/.h
to modular Swift implementationSources/Swift/Tools/MsgPack/
directory:SentryMsgPackSerializer.swift
- Main serialization logicSentryStreamable.swift
- Protocol definitionSentryMsgPackSerializerError.swift
- Error typesData+SentryStreamable.swift
- Data extensionURL+SentryStreamable.swift
- URL extensionNSData+SentryStreamable.swift
- NSData extensionNSURL+SentryStreamable.swift
- NSURL extensionBehavioral Compatibility
Enhanced Test Coverage
Implementation Improvements
SentryMsgPackSerializerError
Key Technical Details
streamSize() -> Int
to support -1 error values from Objective-CUInt8(truncatingIfNeeded:)
to match Objective-C silent truncationData.write(to:)
methodTesting
All existing functionality verified through comprehensive test suite:
Closes #6140
#skip-changelog