feat(native): update before_send docs #13984
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
2 Skipped Deployments
|
Bundle ReportChanges will decrease total bundle size by 237 bytes (-0.0%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: sentry-docs-client-array-pushAssets Changed:
view changes for bundle: sentry-docs-server-cjsAssets Changed:
|
| const char *custom_error = (const char *)hint; | ||
| const char *custom_error = (const char *)user_data; |
There was a problem hiding this comment.
I think this is still confusing, because it characterizes user_data as something that should contribute to error resolution, which is absolutely not what it is used for. user_data is about passing in context from the user, so they can call into their code or retrieve relevant data.
hint and user_data are entirely unrelated. If user_data gets assigned to a hook variable called custom_error, that weakens the clear distinction we should accomplish in the docs.
| The Native SDK does not pass exception information into the hint, but instead allows to set a custom hint in SDK options. To detect the exception type, inspect the event instead. | ||
| The Native SDK does not pass exception information into the hint, but instead allows to set custom `user_data` in the `before_send` SDK option. To detect the exception type, inspect the event instead. |
There was a problem hiding this comment.
We should note here that the Native SDK's solution to the problem of obtaining additional crash context is the on_crash hook, and that the usage of the before_send hook for handling crashes is deprecated and provided only as a compatibility fallback.
This is particularly confusing because the entire first section of this document clearly addresses this particular issue. I guess that this snippet has only "recently" been added as an included section below as part of a larger refactoring, because otherwise this confusing and contradictory explanation would have been evident in documenting the much later added on_crash hook.
| crash_cleanup( | ||
| const sentry_ucontext_t *uctx, // provides the user-space context of the crash | ||
| sentry_value_t event, // used the same way as in `before_send` | ||
| void *closure // user-data that you can provide at configuration time | ||
| void *user_data // user-data that you can provide at configuration time |
There was a problem hiding this comment.
I'm not quite sure what kind of cleanup one could do with the data one gets from the uctx and event; @supervacuus do you have any inspiration? I was thinking about using fields from uctx->siginfo to discard specific types of crashes, but maybe you know of something nicer.
There was a problem hiding this comment.
Clean-up is the wrong concept for on_crash to convey to users. This is precisely where before_send and on_crash dramatically diverge:
- the event we get in
on_crashis essentially empty for all minidump generating backends inprocis the only backend that passes complete event data to either hook- so, if you aren't using
inproc, theeventparameter is not a proper clean-up or filter discriminator - the event can, up to a certain point, be enriched inside the
on_crashhook as the last client interaction before the crashed application terminates - in the same vein, it can be used to act at all before the crashed application terminates
- while the
ucontextcan provide a rich source of metadata, which can find its way back into the event or as a filter mechanism, the interactions would have to be very specific to a particular use case, and describing those might lead to more confusion than insight - it is enormously rare, for any minidump generating backend to remove any events, solely by looking at the
ucontext, since the biggest source of meta-data is the snapshot collection for the minidump which is currently not accessible in the client (and if we would make it accessible we probably wouldn't go indirectly via minidump)
Having said all that, a concrete example from the field was from a user who reported C++ exceptions via a std::terminate handler that would capture a manual exception event and didn't want the resulting crash of an unhandled C++ exception to generate an additional error report. To prevent sending C++ exceptions on Windows, I provided an on_crash approach for this particular scenario: getsentry/sentry-native#860 (comment). However, I question whether this approach falls into the "nice" category.
| /* modify event here or return NULL to discard the event */ | ||
| (void)hint; |
There was a problem hiding this comment.
I think this would be a sensible location to specify that hint is currently not expected to contain any data, and maybe to repeat that users who want more crash context should use the on_crash hook.
| crash_cleanup( | ||
| const sentry_ucontext_t *uctx, // provides the user-space context of the crash | ||
| sentry_value_t event, // used the same way as in `before_send` | ||
| void *closure // user-data that you can provide at configuration time | ||
| void *user_data // user-data that you can provide at configuration time |
There was a problem hiding this comment.
Clean-up is the wrong concept for on_crash to convey to users. This is precisely where before_send and on_crash dramatically diverge:
- the event we get in
on_crashis essentially empty for all minidump generating backends inprocis the only backend that passes complete event data to either hook- so, if you aren't using
inproc, theeventparameter is not a proper clean-up or filter discriminator - the event can, up to a certain point, be enriched inside the
on_crashhook as the last client interaction before the crashed application terminates - in the same vein, it can be used to act at all before the crashed application terminates
- while the
ucontextcan provide a rich source of metadata, which can find its way back into the event or as a filter mechanism, the interactions would have to be very specific to a particular use case, and describing those might lead to more confusion than insight - it is enormously rare, for any minidump generating backend to remove any events, solely by looking at the
ucontext, since the biggest source of meta-data is the snapshot collection for the minidump which is currently not accessible in the client (and if we would make it accessible we probably wouldn't go indirectly via minidump)
Having said all that, a concrete example from the field was from a user who reported C++ exceptions via a std::terminate handler that would capture a manual exception event and didn't want the resulting crash of an unhandled C++ exception to generate an additional error report. To prevent sending C++ exceptions on Windows, I provided an on_crash approach for this particular scenario: getsentry/sentry-native#860 (comment). However, I question whether this approach falls into the "nice" category.
Latest Vercel Preview
DESCRIBE YOUR PR
Fixes getsentry/sentry-native#1268
We fix some ancient docs which wrongly state that
before_senduses thehintparameter in the Native SDK, by cleaning up the unnecessary 'Event Hints' section.We move the example snippet into the corresponding
before_sendandon_crashsections, better showcasing how to use the passed-in values.IS YOUR CHANGE URGENT?
Help us prioritize incoming PRs by letting us know when the change needs to go live.
SLA
Thanks in advance for your help!
PRE-MERGE CHECKLIST
Make sure you've checked the following before merging your changes: