search

getsentry / sentry-native

Sentry SDK for C, C++ and native applications.
MIT License
404 stars 170 forks source link

feat: programatic minidump capture #1052

Closed PlasmaDev5 closed 3 weeks ago

PlasmaDev5 commented 1 month ago

Resolves: https://github.com/getsentry/sentry-native/issues/1050

Summary

Provides a new function that allows sentry to capture user created minidumps.

github-actions[bot] commented 1 month ago
Messages
:book: Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by :no_entry_sign: dangerJS against 67bd3af80234b611247cd8b8febbc806288530f7

PlasmaDev5 commented 1 month ago

Thanks for the change. looks straightforard so far.

Is this PR done and ready for review? because it's marked as such, I'll asume so (if not, you ca use draft PRs in the future so that reviewers don't get notifications yet).

CI is currently failing so that will need to be fixed. Also, the newly added API should have a new test case.

i guess somewhere between draft and final. I for sure not indented to be be merged yet but at the same time i need feedback and review before moving further.

PlasmaDev5 commented 1 month ago

Feel i addressed feedback provided. Unit test and mentioned example.c still need doing tomorrow. but wanted to get a little more feedback

codecov[bot] commented 1 month ago

Codecov Report

Attention: Patch coverage is 9.52381% with 38 lines in your changes missing coverage. Please review.

Project coverage is 81.25%. Comparing base (21c6d0a) to head (8233951). Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1052 +/- ## ========================================== - Coverage 81.81% 81.25% -0.57% ========================================== Files 53 53 Lines 6363 6402 +39 Branches 1207 1214 +7 ========================================== - Hits 5206 5202 -4 - Misses 1045 1086 +41 - Partials 112 114 +2 ```
PlasmaDev5 commented 1 month ago

Addressed review feedback. Note: feedback to split declaration and initialization of envelope creates a lint error.

I also have work towards the test case and will post a draft version in a comment on here once i start work tomorrow for feedback as i am a lot more unsure with how we want to handle this step.

PlasmaDev5 commented 1 month ago

To give a rundown on what i have planned for the tests

I am referencing the attachment tests. It essentially comes down to using sentry__path_write_buffer, from what i can tell it should be able to create a .dmp file and just add some unused data. I can then use the new API to send capture this file. Im not sure if there is a better solution as im still getting to grips with the available API.

PlasmaDev5 commented 1 month ago

@supervacuus Any feedback on the test case stuff? and hopefully all review feedback is addressed.

bruno-garcia commented 3 weeks ago

Closing in favor of: