Skip to content

Fix calling beforeSend handler during post-loading #589

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Jul 16, 2024
Merged

Fix calling beforeSend handler during post-loading #589

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ade498a
Set thread-safe user policy for error delegate
tustanivsky Jul 3, 2024
aa26aca
Rename delegate
tustanivsky Jul 3, 2024
ad7e9fe
Update changelog
tustanivsky Jul 3, 2024
ca96548
Add post-loading check before calling beforeSend handler on desktop
tustanivsky Jul 8, 2024
764a25d
Set thread-safe user policy for error delegate
tustanivsky Jul 8, 2024
32c65de
Update changelog
tustanivsky Jul 8, 2024
3c43e54
Merge branch 'main' into fix/assert-del-threading
tustanivsky Jul 10, 2024
dedf572
Fix build error after merge
tustanivsky Jul 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

### Breaking Changes

- Rename public `OnError` delegate field in `FSentryOutputDeviceError` class to `OnAssert`

### Features

- On Windows/Linux the SDK can now automatically attach a screenshot to crash events([#582](https://github.com/getsentry/sentry-unreal/pull/582))
Expand All @@ -10,6 +14,7 @@
### Fixes

- The SDK no longer intercepts assertions when using crash-reporter ([#586](https://github.com/getsentry/sentry-unreal/pull/586))
- Fix calling `beforeSend` handler during post-loading ([#589](https://github.com/getsentry/sentry-unreal/pull/589))
- Fix crash when re-initializing Sentry ([#594](https://github.com/getsentry/sentry-unreal/pull/594))

### Dependencies
Expand Down
23 changes: 17 additions & 6 deletions plugin-dev/Source/Sentry/Private/Desktop/SentrySubsystemDesktop.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "GenericPlatform/GenericPlatformOutputDevices.h"
#include "GenericPlatform/GenericPlatformCrashContext.h"
#include "UObject/GarbageCollection.h"
#include "UObject/UObjectThreadContext.h"

#if PLATFORM_WINDOWS
#include "Windows/WindowsPlatformMisc.h"
Expand Down Expand Up @@ -81,9 +82,14 @@ sentry_value_t HandleBeforeSend(sentry_value_t event, void *hint, void *closure)
USentryEvent* EventToProcess = NewObject<USentryEvent>();
EventToProcess->InitWithNativeImpl(eventDesktop);

return SentrySubsystem->GetBeforeSendHandler()->HandleBeforeSend(EventToProcess, nullptr)
? event
: sentry_value_new_null();
USentryEvent* ProcessedEvent = EventToProcess;
if(!FUObjectThreadContext::Get().IsRoutingPostLoad)
{
// Executing UFUNCTION is allowed only when not post-loading
ProcessedEvent = SentrySubsystem->GetBeforeSendHandler()->HandleBeforeSend(EventToProcess, nullptr);
}

return ProcessedEvent ? event : sentry_value_new_null();
}

sentry_value_t HandleBeforeCrash(const sentry_ucontext_t *uctx, sentry_value_t event, void *closure)
Expand All @@ -102,9 +108,14 @@ sentry_value_t HandleBeforeCrash(const sentry_ucontext_t *uctx, sentry_value_t e
USentryEvent* EventToProcess = NewObject<USentryEvent>();
EventToProcess->InitWithNativeImpl(eventDesktop);

return SentrySubsystem->GetBeforeSendHandler()->HandleBeforeSend(EventToProcess, nullptr)
? event
: sentry_value_new_null();
USentryEvent* ProcessedEvent = EventToProcess;
if(!FUObjectThreadContext::Get().IsRoutingPostLoad)
{
// Executing UFUNCTION is allowed only when not post-loading
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide some context what this means?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Post-loading is a part of an actor lifecycle that goes right after finishing loading it from disk (i.e. when a game map opened in editor). Typically at this point some related actor's versioning or fixup behavior should be called. It's unsafe to use any blueprint-implemented functions (like HandleBeforeSend in our case) during this stage so we've added an extra check here to avoid triggering a corresponding assertion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation!

ProcessedEvent = SentrySubsystem->GetBeforeSendHandler()->HandleBeforeSend(EventToProcess, nullptr);
}

return ProcessedEvent ? event : sentry_value_new_null();
}
else
{
Expand Down
2 changes: 1 addition & 1 deletion plugin-dev/Source/Sentry/Private/SentryOutputDeviceError.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ void FSentryOutputDeviceError::Serialize(const TCHAR* V, ELogVerbosity::Type Ver
{
if(FDebug::HasAsserted())
{
OnError.Broadcast(V);
OnAssert.Broadcast(V);
}

if (!ParentDevice)
Expand Down
5 changes: 3 additions & 2 deletions plugin-dev/Source/Sentry/Private/SentrySubsystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ void USentrySubsystem::Close()
{
if(OnAssertDelegate.IsValid())
{
OutputDeviceError->OnError.Remove(OnAssertDelegate);
OutputDeviceError->OnAssert.Remove(OnAssertDelegate);
OnAssertDelegate.Reset();
}

Expand Down Expand Up @@ -665,7 +665,8 @@ void USentrySubsystem::ConfigureOutputDeviceError()
OutputDeviceError = MakeShareable(new FSentryOutputDeviceError(GError));
if (OutputDeviceError)
{
OnAssertDelegate = OutputDeviceError->OnError.AddLambda([this](const FString& Message)
OnAssertDelegate = OutputDeviceError->OnAssert.AddLambda([this](const FString& Message)

{
SubsystemNativeImpl->CaptureAssertion(TEXT("Assertion failed"), Message);

Expand Down
2 changes: 1 addition & 1 deletion plugin-dev/Source/Sentry/Public/SentryOutputDeviceError.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class FSentryOutputDeviceError : public FOutputDeviceError

FOutputDeviceError* GetParentDevice();

TMulticastDelegate<void(const FString&)> OnError;
TMulticastDelegate<void(const FString&), FDefaultTSDelegateUserPolicy> OnAssert;

private:
FOutputDeviceError* ParentDevice;
Expand Down