Add callback to pre-process breadcrumbs before adding #814
Conversation
|
| { | ||
| if (!FTaskTagScope::IsCurrentTag(ETaskTag::EGameThread)) | ||
| { | ||
| // Executing `onBeforeBreadcrumb` handler is not allowed when called from Android main thread. |
There was a problem hiding this comment.
running on the main thread will be quite common. Why is it not allowed?
There was a problem hiding this comment.
This causes issues when wrapping Android jobject instances in Unreal since USentryBreadcrumb and USentryHint (both derived from UObject) can only be safely created on the GameThread.
Despite certain breadcrumb types (i.e. Android activity and app lifecycle callbacks) originating from the main thread and lacking pre-processing support they will still be included in captured events.
There was a problem hiding this comment.
what's the behavior a user can expect? some crumbs will go through the callback and some won't?
it just seems like a surprising behavior.
"why can't I drop these breadcrumbs through beforeBreadcrumb?"
Should we avoid adding a callback altogether then?
Or how do we avoid surprises/disapointments?
There was a problem hiding this comment.
@bruno-garcia The original error message I've seen while testing this led me to believe that calling the beforeBreadcrumb hook outside the game thread on Android was the issue but it turned out to be misleading. Similar to beforeSend (#860) the real limitation is that we can't call it during GC otherwise it works as expected.
I've added this limitation to the README and will also ensure it's mentioned in the documentation as @bitsandfoxes suggested.
# Conflicts: # CHANGELOG.md # plugin-dev/Source/Sentry/Private/GenericPlatform/GenericPlatformSentrySubsystem.cpp # scripts/packaging/package-github.snapshot # scripts/packaging/package-marketplace.snapshot
This PR introduces the following changes to Unreal SDK docs: - Added `beforeBreadcrumb` hook description (getsentry/sentry-unreal#814) - Added [notice](getsentry/sentry-unreal#860 (review)) that hooks can't be invoked during garbage collection in Unreal - Added fast-fail crash capturing to the feature comparison table for different plugin versions (getsentry/sentry-unreal#828) - Added [notice](getsentry/sentry-unreal#812 (comment)) about `EngineVersion` key in the plugin's descriptor file for GitHub package - Added GPU crash dump attachment option (getsentry/sentry-unreal#712)
This PR introduces the following changes to Unreal SDK docs: - Added `beforeBreadcrumb` hook description (getsentry/sentry-unreal#814) - Added [notice](getsentry/sentry-unreal#860 (review)) that hooks can't be invoked during garbage collection in Unreal - Added fast-fail crash capturing to the feature comparison table for different plugin versions (getsentry/sentry-unreal#828) - Added [notice](getsentry/sentry-unreal#812 (comment)) about `EngineVersion` key in the plugin's descriptor file for GitHub package - Added GPU crash dump attachment option (getsentry/sentry-unreal#712)
This PR adds
beforeBreadcrumbhook allowing to modify the breadcrumb or decide to discard it entirely by returningnullptrbefore it's added.The suggested changes are similar to #318.
beforeBreadcrumbhook won't be called on Android if the underlying SDK creates breadcrumb in a main thread (i.e. during Activity lifecycle callback handling).Closes #807
Related to getsentry/sentry-native#1166