feat: Support modifying attachments after init #433
Conversation
Moves the attachments to the scope, and adds `sentry_add_attachment` and `sentry_remove_attachment` and wstr variants that modify this attachment list after calling init. Attachments are identified by their path.
There was a problem hiding this comment.
Thanks @Swatinem 💯. The API seems sufficient for synching attachments from Java to native.
|
I'm sorry I was wrong. I think we need some more changes to this PR.
In the SDK API Evolution meeting, we decided it is fine to just copy the byte array to native with the tradeoff of increasing the memory footprint. |
| * | ||
| * See the NOTE on attachments above for restrictions of this API. | ||
| */ | ||
| SENTRY_API void sentry_add_attachment(const char *path); |
There was a problem hiding this comment.
Will we always want this to be void? Does a status return make any sense now or in the future?
| * | ||
| * See the NOTE on attachments above for restrictions of this API. | ||
| */ | ||
| SENTRY_API void sentry_remove_attachment(const char *path); |
There was a problem hiding this comment.
While an idempotent API is good, would a return from which you can tell whether anything happened or not be useful?
| * API Users on windows are encouraged to use `sentry_add_attachmentw` instead. | ||
| * | ||
| * See the NOTE on attachments above for restrictions of this API. | ||
| */ |
There was a problem hiding this comment.
Does it make sense to explicitly say something about the ownership of *path? (same for remove attachment)
| sentry__envelope_serialize_into_stringbuilder(envelope, &sb); | ||
|
|
||
| *size_out = sentry__stringbuilder_len(&sb); | ||
| if (size_out) { |
|
This issue has gone three weeks without activity. In another week, I will close it. But! If you comment or otherwise update it, I will reset the clock, and if you remove the label "A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #433 +/- ##
==========================================
+ Coverage 87.61% 87.65% +0.04%
==========================================
Files 49 50 +1
Lines 4222 4260 +38
==========================================
+ Hits 3699 3734 +35
- Misses 523 526 +3 🚀 New features to boost your workflow:
|
|
great to see this finally land 🎉 , thanks for pushing this over the finish line. |
Moves the attachments to the scope, and adds
sentry_add_attachmentandsentry_remove_attachmentand wstr variants that modify this attachmentlist after calling init. Attachments are identified by their path.
Attachments are still based on file paths and loaded lazily, this does not add support for attachments based on in-memory buffers.