feat: add Hints API for beforeSend callbacks - #8942
Conversation
|
itaybre
added
the
run-full-ci
📲 Install BuildsiOS
|
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6844d69 | 1228.25 ms | 1265.20 ms | 36.95 ms |
| d929b8b | 1227.85 ms | 1265.15 ms | 37.30 ms |
| 89c6f79 | 1215.43 ms | 1256.32 ms | 40.89 ms |
| fc4e913 | 1221.61 ms | 1253.82 ms | 32.21 ms |
| 487d993 | 1223.12 ms | 1261.37 ms | 38.25 ms |
| 50fe369 | 1226.48 ms | 1268.96 ms | 42.48 ms |
| 0f52723 | 1239.90 ms | 1271.96 ms | 32.06 ms |
| bf10fe6 | 1226.00 ms | 1260.76 ms | 34.76 ms |
| 445b440 | 1226.80 ms | 1257.72 ms | 30.93 ms |
| c955ea9 | 1210.87 ms | 1245.78 ms | 34.91 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| 6844d69 | 24.14 KiB | 1.20 MiB | 1.17 MiB |
| d929b8b | 24.14 KiB | 1.18 MiB | 1.16 MiB |
| 89c6f79 | 24.14 KiB | 1.29 MiB | 1.27 MiB |
| fc4e913 | 24.14 KiB | 1.17 MiB | 1.15 MiB |
| 487d993 | 24.14 KiB | 1.24 MiB | 1.21 MiB |
| 50fe369 | 24.14 KiB | 1.24 MiB | 1.22 MiB |
| 0f52723 | 24.14 KiB | 1.24 MiB | 1.21 MiB |
| bf10fe6 | 24.14 KiB | 1.17 MiB | 1.15 MiB |
| 445b440 | 24.14 KiB | 1.17 MiB | 1.14 MiB |
| c955ea9 | 24.14 KiB | 1.25 MiB | 1.23 MiB |
Add a Hint class that provides metadata about the origin of an event (original error/exception, attachments, key-value data) flowing alongside events through the capture pipeline. New beforeSendWithHint and beforeBreadcrumbWithHint callbacks take precedence over their existing counterparts. The WithHint variants are deprecated and will be removed in v10 when the hint parameter is added to beforeSend/beforeBreadcrumb directly.
6ce73f0 to
dd59e88
Compare
philprime
left a comment
philprime
left a comment
There was a problem hiding this comment.
Left some early review comments to consider
| } | ||
| } | ||
|
|
||
| @objc public var beforeSendWithHint: ((SentryObjCEvent, SentryObjCHint) -> SentryObjCEvent?)? { |
There was a problem hiding this comment.
m: we should consider marking this as deprecated already and mention that this will be the default in v10 as proposed by @NinjaLikesCheez
There was a problem hiding this comment.
Adding it to the public var brought a lot of issues but TIL I can add the deprecated only to the setter.
Updated now
|
|
||
| class SentryHintTests: XCTestCase { | ||
|
|
||
| func testInit_shouldHaveNilProperties() { |
There was a problem hiding this comment.
m: Should add some concurrency tests here to verify the properties are behind locks/mutex
Guard Hint state with SentryMutex instead of NSLock, and deprecate the WithHint callback setters via @available and DEPRECATED_MSG_ATTRIBUTE, since the hint parameter moves into beforeSend/beforeBreadcrumb in the next major version. Pre-populate hint.attachments with the scope attachments before beforeSendWithHint runs and treat the hint list as authoritative afterwards, so the callback can remove attachments as well as add them. Add concurrency tests for Hint and attachment add/remove coverage for the client.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6a2b13b. Configure here.
| alwaysAttachStacktrace:alwaysAttachStacktrace | ||
| isFatalEvent:isFatalEvent | ||
| hint:hint]; | ||
| } |
There was a problem hiding this comment.
Empty hints skip attachment population
Medium Severity
The no-hint prepareEvent overloads allocate a fresh empty Hint and invoke beforeSendWithHint without calling populateHintAttachments. Callers such as captureReplayEvent and saveCrashTransaction therefore present an empty attachment list, and any add or remove the callback performs is discarded because those paths never read that hint afterward.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 6a2b13b. Configure here.
2 participants
Add a Hints API that gives
beforeSendandbeforeBreadcrumbcallbacks access to the raw source material of an event: the originalNSErrororNSExceptionthat triggered the capture, the attachments that will be sent with the event, and generic key-value storage. The newHintclass (SentryHintin ObjC) is threaded through the entire capture pipeline inSentryClientandSentryHub, and is exposed through the newbeforeSendWithHintandbeforeBreadcrumbWithHintcallbacks, which take precedence overbeforeSend/beforeBreadcrumbwhen both are set. The ObjC wrapper surface gets a matchingSentryObjCHinttype and callback properties.Attachments can be added and removed
Before
beforeSendWithHintruns, the SDK pre-populateshint.attachmentswith the attachments that will be sent with the event (the scope's attachments, filtered for fatal events). The list left in the hint when the callback returns is what the SDK sends, so the callback can add and remove attachments, as proposed in #1460.The WithHint callbacks ship deprecated
They are transitional API: in v10 the hint parameter will be added to
beforeSend/beforeBreadcrumbdirectly and theWithHintvariants will be removed. The deprecation is on the setters only (via@availableandDEPRECATED_MSG_ATTRIBUTE), so adopters get the warning while the SDK's internal reads stay clean.Hintguards its state withSentryMutex, so it is safe to read and write from any queue.Refs #1460