Fix warning about missing GPU crash dump attachment (#1022)

* Update gpu dump attachment handling

* Fix comment

* Add more crash types to demo app

* Update changelog

Author: tustanivsky

CHANGELOG.md

@@ -2,6 +2,10 @@
 
 ## Unreleased
 
+### Fixes
+
+- No more warnings about missing GPU crash dump attachment when capturing non-GPU events ([#1022](https://github.com/getsentry/sentry-unreal/pull/1022))
+
 ### Dependencies
 
 - Bump CLI from v2.50.0 to v2.50.2 ([#1020](https://github.com/getsentry/sentry-unreal/pull/1020), [#1021](https://github.com/getsentry/sentry-unreal/pull/1021))

plugin-dev/Source/Sentry/Private/GenericPlatform/GenericPlatformSentrySubsystem.cpp

@@ -177,9 +177,9 @@ sentry_value_t FGenericPlatformSentrySubsystem::OnCrash(const sentry_ucontext_t*
 		TryCaptureScreenshot();
 	}
 
-	if (GIsGPUCrashed)
+	if (GIsGPUCrashed && isGpuDumpAttachmentEnabled)
 	{
-		IFileManager::Get().Copy(*GetGpuDumpBackupPath(), *SentryFileUtils::GetGpuDumpPath());
+		TryCaptureGpuDump();
 	}
 
 	// At this point crash events are handled the same way as non-fatal ones,
@@ -238,6 +238,7 @@ FGenericPlatformSentrySubsystem::FGenericPlatformSentrySubsystem()
 	, isStackTraceEnabled(true)
 	, isPiiAttachmentEnabled(false)
 	, isScreenshotAttachmentEnabled(false)
+	, isGpuDumpAttachmentEnabled(false)
 {
 }
 
@@ -269,18 +270,14 @@ void FGenericPlatformSentrySubsystem::InitWithSettings(const USentrySettings* se
 		databaseParentPath = FPaths::ProjectUserDir();
 	}
 
-	if (settings->AttachScreenshot)
+	isScreenshotAttachmentEnabled = settings->AttachScreenshot;
+	if (isScreenshotAttachmentEnabled)
 	{
-		isScreenshotAttachmentEnabled = true;
-
 		// Clear screenshot captured during previous session if any
 		IFileManager::Get().DeleteDirectory(*FPaths::Combine(GetDatabasePath(), TEXT("screenshots")), false, true);
 	}
 
-	if (settings->AttachGpuDump)
-	{
-		ConfigureGpuDumpAttachment(options);
-	}
+	isGpuDumpAttachmentEnabled = settings->AttachGpuDump;
 
 	if (settings->UseProxy)
 	{
@@ -683,14 +680,19 @@ void FGenericPlatformSentrySubsystem::TryCaptureScreenshot()
 	AddFileAttachment(ScreenshotAttachment);
 }
 
-FString FGenericPlatformSentrySubsystem::GetGpuDumpBackupPath() const
+void FGenericPlatformSentrySubsystem::TryCaptureGpuDump()
 {
-	static const FString DateTimeString = FDateTime::Now().ToString();
+	const FString& GpuDumpPath = SentryFileUtils::GetGpuDumpPath();
+
+	if (!IFileManager::Get().FileExists(*GpuDumpPath))
+	{
+		return;
+	}
 
-	const FString GpuDumpPath = FPaths::Combine(GetDatabasePath(), TEXT("gpudumps"), *FString::Printf(TEXT("UEAftermath-%s.nv-gpudmp"), *DateTimeString));
-	const FString GpuDumpFullPath = FPaths::ConvertRelativePathToFull(GpuDumpPath);
+	TSharedPtr<ISentryAttachment> GpuDumpAttachment =
+		MakeShareable(new FGenericPlatformSentryAttachment(GpuDumpPath, FPaths::GetCleanFilename(GpuDumpPath), TEXT("application/octet-stream")));
 
-	return GpuDumpFullPath;
+	AddFileAttachment(GpuDumpAttachment);
 }
 
 FString FGenericPlatformSentrySubsystem::GetHandlerPath() const

plugin-dev/Source/Sentry/Private/GenericPlatform/GenericPlatformSentrySubsystem.h

@@ -55,15 +55,13 @@ class FGenericPlatformSentrySubsystem : public ISentrySubsystem
 	USentryBeforeBreadcrumbHandler* GetBeforeBreadcrumbHandler();
 
 	void TryCaptureScreenshot();
-
-	FString GetGpuDumpBackupPath() const;
+	void TryCaptureGpuDump();
 
 protected:
 	virtual void ConfigureHandlerPath(sentry_options_t* Options) {}
 	virtual void ConfigureDatabasePath(sentry_options_t* Options) {}
 	virtual void ConfigureCertsPath(sentry_options_t* Options) {}
 	virtual void ConfigureLogFileAttachment(sentry_options_t* Options) {}
-	virtual void ConfigureGpuDumpAttachment(sentry_options_t* Options) {}
 	virtual void ConfigureNetworkConnectFunc(sentry_options_t* Options) {}
 
 	FString GetHandlerPath() const;
@@ -100,6 +98,7 @@ class FGenericPlatformSentrySubsystem : public ISentrySubsystem
 	bool isStackTraceEnabled;
 	bool isPiiAttachmentEnabled;
 	bool isScreenshotAttachmentEnabled;
+	bool isGpuDumpAttachmentEnabled;
 
 	FString databaseParentPath;
 };

plugin-dev/Source/Sentry/Private/Linux/LinuxSentrySubsystem.cpp

@@ -60,9 +60,4 @@ void FLinuxSentrySubsystem::ConfigureLogFileAttachment(sentry_options_t* Options
 	sentry_options_add_attachment(Options, TCHAR_TO_UTF8(*FPaths::ConvertRelativePathToFull(LogFilePath)));
 }
 
-void FLinuxSentrySubsystem::ConfigureGpuDumpAttachment(sentry_options_t* Options)
-{
-	sentry_options_add_attachment(Options, TCHAR_TO_UTF8(*GetGpuDumpBackupPath()));
-}
-
 #endif // USE_SENTRY_NATIVE

plugin-dev/Source/Sentry/Private/Linux/LinuxSentrySubsystem.h

@@ -16,7 +16,6 @@ class FLinuxSentrySubsystem : public FGenericPlatformSentrySubsystem
 	virtual void ConfigureDatabasePath(sentry_options_t* Options) override;
 	virtual void ConfigureCertsPath(sentry_options_t* Options) override;
 	virtual void ConfigureLogFileAttachment(sentry_options_t* Options) override;
-	virtual void ConfigureGpuDumpAttachment(sentry_options_t* Options) override;
 
 	virtual FString GetHandlerExecutableName() const override { return TEXT("crashpad_handler"); }
 };

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

@@ -59,12 +59,15 @@ FString SentryFileUtils::GetGpuDumpPath()
 		return FString("");
 	}
 
-	if (GpuDumpFiles.Num() > 1)
+	// By default, engine cleans up GPU dumps from the previous runs however this doesn't seem to be the case
+	// if https://github.com/EpicGames/UnrealEngine/pull/12648 patch is applied so we just return the newest one
+
+	for (int i = 0; i < GpuDumpFiles.Num(); ++i)
 	{
-		// By default, engine should handle clean up of GPU dumps  from the previous runs
-		UE_LOG(LogSentrySdk, Log, TEXT("There are multiple GPU dump files, can't determine reliably which one to pick."));
-		return FString("");
+		GpuDumpFiles[i] = FPaths::ProjectLogDir() / GpuDumpFiles[i];
 	}
 
-	return IFileManager::Get().ConvertToAbsolutePathForExternalAppForRead(*(FPaths::ProjectLogDir() / GpuDumpFiles[0]));
+	GpuDumpFiles.Sort(FSentrySortFileByDatePredicate());
+
+	return IFileManager::Get().ConvertToAbsolutePathForExternalAppForRead(*GpuDumpFiles[0]);
 }

plugin-dev/Source/Sentry/Private/Windows/WindowsSentrySubsystem.cpp

@@ -10,11 +10,6 @@
 #include "Windows/Infrastructure/WindowsSentryConverters.h"
 #include "Windows/WindowsPlatformStackWalk.h"
 
-void FWindowsSentrySubsystem::ConfigureGpuDumpAttachment(sentry_options_t* Options)
-{
-	sentry_options_add_attachmentw(Options, *GetGpuDumpBackupPath());
-}
-
 static void PrintCrashLog(const sentry_ucontext_t* uctx)
 {
 #if !UE_VERSION_OLDER_THAN(5, 0, 0)

plugin-dev/Source/Sentry/Private/Windows/WindowsSentrySubsystem.h

@@ -9,7 +9,6 @@
 class FWindowsSentrySubsystem : public FMicrosoftSentrySubsystem
 {
 protected:
-	virtual void ConfigureGpuDumpAttachment(sentry_options_t* Options) override;
 	virtual FString GetHandlerExecutableName() const override { return TEXT("crashpad_handler.exe"); }
 
 	virtual sentry_value_t OnCrash(const sentry_ucontext_t* uctx, sentry_value_t event, void* closure) override;

sample/Content/UI/W_SentryDemo.uasset

@@ -2,6 +2,7 @@
 
 #include "SentryPlaygroundUtils.h"
 
+#include "Engine/Engine.h"
 #include "HAL/FileManager.h"
 #include "Misc/FileHelper.h"
 #include "Misc/Paths.h"
@@ -44,6 +45,22 @@ void USentryPlaygroundUtils::Terminate(ESentryAppTerminationType Type)
 			UE_LOG(LogTemp, VeryVerbose, TEXT("Stack addr: %p"), &buffer);
 			Terminate(Type);
 			break;
+		case ESentryAppTerminationType::OutOfMemory:
+			{
+				size_t Count = 1024;
+				while (true)
+				{
+					void* _ = FMemory::Malloc(Count);
+					Count *= 2;
+				}
+			}
+			break;
+		case ESentryAppTerminationType::RenderThreadCrash:
+			GEngine->Exec(nullptr, TEXT("Debug RenderCrash"));
+			break;
+		case ESentryAppTerminationType::GpuDebugCrash:
+			GEngine->Exec(nullptr, TEXT("GPUDebugCrash platformbreak"));
+			break;
 		case ESentryAppTerminationType::FastFail:
 			{
 #if PLATFORM_MICROSOFT