Skip to content

Fix invalid log file being attached to crash events on Mac/iOS #873

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+136 −54
Open

Fix invalid log file being attached to crash events on Mac/iOS #873

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
cb1209c
Replace log file path getter with utility method
tustanivsky Jan 15, 2025
3040265
Merge branch 'main' into fix/log-attachment
tustanivsky Apr 16, 2025
015b187
Update log file attachment for crashes
tustanivsky Apr 17, 2025
665e974
Fix redundant log file attached to crash events
tustanivsky Apr 17, 2025
3e72b60
Update changelog
tustanivsky Apr 17, 2025
211e10d
Add workaround for issue with invalid log file path on iOS
tustanivsky Apr 22, 2025
28a8a93
Fix compilation errors on iOS
tustanivsky Apr 23, 2025
5ace245
Merge branch 'main' into fix/log-attachment
tustanivsky Apr 25, 2025
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
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
### Fixes

- Fix warnings caused by deprecated Cocoa SDK API usages ([#868](https://github.com/getsentry/sentry-unreal/pull/868))
- Fix invalid log file being attached to crash events on Mac/iOS ([#873](https://github.com/getsentry/sentry-unreal/pull/873))
Copy link
Member

Choose a reason for hiding this comment

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

The section on the changelog is already "Fixes" so we could (going forward, from the next release perhaps) avoid starting the message with Fix.

Personally I'd be more interested in which platform the fix relates to. So this is a iOS/Mac, could be:

  • iOS/macOS: Correct log file now attached to crashes. Since crashes are sent on restart, the SDK now looks at the previous run's log files to attach. Leaving the current run's log file untouched

Or something like that.

- Fix Sentry cURL transport can't send envelopes on Linux ([#882](https://github.com/getsentry/sentry-unreal/pull/882))
- The SDK now ensures the execute permission is set for Sentry CLI and symbol upload script when they have been downloaded via the Editor ([#881](https://github.com/getsentry/sentry-unreal/pull/881))

Expand Down
76 changes: 48 additions & 28 deletions plugin-dev/Source/Sentry/Private/Apple/AppleSentrySubsystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
#include "Convenience/SentryInclude.h"
#include "Convenience/SentryMacro.h"

#include "Utils/SentryFileUtils.h"
#include "Utils/SentryLogUtils.h"

#include "GenericPlatform/GenericPlatformOutputDevices.h"
#include "HAL/FileManager.h"
#include "HAL/PlatformSentryAttachment.h"
Expand All @@ -34,10 +37,12 @@
#include "Misc/Paths.h"
#include "UObject/GarbageCollection.h"
#include "UObject/UObjectThreadContext.h"
#include "Utils/SentryLogUtils.h"

void FAppleSentrySubsystem::InitWithSettings(const USentrySettings* settings, USentryBeforeSendHandler* beforeSendHandler, USentryBeforeBreadcrumbHandler* beforeBreadcrumbHandler, USentryTraceSampler* traceSampler)
{
isScreenshotAttachmentEnabled = settings->AttachScreenshot;
isGameLogAttachmentEnabled = settings->EnableAutoLogAttachment;
Comment on lines +43 to +44
Copy link
Contributor

@bitsandfoxes bitsandfoxes Apr 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Should these go on the actual options? We're starting to mix settings. with options. here. I'd really prefer the options to be the source of truth of:

  1. What options exist?
  2. What options does the SDK use.

This will also make it easier for users to browse and explore and for us to reason through the SDK's behaviour.

Copy link
Collaborator Author

@tustanivsky tustanivsky Apr 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

These two flags are cached separately here since they don't necessarily have a direct representation in Cocoa SDK options:

  • isGameLogAttachmentEnabled is Unreal-specific feature
  • isScreenshotAttachmentEnabled can't be just set via options.attachScreenshot as it's an iOS-only thing in sentry-cocoa

Accessing them using FSentryModule::Get().GetSettings() could cause unwanted ensures when called from a non-game thread.

Copy link
Member

Choose a reason for hiding this comment

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

attachScreenshot

This option (with this name) also exists on Android, .NET, Godot, Unity.

Copy link
Collaborator Author

@tustanivsky tustanivsky Apr 25, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I might be missing the concern here. settings properties are passed to the Cocoa SDK’s options within the startWithConfigureOptions block right after we make copies of these flags here (mainly for convenience as we use them in a few other places beyond initialization). Or is this about the naming?


[SENTRY_APPLE_CLASS(PrivateSentrySDKOnly) setSdkName:@"sentry.cocoa.unreal"];

dispatch_group_t sentryDispatchGroup = dispatch_group_create();
Expand Down Expand Up @@ -65,14 +70,6 @@ void FAppleSentrySubsystem::InitWithSettings(const USentrySettings* settings, US
#if SENTRY_UIKIT_AVAILABLE
options.attachScreenshot = settings->AttachScreenshot;
#endif
options.initialScope = ^(SentryScope* scope) {
if(settings->EnableAutoLogAttachment) {
const FString logFilePath = IFileManager::Get().ConvertToAbsolutePathForExternalAppForRead(*FGenericPlatformOutputDevices::GetAbsoluteLogFilename());
SentryAttachment* logAttachment = [[SENTRY_APPLE_CLASS(SentryAttachment) alloc] initWithPath:logFilePath.GetNSString()];
[scope addAttachment:logAttachment];
}
return scope;
};
options.onCrashedLastRun = ^(SentryEvent* event) {
if (settings->AttachScreenshot)
{
Expand All @@ -81,7 +78,16 @@ void FAppleSentrySubsystem::InitWithSettings(const USentrySettings* settings, US
const FString& screenshotPath = GetLatestScreenshot();
if (!screenshotPath.IsEmpty())
{
UploadScreenshotForEvent(MakeShareable(new SentryIdApple(event.eventId)), screenshotPath);
UploadAttachmentForEvent(MakeShareable(new SentryIdApple(event.eventId)), screenshotPath, TEXT("screenshot.png"), true);
}
}
if(settings->EnableAutoLogAttachment) {
// Unreal creates game log backups automatically on every app run. If logging is enabled for current configuration SDK can
// find the most recent one and upload it to Sentry.
const FString logFilePath = GetLatestGameLog();
if (!logFilePath.IsEmpty())
{
UploadAttachmentForEvent(MakeShareable(new SentryIdApple(event.eventId)), logFilePath, SentryFileUtils::GetGameLogName());
}
}
};
Expand Down Expand Up @@ -206,17 +212,19 @@ void FAppleSentrySubsystem::ClearBreadcrumbs()

TSharedPtr<ISentryId> FAppleSentrySubsystem::CaptureMessage(const FString& message, ESentryLevel level)
{
SentryId* id = [SENTRY_APPLE_CLASS(SentrySDK) captureMessage:message.GetNSString() withScopeBlock:^(SentryScope* scope){
[scope setLevel:SentryConvertersApple::SentryLevelToNative(level)];
}];

return MakeShareable(new SentryIdApple(id));
FSentryScopeDelegate onConfigureScope;
return CaptureMessageWithScope(message, onConfigureScope, level);
}

TSharedPtr<ISentryId> FAppleSentrySubsystem::CaptureMessageWithScope(const FString& message, const FSentryScopeDelegate& onConfigureScope, ESentryLevel level)
{
SentryId* id = [SENTRY_APPLE_CLASS(SentrySDK) captureMessage:message.GetNSString() withScopeBlock:^(SentryScope* scope){
[scope setLevel:SentryConvertersApple::SentryLevelToNative(level)];
if(isGameLogAttachmentEnabled) {
const FString logFilePath = GetGameLogPath();
SentryAttachment* logAttachment = [[SENTRY_APPLE_CLASS(SentryAttachment) alloc] initWithPath:logFilePath.GetNSString()];
[scope addAttachment:logAttachment];
}
onConfigureScope.ExecuteIfBound(MakeShareable(new SentryScopeApple(scope)));
}];

Expand All @@ -225,17 +233,20 @@ TSharedPtr<ISentryId> FAppleSentrySubsystem::CaptureMessageWithScope(const FStri

TSharedPtr<ISentryId> FAppleSentrySubsystem::CaptureEvent(TSharedPtr<ISentryEvent> event)
{
TSharedPtr<SentryEventApple> eventIOS = StaticCastSharedPtr<SentryEventApple>(event);

SentryId* id = [SENTRY_APPLE_CLASS(SentrySDK) captureEvent:eventIOS->GetNativeObject()];
return MakeShareable(new SentryIdApple(id));
FSentryScopeDelegate onConfigureScope;
return CaptureEventWithScope(event, onConfigureScope);
}

TSharedPtr<ISentryId> FAppleSentrySubsystem::CaptureEventWithScope(TSharedPtr<ISentryEvent> event, const FSentryScopeDelegate& onConfigureScope)
{
TSharedPtr<SentryEventApple> eventIOS = StaticCastSharedPtr<SentryEventApple>(event);

SentryId* id = [SENTRY_APPLE_CLASS(SentrySDK) captureEvent:eventIOS->GetNativeObject() withScopeBlock:^(SentryScope* scope) {
if(isGameLogAttachmentEnabled) {
const FString logFilePath = GetGameLogPath();
SentryAttachment* logAttachment = [[SENTRY_APPLE_CLASS(SentryAttachment) alloc] initWithPath:logFilePath.GetNSString()];
Copy link
Member

Choose a reason for hiding this comment

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

I'm trying to diff the 3 versions of this snippet in this PR but they all seem the same?

If we're changing things would you want them all to be consistent?
I think it's better to throw this into a function, add some debug logging in there (central point) for example

[scope addAttachment:logAttachment];
}
onConfigureScope.ExecuteIfBound(MakeShareable(new SentryScopeApple(scope)));
}];

Expand All @@ -251,7 +262,14 @@ TSharedPtr<ISentryId> FAppleSentrySubsystem::CaptureEnsure(const FString& type,
SentryEvent *exceptionEvent = [[SENTRY_APPLE_CLASS(SentryEvent) alloc] init];
exceptionEvent.exceptions = nativeExceptionArray;

SentryId* id = [SENTRY_APPLE_CLASS(SentrySDK) captureEvent:exceptionEvent];
SentryId* id = [SENTRY_APPLE_CLASS(SentrySDK) captureEvent:exceptionEvent withScopeBlock:^(SentryScope* scope) {
if(isGameLogAttachmentEnabled) {
const FString logFilePath = GetGameLogPath();
SentryAttachment* logAttachment = [[SENTRY_APPLE_CLASS(SentryAttachment) alloc] initWithPath:logFilePath.GetNSString()];
[scope addAttachment:logAttachment];
}
}];

return MakeShareable(new SentryIdApple(id));
}

Expand Down Expand Up @@ -389,18 +407,18 @@ TSharedPtr<ISentryTransactionContext> FAppleSentrySubsystem::ContinueTrace(const
return MakeShareable(new SentryTransactionContextApple(transactionContext));
}

void FAppleSentrySubsystem::UploadScreenshotForEvent(TSharedPtr<ISentryId> eventId, const FString& screenshotPath) const
void FAppleSentrySubsystem::UploadAttachmentForEvent(TSharedPtr<ISentryId> eventId, const FString& filePath, const FString& name, bool deleteAfterUpload) const
{
IFileManager& fileManager = IFileManager::Get();
if (!fileManager.FileExists(*screenshotPath))
if (!fileManager.FileExists(*filePath))
{
UE_LOG(LogSentrySdk, Error, TEXT("Failed to upload screenshot - path provided did not exist: %s"), *screenshotPath);
UE_LOG(LogSentrySdk, Error, TEXT("Failed to upload attachment - file path provided did not exist: %s"), *filePath);
return;
}

const FString& screenshotFilePathExt = fileManager.ConvertToAbsolutePathForExternalAppForRead(*screenshotPath);
const FString& filePathExt = fileManager.ConvertToAbsolutePathForExternalAppForRead(*filePath);

SentryAttachment* screenshotAttachment = [[SENTRY_APPLE_CLASS(SentryAttachment) alloc] initWithPath:screenshotFilePathExt.GetNSString() filename:@"screenshot.png"];
SentryAttachment* screenshotAttachment = [[SENTRY_APPLE_CLASS(SentryAttachment) alloc] initWithPath:filePathExt.GetNSString() filename:name.GetNSString()];

SentryOptions* options = [SENTRY_APPLE_CLASS(PrivateSentrySDKOnly) options];
int32 size = options.maxAttachmentSize;
Expand All @@ -413,10 +431,12 @@ void FAppleSentrySubsystem::UploadScreenshotForEvent(TSharedPtr<ISentryId> event

[SENTRY_APPLE_CLASS(PrivateSentrySDKOnly) captureEnvelope:envelope];

// After uploading screenshot it's no longer needed so delete
if (!fileManager.Delete(*screenshotPath))
if (deleteAfterUpload)
{
UE_LOG(LogSentrySdk, Error, TEXT("Failed to delete screenshot: %s"), *screenshotPath);
if (!fileManager.Delete(*filePath))
{
UE_LOG(LogSentrySdk, Error, TEXT("Failed to delete file attachment after upload: %s"), *filePath);
}
}
}

Expand Down
8 changes: 7 additions & 1 deletion plugin-dev/Source/Sentry/Private/Apple/AppleSentrySubsystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,14 @@ class FAppleSentrySubsystem : public ISentrySubsystem
virtual FString TryCaptureScreenshot() const { return FString(); };

protected:
void UploadScreenshotForEvent(TSharedPtr<ISentryId> eventId, const FString& screenshotPath) const;
void UploadAttachmentForEvent(TSharedPtr<ISentryId> eventId, const FString& filePath, const FString& name, bool deleteAfterUpload = false) const;

virtual FString GetScreenshotPath() const;
virtual FString GetLatestScreenshot() const;
virtual FString GetGameLogPath() const { return FString(); };
virtual FString GetLatestGameLog() const { return FString(); };

protected:
bool isScreenshotAttachmentEnabled = false;
bool isGameLogAttachmentEnabled = false;
};
33 changes: 32 additions & 1 deletion plugin-dev/Source/Sentry/Private/IOS/IOSSentrySubsystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,13 @@
#include "SentryDefines.h"
#include "SentrySettings.h"

#include "Utils/SentryScreenshotUtils.h"
#include "Utils/SentryFileUtils.h"

#include "HAL/FileManager.h"
#include "Misc/CoreDelegates.h"
#include "Misc/FileHelper.h"
#include "Misc/Paths.h"
#include "Utils/SentryScreenshotUtils.h"

static FIOSSentrySubsystem* GIOSSentrySubsystem = nullptr;

Expand Down Expand Up @@ -103,3 +106,31 @@ FString FIOSSentrySubsystem::TryCaptureScreenshot() const

return ScreenshotPath;
}

FString FIOSSentrySubsystem::GetGameLogPath() const
{
const FString& logFilePath = SentryFileUtils::GetGameLogPath();
return IFileManager::Get().FileExists(*logFilePath) ? logFilePath : NormalizeToPublicIOSPath(logFilePath);
}

FString FIOSSentrySubsystem::GetLatestGameLog() const
{
const FString logFilePath = SentryFileUtils::GetGameLogBackupPath();
return IFileManager::Get().FileExists(*logFilePath) ? logFilePath : NormalizeToPublicIOSPath(logFilePath);
}

FString FIOSSentrySubsystem::NormalizeToPublicIOSPath(const FString& logFilePath) const
{
// This is a workaround for iOS log file not being accessible via the path returned by engine's API.
// See https://github.com/getsentry/sentry-unreal/pull/732

static FString PublicWritePathBase = FString([NSSearchPathForDirectoriesInDomains(NSDocumentDirectory, NSUserDomainMask, YES) objectAtIndex:0]);
static FString PrivateWritePathBase = FString([NSSearchPathForDirectoriesInDomains(NSLibraryDirectory, NSUserDomainMask, YES) objectAtIndex:0]);

if (logFilePath.StartsWith(PrivateWritePathBase))
{
return logFilePath.Replace(*PrivateWritePathBase, *PublicWritePathBase);
}

return logFilePath;
}
43 changes: 25 additions & 18 deletions plugin-dev/Source/Sentry/Private/IOS/IOSSentrySubsystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,25 @@
#pragma once

#include "Apple/AppleSentrySubsystem.h"

class FIOSSentrySubsystem : public FAppleSentrySubsystem
{
public:
virtual void InitWithSettings(
const USentrySettings* Settings,
USentryBeforeSendHandler* BeforeSendHandler,
USentryBeforeBreadcrumbHandler* BeforeBreadcrumbHandler,
USentryTraceSampler* TraceSampler
) override;

virtual FString TryCaptureScreenshot() const override;
};

typedef FIOSSentrySubsystem FPlatformSentrySubsystem;
#pragma once

#include "Apple/AppleSentrySubsystem.h"

class FIOSSentrySubsystem : public FAppleSentrySubsystem
{
public:
virtual void InitWithSettings(
const USentrySettings* Settings,
USentryBeforeSendHandler* BeforeSendHandler,
USentryBeforeBreadcrumbHandler* BeforeBreadcrumbHandler,
USentryTraceSampler* TraceSampler
) override;

virtual FString TryCaptureScreenshot() const override;

protected:
virtual FString GetGameLogPath() const override;
virtual FString GetLatestGameLog() const override;

private:
FString NormalizeToPublicIOSPath(const FString& logFilePath) const;
};

typedef FIOSSentrySubsystem FPlatformSentrySubsystem;
18 changes: 14 additions & 4 deletions plugin-dev/Source/Sentry/Private/Mac/MacSentrySubsystem.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include "SentryModule.h"
#include "SentrySettings.h"

#include "Utils/SentryFileUtils.h"

#include "Misc/CoreDelegates.h"
#include "Misc/FileHelper.h"
#include "Misc/Paths.h"
Expand All @@ -15,9 +17,7 @@ void FMacSentrySubsystem::InitWithSettings(const USentrySettings* Settings, USen
{
FAppleSentrySubsystem::InitWithSettings(Settings, BeforeSendHandler, BeforeBreadcrumbHandler, TraceSampler);

isScreenshotAttachmentEnabled = Settings->AttachScreenshot;

if (IsEnabled() && Settings->AttachScreenshot)
if (IsEnabled() && isScreenshotAttachmentEnabled)
{
FCoreDelegates::OnHandleSystemError.AddLambda([this]()
{
Expand All @@ -35,7 +35,7 @@ TSharedPtr<ISentryId> FMacSentrySubsystem::CaptureEnsure(const FString& type, co
const FString& screenshotPath = TryCaptureScreenshot();
if (!screenshotPath.IsEmpty())
{
UploadScreenshotForEvent(id, screenshotPath);
UploadAttachmentForEvent(id, screenshotPath, TEXT("screenshot.png"), true);
}
}

Expand Down Expand Up @@ -81,3 +81,13 @@ FString FMacSentrySubsystem::TryCaptureScreenshot() const

return ScreenshotPath;
}

FString FMacSentrySubsystem::GetGameLogPath() const
{
return SentryFileUtils::GetGameLogPath();
}

FString FMacSentrySubsystem::GetLatestGameLog() const
{
return SentryFileUtils::GetGameLogBackupPath();
}
5 changes: 3 additions & 2 deletions plugin-dev/Source/Sentry/Private/Mac/MacSentrySubsystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ class FMacSentrySubsystem : public FAppleSentrySubsystem

virtual FString TryCaptureScreenshot() const override;

private:
bool isScreenshotAttachmentEnabled = false;
protected:
virtual FString GetGameLogPath() const override;
virtual FString GetLatestGameLog() const override;
};

typedef FMacSentrySubsystem FPlatformSentrySubsystem;
5 changes: 5 additions & 0 deletions plugin-dev/Source/Sentry/Private/Utils/SentryFileUtils.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ struct FSentrySortFileByDatePredicate
}
};

FString SentryFileUtils::GetGameLogName()
{
return FPaths::GetCleanFilename(FGenericPlatformOutputDevices::GetAbsoluteLogFilename());
}

FString SentryFileUtils::GetGameLogPath()
{
return IFileManager::Get().ConvertToAbsolutePathForExternalAppForRead(*FGenericPlatformOutputDevices::GetAbsoluteLogFilename());
Expand Down
1 change: 1 addition & 0 deletions plugin-dev/Source/Sentry/Private/Utils/SentryFileUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
class SentryFileUtils
{
public:
static FString GetGameLogName();
static FString GetGameLogPath();
static FString GetGameLogBackupPath();
static FString GetGpuDumpPath();
Expand Down