Improve error reporting when running Python action plugins.

Python error traceback is now logged to stderr.
On Windows, a console window will pop up if wxTheApp gets destroyed,
because message dialogs crash due to hooks that wxWidgets sets up.

Fixes https://gitlab.com/kicad/code/kicad/-/issues/15520

libs/kiplatform/gtk/app.cpp

@@ -48,6 +48,13 @@ bool KIPLATFORM::APP::Init()
 }
 
 
+bool KIPLATFORM::APP::AttachConsole( bool aTryAlloc )
+{
+    // Not implemented on this platform
+    return true;
+}
+
+
 bool KIPLATFORM::APP::IsOperatingSystemUnsupported()
 {
     // Not implemented on this platform

libs/kiplatform/include/kiplatform/app.h

@@ -36,6 +36,14 @@ namespace KIPLATFORM
          */
         bool Init();
 
+        /**
+         * Tries to attach a console window with stdout, stderr and stdin.
+         * 
+         * @param aTryAlloc try to allocate the console if cannot attach to it.
+         * @return true if attach successful, false if unsuccessful
+         */
+        bool AttachConsole( bool aTryAlloc );
+
         /**
          * Checks if the Operating System is explicitly unsupported and we want to prevent
          * users from sending bug reports and show them a disclaimer on startup.

libs/kiplatform/msw/app.cpp

@@ -84,43 +84,7 @@ bool KIPLATFORM::APP::Init()
 
     // In order to support GUI and CLI
     // Let's attach to console when it's possible, or allocate if requested.
-    bool tryAlloc = wxGetEnv( wxS( "KICAD_ALLOC_CONSOLE" ), nullptr );
+    AttachConsole( wxGetEnv( wxS( "KICAD_ALLOC_CONSOLE" ), nullptr ) );
-
-    HANDLE handle;
-    if( AttachConsole( ATTACH_PARENT_PROCESS ) || ( tryAlloc && AllocConsole() ) )
-    {
-        #if !defined( __MINGW32__ ) // These redirections create problems on mingw:
-                                    // Nothing is printed to the console
-
-
-        if( GetStdHandle( STD_INPUT_HANDLE ) != INVALID_HANDLE_VALUE )
-        {
-            freopen( "CONIN$", "r", stdin );
-            setvbuf( stdin, NULL, _IONBF, 0 );
-        }
-
-        if( GetStdHandle( STD_OUTPUT_HANDLE ) != INVALID_HANDLE_VALUE )
-        {
-            freopen( "CONOUT$", "w", stdout );
-            setvbuf( stdout, NULL, _IONBF, 0 );
-        }
-
-        if( GetStdHandle( STD_ERROR_HANDLE ) != INVALID_HANDLE_VALUE )
-        {
-            freopen( "CONOUT$", "w", stderr );
-            setvbuf( stderr, NULL, _IONBF, 0 );
-        }
-        #endif
-
-        std::ios::sync_with_stdio( true );
-
-        std::wcout.clear();
-        std::cout.clear();
-        std::wcerr.clear();
-        std::cerr.clear();
-        std::wcin.clear();
-        std::cin.clear();
-    }
 
     // It may be useful to log up to traces in a console, but in Release builds the log level changes to Info
     // Also we have to force the active target to stderr or else it goes to the void
@@ -138,6 +102,48 @@ bool KIPLATFORM::APP::Init()
 }
 
 
+bool KIPLATFORM::APP::AttachConsole( bool aTryAlloc )
+{
+    if( ::AttachConsole( ATTACH_PARENT_PROCESS ) || ( aTryAlloc && ::AllocConsole() ) )
+    {
+        #if !defined( __MINGW32__ ) // These redirections create problems on mingw:
+                                    // Nothing is printed to the console
+
+        if( ::GetStdHandle( STD_INPUT_HANDLE ) != INVALID_HANDLE_VALUE )
+        {
+            freopen( "CONIN$", "r", stdin );
+            setvbuf( stdin, NULL, _IONBF, 0 );
+        }
+
+        if( ::GetStdHandle( STD_OUTPUT_HANDLE ) != INVALID_HANDLE_VALUE )
+        {
+            freopen( "CONOUT$", "w", stdout );
+            setvbuf( stdout, NULL, _IONBF, 0 );
+        }
+
+        if( ::GetStdHandle( STD_ERROR_HANDLE ) != INVALID_HANDLE_VALUE )
+        {
+            freopen( "CONOUT$", "w", stderr );
+            setvbuf( stderr, NULL, _IONBF, 0 );
+        }
+        #endif
+
+        std::ios::sync_with_stdio( true );
+
+        std::wcout.clear();
+        std::cout.clear();
+        std::wcerr.clear();
+        std::cerr.clear();
+        std::wcin.clear();
+        std::cin.clear();
+
+        return true;
+    }
+
+    return false;
+}
+
+
 bool KIPLATFORM::APP::IsOperatingSystemUnsupported()
 {
 #if defined( PYTHON_VERSION_MAJOR ) && ( ( PYTHON_VERSION_MAJOR == 3 && PYTHON_VERSION_MINOR >= 8 ) \

libs/kiplatform/osx/app.mm

@@ -32,6 +32,13 @@ bool KIPLATFORM::APP::Init()
 }
 
 
+bool KIPLATFORM::APP::AttachConsole( bool aTryAlloc )
+{
+    // Not implemented on this platform
+    return true;
+}
+
+
 bool KIPLATFORM::APP::IsOperatingSystemUnsupported()
 {
     // Not implemented on this platform

pcbnew/python/scripting/pcbnew_action_plugins.cpp

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2017-2021 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2017-2023 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -41,6 +41,7 @@
 #include <tools/pcb_selection_tool.h>
 #include <pcb_painter.h>
 #include <wx/msgdlg.h>
+#include <kiplatform/app.h>
 #include "../../scripting/python_scripting.h"
 
 PYTHON_ACTION_PLUGIN::PYTHON_ACTION_PLUGIN( PyObject* aAction )
@@ -73,11 +74,57 @@ PyObject* PYTHON_ACTION_PLUGIN::CallMethod( const char* aMethod, PyObject* aArgl
     {
         PyObject* result = PyObject_CallObject( pFunc, aArglist );
 
+        if( !wxTheApp )
+            KIPLATFORM::APP::AttachConsole( true );
+
         if( PyErr_Occurred() )
         {
-            wxMessageBox( PyErrStringWithTraceback(),
+            wxString message = _HKI( "Exception on python action plugin code" );
-                          _( "Exception on python action plugin code" ),
+            wxString traceback = PyErrStringWithTraceback();
-                          wxICON_ERROR | wxOK );
+
+            std::cerr << message << std::endl << std::endl;
+            std::cerr << traceback << std::endl;
+
+            if( wxTheApp )
+                wxSafeShowMessage( wxGetTranslation( message ), traceback );
+        }
+
+        if( !wxTheApp )
+        {
+            wxArrayString messages;
+            messages.Add( "Fatal error");
+            messages.Add( wxString::Format(
+                    "The application handle was destroyed after running Python plugin '%s'.",
+                    m_cachedName ) );
+
+            // Poor man's ASCII message box
+            {
+                int maxLen = 0;
+
+                for( const wxString& msg : messages )
+                    if( msg.length() > maxLen )
+                        maxLen = msg.length();
+
+                wxChar   ch = '*';
+                wxString border( ch, 3 );
+
+                std::cerr << wxString( ch, maxLen + 2 + border.length() * 2 ) << std::endl;
+                std::cerr << border << ' ' << wxString( ' ', maxLen ) << ' ' << border << std::endl;
+
+                for( wxString msg : messages )
+                    std::cerr << border << ' ' << msg.Pad( maxLen - msg.length() ) << ' ' << border
+                              << std::endl;
+
+                std::cerr << border << ' ' << wxString( ' ', maxLen ) << ' ' << border << std::endl;
+                std::cerr << wxString( ch, maxLen + 2 + border.length() * 2 ) << std::endl;
+            }
+
+#ifdef _WIN32
+            std::cerr << std::endl << "Press any key to abort..." << std::endl;
+            (void) std::getchar();
+#endif
+
+            abort();
         }
 
         if( result )
@@ -127,7 +174,10 @@ wxString PYTHON_ACTION_PLUGIN::GetName()
 {
     PyLOCK lock;
 
-    return CallRetStrMethod( "GetName" );
+    wxString name = CallRetStrMethod( "GetName" );
+    m_cachedName = name;
+
+    return name;
 }
 
 

pcbnew/python/scripting/pcbnew_action_plugins.h

@@ -1,7 +1,7 @@
 /*
  * This program source code file is part of KiCad, a free EDA CAD application.
  *
- * Copyright (C) 2017-2021 KiCad Developers, see AUTHORS.txt for contributors.
+ * Copyright (C) 2017-2023 KiCad Developers, see AUTHORS.txt for contributors.
  *
  * This program is free software; you can redistribute it and/or
  * modify it under the terms of the GNU General Public License
@@ -52,9 +52,11 @@ public:
     void*       GetObject() override;
 
 private:
+    wxString  m_cachedName;
+
     PyObject* m_PyAction;
     PyObject* CallMethod( const char* aMethod, PyObject* aArglist = nullptr );
-    wxString CallRetStrMethod( const char* aMethod, PyObject* aArglist = nullptr );
+    wxString  CallRetStrMethod( const char* aMethod, PyObject* aArglist = nullptr );
 };