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

* Re-enable external crash handler on windows

Signed-off-by: guillaume-haerinck <[email protected]>

* Include crash handler to most standalone tools

Signed-off-by: guillaume-haerinck <[email protected]>

* Allow to manually report an issue

Signed-off-by: guillaume-haerinck <[email protected]>

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

Signed-off-by: guillaume-haerinck <[email protected]>

* Conditionally add external crash reporter as dependency

Signed-off-by: guillaume-haerinck <[email protected]>

* Remove old pal annotations

Signed-off-by: guillaume-haerinck <[email protected]>

---------

Signed-off-by: guillaume-haerinck <[email protected]>

.gitignore

@@ -23,3 +23,4 @@ _savebackup/
 *.swatches
 /imgui.ini
 .command_settings
+CrashDB/

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 

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;

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.

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

Code/Tools/CrashHandler/Platform/Android/PAL_android.cmake

@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)

Code/Tools/CrashHandler/Platform/Linux/PAL_linux.cmake

@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)

Code/Tools/CrashHandler/Platform/Mac/PAL_mac.cmake

@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)

Code/Tools/CrashHandler/Platform/Windows/PAL_windows.cmake

@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED TRUE)

Code/Tools/CrashHandler/Platform/iOS/PAL_ios.cmake

@@ -6,4 +6,3 @@
 #
 #
 
-set(PAL_TRAIT_BUILD_CRASH_HANDLER_SUPPORTED FALSE)

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

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

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)

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>

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);
     }

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

Code/Tools/CrashHandler/Tools/Uploader/ToolsCrashUploader.cpp

@@ -21,8 +21,12 @@
 #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>
 
@@ -30,6 +34,9 @@
 
 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;

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

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)

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

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

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

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");

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

Gems/Atom/Tools/MaterialEditor/Code/Source/MaterialEditorApplication.cpp

@@ -13,6 +13,10 @@
 #include <Document/MaterialDocument.h>
 #include <Window/MaterialEditorMainWindow.h>
 
+#if defined(EXTERNAL_CRASH_REPORTING)
+#include <ToolsCrashHandler.h>
+#endif
+
 void InitMaterialEditorResources()
 {
     // Must register qt resources from other modules
@@ -34,6 +38,9 @@ namespace MaterialEditor
     MaterialEditorApplication::MaterialEditorApplication(int* argc, char*** argv)
         : Base(GetBuildTargetName(), argc, argv)
     {
+#if defined(EXTERNAL_CRASH_REPORTING)
+        CrashHandler::ToolsCrashHandler::InitCrashHandler("MaterialEditor", {});
+#endif
         InitMaterialEditorResources();
 
         QApplication::setOrganizationName("O3DE");

Gems/Atom/Tools/PassCanvas/Code/CMakeLists.txt

@@ -43,6 +43,10 @@ ly_add_target(
         Gem::AtomToolsFramework.Editor
 )
 
+if(PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED)
+    ly_target_link_libraries(PassCanvas PRIVATE AZ::ToolsCrashHandler)
+endif()
+
 ly_set_gem_variant_to_load(TARGETS PassCanvas VARIANTS Tools)
 
 # Add a 'builders' alias to allow the PassCanvas root gem path to be added to the generated

Gems/Atom/Tools/PassCanvas/Code/Source/PassCanvasApplication.cpp

@@ -27,6 +27,10 @@
 
 #include <QLabel>
 
+#if defined(EXTERNAL_CRASH_REPORTING)
+#include <ToolsCrashHandler.h>
+#endif
+
 void InitPassCanvasResources()
 {
     // Must register qt resources from other modules
@@ -49,6 +53,9 @@ namespace PassCanvas
     PassCanvasApplication::PassCanvasApplication(int* argc, char*** argv)
         : Base(GetBuildTargetName(), argc, argv)
     {
+#if defined(EXTERNAL_CRASH_REPORTING)
+        CrashHandler::ToolsCrashHandler::InitCrashHandler("PassCanvas", {});
+#endif
         InitPassCanvasResources();
 
         QApplication::setOrganizationName("O3DE");

Gems/Atom/Tools/ShaderManagementConsole/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(ShaderManagementConsole PRIVATE AZ::ToolsCrashHandler)
+endif()
+
 ly_set_gem_variant_to_load(TARGETS ShaderManagementConsole VARIANTS Tools)
 
 # Add a 'builders' alias to allow the ShaderManagementConsole root gem path to be added to the generated

Gems/Atom/Tools/ShaderManagementConsole/Code/Source/ShaderManagementConsoleApplication.cpp

@@ -24,6 +24,10 @@
 
 #include <QMenu>
 
+#if defined(EXTERNAL_CRASH_REPORTING)
+#include <ToolsCrashHandler.h>
+#endif
+
 void InitShaderManagementConsoleResources()
 {
     // Must register qt resources from other modules
@@ -45,6 +49,9 @@ namespace ShaderManagementConsole
     ShaderManagementConsoleApplication::ShaderManagementConsoleApplication(int* argc, char*** argv)
         : Base(GetBuildTargetName(), argc, argv)
     {
+#if defined(EXTERNAL_CRASH_REPORTING)
+        CrashHandler::ToolsCrashHandler::InitCrashHandler("ShaderManagementConsole", {});
+#endif
         InitShaderManagementConsoleResources();
 
         QApplication::setOrganizationName("O3DE");

Gems/CrashReporting/Code/CMakeLists.txt

@@ -10,7 +10,7 @@ o3de_pal_dir(pal_dir ${CMAKE_CURRENT_LIST_DIR}/Platform/${PAL_PLATFORM_NAME} "${
 
 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()
 

cmake/Platform/Windows/PAL_windows.cmake

@@ -18,6 +18,7 @@ ly_set(PAL_TRAIT_BUILD_UNITY_SUPPORTED TRUE)
 ly_set(PAL_TRAIT_BUILD_UNITY_EXCLUDE_EXTENSIONS)
 ly_set(PAL_TRAIT_BUILD_EXCLUDE_ALL_TEST_RUNS_FROM_IDE FALSE)
 ly_set(PAL_TRAIT_BUILD_CPACK_SUPPORTED TRUE)
+ly_set(PAL_TRAIT_BUILD_EXTERNAL_CRASH_HANDLER_SUPPORTED FALSE)
 
 ly_set(PAL_TRAIT_PROF_PIX_SUPPORTED TRUE)