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

* Replace log file path getter with utility method

* Update log file attachment for crashes

* Fix redundant log file attached to crash events

* Update changelog

* Add workaround for issue with invalid log file path on iOS

* Fix compilation errors on iOS

* Refactor attaching log to events

Author: tustanivsky

CHANGELOG.md

@@ -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))
 - 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))
 

plugin-dev/Source/Sentry/Private/Apple/AppleSentrySubsystem.cpp

@@ -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"
@@ -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;
+
 	[SENTRY_APPLE_CLASS(PrivateSentrySDKOnly) setSdkName:@"sentry.cocoa.unreal"];
 
 	dispatch_group_t sentryDispatchGroup = dispatch_group_create();
@@ -65,24 +70,18 @@ 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)
 				{
 					// If a screenshot was captured during assertion/crash in the previous app run
 					// find the most recent one and upload it to Sentry.
-					const FString& screenshotPath = GetLatestScreenshot();
-					if (!screenshotPath.IsEmpty())
-					{
-						UploadScreenshotForEvent(MakeShareable(new SentryIdApple(event.eventId)), screenshotPath);
-					}
+					UploadScreenshotForEvent(MakeShareable(new SentryIdApple(event.eventId)), GetLatestScreenshot());
+				}
+				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.
+					UploadGameLogForEvent(MakeShareable(new SentryIdApple(event.eventId)), GetLatestGameLog());
 				}
 			};
 			options.beforeSend = ^SentryEvent* (SentryEvent* event) {
@@ -206,40 +205,49 @@ 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){
+	SentryId* nativeId = [SENTRY_APPLE_CLASS(SentrySDK) captureMessage:message.GetNSString() withScopeBlock:^(SentryScope* scope){
 		[scope setLevel:SentryConvertersApple::SentryLevelToNative(level)];
 		onConfigureScope.ExecuteIfBound(MakeShareable(new SentryScopeApple(scope)));
 	}];
 
-	return MakeShareable(new SentryIdApple(id));
+	TSharedPtr<ISentryId> id = MakeShareable(new SentryIdApple(nativeId));
+
+	if (isGameLogAttachmentEnabled)
+	{
+		UploadGameLogForEvent(id, GetGameLogPath());
+	}
+
+	return id;
 }
 
 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);
+	TSharedPtr<SentryEventApple> eventApple = StaticCastSharedPtr<SentryEventApple>(event);
 
-	SentryId* id = [SENTRY_APPLE_CLASS(SentrySDK) captureEvent:eventIOS->GetNativeObject() withScopeBlock:^(SentryScope* scope) {
+	SentryId* nativeId = [SENTRY_APPLE_CLASS(SentrySDK) captureEvent:eventApple->GetNativeObject() withScopeBlock:^(SentryScope* scope) {
 		onConfigureScope.ExecuteIfBound(MakeShareable(new SentryScopeApple(scope)));
 	}];
 
-	return MakeShareable(new SentryIdApple(id));
+	TSharedPtr<ISentryId> id = MakeShareable(new SentryIdApple(nativeId));
+
+	if (isGameLogAttachmentEnabled)
+	{
+		UploadGameLogForEvent(id, GetGameLogPath());
+	}
+
+	return id;
 }
 
 TSharedPtr<ISentryId> FAppleSentrySubsystem::CaptureEnsure(const FString& type, const FString& message)
@@ -251,8 +259,16 @@ 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];
-	return MakeShareable(new SentryIdApple(id));
+	SentryId* nativeId = [SENTRY_APPLE_CLASS(SentrySDK) captureEvent:exceptionEvent];
+
+	TSharedPtr<ISentryId> id = MakeShareable(new SentryIdApple(nativeId));
+
+	if (isGameLogAttachmentEnabled)
+	{
+		UploadGameLogForEvent(id, GetGameLogPath());
+	}
+
+	return id;
 }
 
 
@@ -389,37 +405,60 @@ 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* attachment = [[SENTRY_APPLE_CLASS(SentryAttachment) alloc] initWithPath:filePathExt.GetNSString() filename:name.GetNSString()];
 
 	SentryOptions* options = [SENTRY_APPLE_CLASS(PrivateSentrySDKOnly) options];
 	int32 size = options.maxAttachmentSize;
 
-	SentryEnvelopeItem* envelopeItem = [[SENTRY_APPLE_CLASS(SentryEnvelopeItem) alloc] initWithAttachment:screenshotAttachment maxAttachmentSize:size];
+	SentryEnvelopeItem* envelopeItem = [[SENTRY_APPLE_CLASS(SentryEnvelopeItem) alloc] initWithAttachment:attachment maxAttachmentSize:size];
 
 	SentryId* id = StaticCastSharedPtr<SentryIdApple>(eventId)->GetNativeObject();
 
 	SentryEnvelope* envelope = [[SENTRY_APPLE_CLASS(SentryEnvelope) alloc] initWithId:id singleItem:envelopeItem];
 
 	[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);
+		}
 	}
 }
 
+void FAppleSentrySubsystem::UploadScreenshotForEvent(TSharedPtr<ISentryId> eventId, const FString& screenshotPath) const
+{
+	if (screenshotPath.IsEmpty())
+	{
+		// Screenshot capturing is a best-effort solution so if one wasn't captured (path is empty) skip the upload
+		return;
+	}
+
+	UploadAttachmentForEvent(eventId, screenshotPath, TEXT("screenshot.png"), true);
+}
+
+void FAppleSentrySubsystem::UploadGameLogForEvent(TSharedPtr<ISentryId> eventId, const FString& logFilePath) const
+{
+#if NO_LOGGING
+	// If writing logs to a file is disabled (i.e. default behavior for Shipping builds) skip the upload
+	return;
+#endif
+
+	UploadAttachmentForEvent(eventId, logFilePath, SentryFileUtils::GetGameLogName());
+}
+
 FString FAppleSentrySubsystem::GetScreenshotPath() const
 {
 	return FPaths::Combine(FPaths::ProjectSavedDir(), TEXT("SentryScreenshots"), FString::Printf(TEXT("screenshot-%s.png"), *FDateTime::Now().ToString()));

plugin-dev/Source/Sentry/Private/Apple/AppleSentrySubsystem.h

@@ -38,8 +38,17 @@ class FAppleSentrySubsystem : public ISentrySubsystem
 	virtual FString TryCaptureScreenshot() const { return FString(); };
 
 protected:
+	void UploadAttachmentForEvent(TSharedPtr<ISentryId> eventId, const FString& filePath, const FString& name, bool deleteAfterUpload = false) const;
+
 	void UploadScreenshotForEvent(TSharedPtr<ISentryId> eventId, const FString& screenshotPath) const;
+	void UploadGameLogForEvent(TSharedPtr<ISentryId> eventId, const FString& logFilePath) 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;
 };

plugin-dev/Source/Sentry/Private/IOS/IOSSentrySubsystem.cpp

@@ -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;
 
@@ -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;
+}

plugin-dev/Source/Sentry/Private/IOS/IOSSentrySubsystem.h

@@ -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;

plugin-dev/Source/Sentry/Private/Mac/MacSentrySubsystem.cpp

@@ -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"
@@ -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]()
 		{
@@ -33,10 +33,7 @@ TSharedPtr<ISentryId> FMacSentrySubsystem::CaptureEnsure(const FString& type, co
 	if (isScreenshotAttachmentEnabled)
 	{
 		const FString& screenshotPath = TryCaptureScreenshot();
-		if (!screenshotPath.IsEmpty())
-		{
-			UploadScreenshotForEvent(id, screenshotPath);
-		}
+		UploadScreenshotForEvent(id, screenshotPath);
 	}
 
 	return id;
@@ -81,3 +78,13 @@ FString FMacSentrySubsystem::TryCaptureScreenshot() const
 
 	return ScreenshotPath;
 }
+
+FString FMacSentrySubsystem::GetGameLogPath() const
+{
+	return SentryFileUtils::GetGameLogPath();
+}
+
+FString FMacSentrySubsystem::GetLatestGameLog() const
+{
+	return SentryFileUtils::GetGameLogBackupPath();
+}

plugin-dev/Source/Sentry/Private/Mac/MacSentrySubsystem.h

@@ -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;

plugin-dev/Source/Sentry/Private/Utils/SentryFileUtils.cpp

@@ -17,6 +17,11 @@ struct FSentrySortFileByDatePredicate
 	}
 };
 
+FString SentryFileUtils::GetGameLogName()
+{
+	return FPaths::GetCleanFilename(FGenericPlatformOutputDevices::GetAbsoluteLogFilename());
+}
+
 FString SentryFileUtils::GetGameLogPath()
 {
 	return IFileManager::Get().ConvertToAbsolutePathForExternalAppForRead(*FGenericPlatformOutputDevices::GetAbsoluteLogFilename());