External crash reporter : Extend implementation, but leave it disabled for now (o3de#18631)

guillaume-haerinck · web-flow · commit 183de5ba2aa0 · 2025-05-27T00:47:05.000-07:00
* Re-enable external crash handler on windows

Signed-off-by: guillaume-haerinck &lt;guillaume.haerinck@outlook.com&gt;

* Include crash handler to most standalone tools

Signed-off-by: guillaume-haerinck &lt;guillaume.haerinck@outlook.com&gt;

* Allow to manually report an issue

Signed-off-by: guillaume-haerinck &lt;guillaume.haerinck@outlook.com&gt;

* Disable crash handler for now as need more than just dump

Signed-off-by: guillaume-haerinck &lt;guillaume.haerinck@outlook.com&gt;

* Conditionally add external crash reporter as dependency

Signed-off-by: guillaume-haerinck &lt;guillaume.haerinck@outlook.com&gt;

* Remove old pal annotations

Signed-off-by: guillaume-haerinck &lt;guillaume.haerinck@outlook.com&gt;

---------

Signed-off-by: guillaume-haerinck &lt;guillaume.haerinck@outlook.com&gt;
diff --git a/.gitignore b/.gitignore
@@ -23,3 +23,4 @@ _savebackup/
 *.swatches
 /imgui.ini
 .command_settings
+CrashDB/
diff --git a/Code/Editor/CMakeLists.txt b/Code/Editor/CMakeLists.txt
@@ -122,6 +122,11 @@ ly_add_target(
     RUNTIME_DEPENDENCIES
         Legacy::EditorCommon
 )
+
+if(PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED)
+    ly_target_link_libraries(EditorLib PRIVATE AZ::ToolsCrashHandler)
+endif()
+
 ly_add_source_properties(
     SOURCES CryEdit.cpp
     PROPERTY COMPILE_DEFINITIONS 
diff --git a/Code/Legacy/CrySystem/DebugCallStack.cpp b/Code/Legacy/CrySystem/DebugCallStack.cpp
@@ -604,7 +604,7 @@ void DebugCallStack::SaveExceptionInfoAndShowUserReportDialogs(EXCEPTION_POINTER
     else if (auto nativeUI = AZ::Interface<AZ::NativeUI::NativeUIRequests>::Get(); nativeUI != nullptr)
     {
         AZStd::string msg = AZStd::string::format(
-            "O3DE has encountered an unexpected error.\n\nDo you want to manually report the issue on Github ?\nInformation about the "
+            "O3DE has encountered an unexpected error.\n\nDo you want to manually report the issue ?\nInformation about the "
             "crash are located in %s via error.log and error.dmp",
             path.c_str());
         constexpr bool showCancel = false;
diff --git a/Code/Tools/AssetProcessor/CMakeLists.txt b/Code/Tools/AssetProcessor/CMakeLists.txt
@@ -78,6 +78,10 @@ ly_add_target(
         AZ::AssetBuilder
 )
 
+if(PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED)
+    ly_target_link_libraries(AssetProcessor PRIVATE AZ::ToolsCrashHandler)
+endif()
+
 # Adds the AssetProcessor target as a C preprocessor define so that it can be used as a Settings Registry
 # specialization in order to look up the generated .setreg which contains the dependencies
 # specified for the target.
diff --git a/Code/Tools/CrashHandler/CMakeLists.txt b/Code/Tools/CrashHandler/CMakeLists.txt
@@ -10,7 +10,7 @@ o3de_pal_dir(pal_dir ${CMAKE_CURRENT_LIST_DIR}/Platform/${PAL_PLATFORM_NAME} ${O
 
 include(${pal_dir}/PAL_${PAL_PLATFORM_NAME_LOWERCASE}.cmake)
 
-if(NOT PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED)
+if(NOT PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED)
     return()
 endif()
 
@@ -28,6 +28,9 @@ ly_add_target(
             Shared
         PRIVATE
             ${pal_dir}
+    COMPILE_DEFINITIONS
+        PUBLIC
+            EXTERNAL_CRASH_REPORTING
     BUILD_DEPENDENCIES
         PUBLIC
             AZ::CrashSupport
diff --git a/Code/Tools/CrashHandler/Platform/Android/PAL_android.cmake b/Code/Tools/CrashHandler/Platform/Android/PAL_android.cmake
@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)
diff --git a/Code/Tools/CrashHandler/Platform/Linux/PAL_linux.cmake b/Code/Tools/CrashHandler/Platform/Linux/PAL_linux.cmake
@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)
diff --git a/Code/Tools/CrashHandler/Platform/Mac/PAL_mac.cmake b/Code/Tools/CrashHandler/Platform/Mac/PAL_mac.cmake
@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)
diff --git a/Code/Tools/CrashHandler/Platform/Windows/PAL_windows.cmake b/Code/Tools/CrashHandler/Platform/Windows/PAL_windows.cmake
@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED TRUE)
diff --git a/Code/Tools/CrashHandler/Platform/iOS/PAL_ios.cmake b/Code/Tools/CrashHandler/Platform/iOS/PAL_ios.cmake
@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)
diff --git a/Code/Tools/CrashHandler/Shared/CrashHandler.cpp b/Code/Tools/CrashHandler/Shared/CrashHandler.cpp
@@ -22,6 +22,8 @@
 #include <CrashSupport.h>
 
 #include <AzCore/Module/Environment.h>
+#include <AzCore/Settings/SettingsRegistry.h>
+#include <AzCore/std/string/string.h>
 #include <CrashHandler_Traits_Platform.h>
 
 namespace CrashHandler
@@ -161,8 +163,19 @@ namespace CrashHandler
     {
         ReadConfigFile();
 
-        const std::string url{ crashUrl.length() ? crashUrl : GetCrashSubmissionURL() };
-        const std::string token{ crashToken.length() ? crashToken : GetCrashSubmissionToken() };
+        // If url or token is empty, the client will exit after launch, returning false for WaitForHandlerStart
+        std::string url{ crashUrl.length() ? crashUrl : GetCrashSubmissionURL() };
+        std::string token{ crashToken.length() ? crashToken : GetCrashSubmissionToken() };
+
+        if (url.empty())
+        {
+            url = manualReportUrl;
+        }
+
+        if (token.empty())
+        {
+            token = manualReportToken;
+        }
 
         std::string lyAppRoot{ appRoot };
         AppendSep(lyAppRoot);
diff --git a/Code/Tools/CrashHandler/Shared/CrashHandler.h b/Code/Tools/CrashHandler/Shared/CrashHandler.h
@@ -18,6 +18,8 @@ namespace CrashHandler
 {
     static const char* defaultCrashFolder = "CrashDB/";
     static const char* O3DEProductName = "O3DE";
+    static const char* manualReportToken = "manual";
+    static const char* manualReportUrl = "https://manual";
 
     using CrashHandlerAnnotations = std::map<std::string, std::string>;
     using CrashHandlerArguments = std::vector<std::string>;
diff --git a/Code/Tools/CrashHandler/Tools/CMakeLists.txt b/Code/Tools/CrashHandler/Tools/CMakeLists.txt
@@ -49,3 +49,5 @@ ly_add_target(
     TARGET_PROPERTIES
         Qt5_NO_LINK_QTMAIN TRUE
 )
+
+ly_add_dependencies(Editor ToolsCrashUploader)
diff --git a/Code/Tools/CrashHandler/Tools/UI/submit_report.ui b/Code/Tools/CrashHandler/Tools/UI/submit_report.ui
@@ -96,7 +96,7 @@
     </rect>
    </property>
    <property name="text">
-    <string>An Open 3D Engine Editor crash debugging file has been created at:</string>
+    <string>Information about the crash has been created at:</string>
    </property>
    <property name="wordWrap">
     <bool>true</bool>
@@ -134,13 +134,13 @@
     </rect>
    </property>
    <property name="text">
-    <string>If you are willing to submit this file to Amazon it will help us improve the Open 3D Engine experience.  We will treat this report as confidential.</string>
+    <string>If you are willing to submit this file it will help us to fix this issue.</string>
    </property>
    <property name="wordWrap">
     <bool>true</bool>
    </property>
   </widget>
-  <widget class="QLabel" name="label_4">
+  <widget class="QLabel" name="question_label">
    <property name="geometry">
     <rect>
      <x>6</x>
diff --git a/Code/Tools/CrashHandler/Tools/Uploader/SendReportDialog.cpp b/Code/Tools/CrashHandler/Tools/Uploader/SendReportDialog.cpp
@@ -9,21 +9,29 @@
 #include "SendReportDialog.h"
 #include "UI/ui_submit_report.h"
 
+#include <QLabel>
+
 namespace CrashUploader
 {
 
-    SendReportDialog::SendReportDialog(QWidget* parent)
+    SendReportDialog::SendReportDialog(bool manualReport, QWidget* parent)
         : QDialog(parent)
         , ui(new Ui::SendReportDialog)
+        , m_manualReport(manualReport)
     {
         ui->setupUi(this);
+
+        if (manualReport)
+        {
+            ui->question_label->setText("Would you like to manually report the issue ?");
+        }
     }
 
     SendReportDialog::~SendReportDialog()
     {
     }
 
-    void SendReportDialog::SetReportText(const char* reportText)
+    void SendReportDialog::SetReportText(const QString& reportText)
     {
         ui->dump_label->setText(reportText);
     }
diff --git a/Code/Tools/CrashHandler/Tools/Uploader/SendReportDialog.h b/Code/Tools/CrashHandler/Tools/Uploader/SendReportDialog.h
@@ -24,13 +24,15 @@ namespace CrashUploader
         Q_OBJECT // AUTOMOC
 
     public:
-        explicit SendReportDialog(QWidget* parent = nullptr);
+        SendReportDialog(bool manualReport, QWidget* parent = nullptr);
         ~SendReportDialog() override;
 
-        void SetReportText(const char* reportPath);
+        void SetReportText(const QString& reportPath);
         void SetApplicationName(const char* appName);
+
     private:
         QScopedPointer<Ui::SendReportDialog> ui;
+        bool m_manualReport = false;
     };
 
 }
diff --git a/Code/Tools/CrashHandler/Tools/Uploader/ToolsCrashUploader.cpp b/Code/Tools/CrashHandler/Tools/Uploader/ToolsCrashUploader.cpp
@@ -21,15 +21,22 @@
 #include <AzCore/Settings/SettingsRegistryMergeUtils.h>
 #include <AzCore/std/typetraits/underlying_type.h>
 
-#include <QString>
 #include <QApplication>
+#include <QDesktopServices>
+#include <QDir>
+#include <QFileInfo>
+#include <QString>
+#include <QUrl>
 
 #include <memory>
 
 #include "UI/ui_submit_report.h"
 
 namespace O3de
 {
+    static const char* settingKey_issueReportLink = "/O3DE/Settings/Links/Issue/Create";
+    static const char* issueReportLinkFallback = "https://github.com/o3de/o3de/issues/new/choose";
+
     void InstallCrashUploader(int& argc, char* argv[])
     {
         O3de::CrashUploader::SetCrashUploader(std::make_shared<O3de::ToolsCrashUploader>(argc, argv));
@@ -99,11 +106,14 @@ namespace O3de
             QString reportString{ GetReportString(report.file_path.value()).toStdString().c_str() };
             QAtomicInt dialogResult{ -1 };
 
-            ::CrashUploader::SendReportDialog confirmDialog{ nullptr };
+            const bool manualReport = m_submissionToken.compare("manual") == 0;
+            ::CrashUploader::SendReportDialog confirmDialog{ manualReport, nullptr };
             confirmDialog.SetApplicationName(m_executableName.c_str());
 
             confirmDialog.setWindowIcon(QIcon(":/Icons/editor_icon.ico"));
-            confirmDialog.SetReportText(reportString.toStdString().c_str());
+
+            QFileInfo fileInfo = QFileInfo(reportPath);
+            confirmDialog.SetReportText(fileInfo.absoluteFilePath());
             confirmDialog.setWindowFlags((confirmDialog.windowFlags() | Qt::WindowStaysOnTopHint) & ~Qt::WindowContextHelpButtonHint);
 
             confirmDialog.exec();
@@ -113,7 +123,25 @@ namespace O3de
                 app.processEvents();
                 dialogResult = confirmDialog.result();
             }
-            return dialogResult == QDialog::Accepted;
+
+            const bool accepted = dialogResult == QDialog::Accepted;
+            if (manualReport && accepted)
+            {
+                QDesktopServices::openUrl(QUrl(fileInfo.absolutePath()));
+
+                AZStd::string reportIssueUrl;
+                if (auto settingsRegistry = AZ::SettingsRegistry::Get(); settingsRegistry != nullptr)
+                {
+                    settingsRegistry->Get(reportIssueUrl, settingKey_issueReportLink);
+                }
+
+                if (reportIssueUrl.empty())
+                {
+                    reportIssueUrl = issueReportLinkFallback;
+                }
+                QDesktopServices::openUrl(QUrl(reportIssueUrl.data()));
+            }
+            return accepted;
         }
 #endif
         return true;
diff --git a/Code/Tools/CrashHandler/Uploader/src/CrashUploader.cpp b/Code/Tools/CrashHandler/Uploader/src/CrashUploader.cpp
@@ -199,6 +199,29 @@ namespace O3de
             {nullptr, no_argument, nullptr, EndFlags}
         };
 
+        for (int i = 0; i < argc; ++i)
+        {
+            std::string inputString = argv[i];
+            size_t equalPos = inputString.find_first_of('=');
+            if (equalPos == std::string::npos || equalPos == inputString.length() - 1)
+            {
+                continue;
+            }
+
+            std::string tokenStr = inputString.substr(0, equalPos);
+            std::string valueStr = inputString.substr(equalPos + 1);
+
+            if (tokenStr == "--submission-token")
+            {
+                m_submissionToken = valueStr;
+            }
+            else if (tokenStr == "--executable-name")
+            {
+                m_executableName = valueStr;
+            }
+        }
+
+        // FIXME: argument parsing is broken, fallback above
         int opt{ 0 };
         while ((opt = getopt_long(argc, argv, "", options_list, nullptr)) != -1)
         {
diff --git a/Code/Tools/LuaIDE/CMakeLists.txt b/Code/Tools/LuaIDE/CMakeLists.txt
@@ -40,5 +40,9 @@ ly_add_target(
             STANDALONETOOLS_ENABLE_LUA_IDE
 )
 
+if(PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED)
+    ly_target_link_libraries(LuaIDE PRIVATE AZ::ToolsCrashHandler)
+endif()
+
 ly_set_gem_variant_to_load(TARGETS LuaIDE VARIANTS LuaTools)
 ly_add_dependencies(Editor LuaIDE)
diff --git a/Code/Tools/ProjectManager/CMakeLists.txt b/Code/Tools/ProjectManager/CMakeLists.txt
@@ -47,6 +47,10 @@ ly_add_target(
             AZ::AzToolsFramework
 )
 
+if(PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED)
+    ly_target_link_libraries(ProjectManager.Static PRIVATE AZ::ToolsCrashHandler)
+endif()
+
 ly_add_target(
     NAME ProjectManager APPLICATION
     OUTPUT_NAME o3de
diff --git a/Code/Tools/ProjectManager/Source/Application.cpp b/Code/Tools/ProjectManager/Source/Application.cpp
@@ -24,6 +24,10 @@
 #include <QInputDialog>
 #include <QIcon>
 
+#if defined(EXTERNAL_CRASH_REPORTING)
+#include <ToolsCrashHandler.h>
+#endif
+
 namespace O3DE::ProjectManager
 {
     Application::~Application()
@@ -33,6 +37,10 @@ namespace O3DE::ProjectManager
 
     bool Application::Init(bool interactive, AZStd::unique_ptr<PythonBindings> pythonBindings)
     {
+#if defined(EXTERNAL_CRASH_REPORTING)
+        CrashHandler::ToolsCrashHandler::InitCrashHandler("o3de", {});
+#endif
+
         constexpr const char* applicationName { "O3DE" };
 
         QApplication::setOrganizationName(applicationName);
diff --git a/Gems/Atom/Tools/MaterialCanvas/Code/CMakeLists.txt b/Gems/Atom/Tools/MaterialCanvas/Code/CMakeLists.txt
@@ -44,6 +44,10 @@ ly_add_target(
         Gem::AtomToolsFramework.Editor
 )
 
+if(PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED)
+    ly_target_link_libraries(MaterialCanvas PRIVATE AZ::ToolsCrashHandler)
+endif()
+
 ly_set_gem_variant_to_load(TARGETS MaterialCanvas VARIANTS Tools)
 
 # Add a 'builders' alias to allow the MaterialCanvas root gem path to be added to the generated
diff --git a/Gems/Atom/Tools/MaterialCanvas/Code/Source/MaterialCanvasApplication.cpp b/Gems/Atom/Tools/MaterialCanvas/Code/Source/MaterialCanvasApplication.cpp
@@ -30,6 +30,10 @@
 
 #include <QLabel>
 
+#if defined(EXTERNAL_CRASH_REPORTING)
+#include <ToolsCrashHandler.h>
+#endif
+
 void InitMaterialCanvasResources()
 {
     // Must register qt resources from other modules
@@ -52,6 +56,9 @@ namespace MaterialCanvas
     MaterialCanvasApplication::MaterialCanvasApplication(int* argc, char*** argv)
         : Base(GetBuildTargetName(), argc, argv)
     {
+#if defined(EXTERNAL_CRASH_REPORTING)
+        CrashHandler::ToolsCrashHandler::InitCrashHandler("MaterialCanvas", {});
+#endif
         InitMaterialCanvasResources();
 
         QApplication::setOrganizationName("O3DE");
diff --git a/Gems/Atom/Tools/MaterialEditor/Code/CMakeLists.txt b/Gems/Atom/Tools/MaterialEditor/Code/CMakeLists.txt
@@ -44,6 +44,10 @@ ly_add_target(
         Gem::AtomToolsFramework.Editor
 )
 
+if(PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED)
+    ly_target_link_libraries(MaterialEditor PRIVATE AZ::ToolsCrashHandler)
+endif()
+
 ly_set_gem_variant_to_load(TARGETS MaterialEditor VARIANTS Tools)
 
 # Add a 'builders' alias to allow the MaterialEditor root gem path to be added to the generated
diff --git a/Gems/Atom/Tools/MaterialEditor/Code/Source/MaterialEditorApplication.cpp b/Gems/Atom/Tools/MaterialEditor/Code/Source/MaterialEditorApplication.cpp
diff --git a/Gems/Atom/Tools/PassCanvas/Code/CMakeLists.txt b/Gems/Atom/Tools/PassCanvas/Code/CMakeLists.txt
diff --git a/Gems/Atom/Tools/PassCanvas/Code/Source/PassCanvasApplication.cpp b/Gems/Atom/Tools/PassCanvas/Code/Source/PassCanvasApplication.cpp
diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/CMakeLists.txt b/Gems/Atom/Tools/ShaderManagementConsole/Code/CMakeLists.txt
diff --git a/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp b/Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp
diff --git a/Gems/CrashReporting/Code/CMakeLists.txt b/Gems/CrashReporting/Code/CMakeLists.txt
diff --git a/cmake/Platform/Windows/PAL_windows.cmake b/cmake/Platform/Windows/PAL_windows.cmake

Original file line number	Diff line number	Diff line change
`@@ -604,7 +604,7 @@ void DebugCallStack::SaveExceptionInfoAndShowUserReportDialogs(EXCEPTION_POINTER`
`604`	`604`	`else if (auto nativeUI = AZ::Interface<AZ::NativeUI::NativeUIRequests>::Get(); nativeUI != nullptr)`
`605`	`605`	`{`
`606`	`606`	`AZStd::string msg = AZStd::string::format(`
`607`		`- "O3DE has encountered an unexpected error.\n\nDo you want to manually report the issue on Github ?\nInformation about the "`
	`607`	`+ "O3DE has encountered an unexpected error.\n\nDo you want to manually report the issue ?\nInformation about the "`
`608`	`608`	`"crash are located in %s via error.log and error.dmp",`
`609`	`609`	`path.c_str());`
`610`	`610`	`constexpr bool showCancel = false;`
Original file line number	Diff line number	Diff line change
`@@ -6,4 +6,3 @@`
`6`	`6`	`#`
`7`	`7`	`#`
`8`	`8`
`9`		`-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)`