Empty error message/traceback for custom Python Actions on Linux

[espinafre]

When trying to port some old custom python2 actions over to QGIS 3/Python 3, we don't have the error messages generated by the Python script; instead, we get the message "traceback.print_exception() failed", along with empty "Python version" and "Python path".

I think this the same issue as #31235 , and this existed since QGIS 2.18 ( https://issues.qgis.org/issues/16923 ); Windows users don't seem to have this problem. I believe that it is caused by the way threading works on Posix systems: in src/python/qgspythonutilsimpl.cpp, the method runString() calls runStringUnsafe(), which acquires then releases the GIL; if runStrtingUnsafe() returns error, runString() calls getTraceback(), which again acquires then releases the GIL, but by that time the error information is lost, the global error objects might have been cleared by some other thread, and all we get is nothingness.

To test this, I've made a very crude patch. It creates a copy of getTraceback() called getTracebackNoLock(), which doesn't attempt to acquire/release the GIL; then, it changes the return type of runStringUnsafe() to QString instead of bool; in case of errors, getTracebackNoLock() gets called there, and its output is returned to runStringUnsafe(), which is finally handled by runString().

This works for me, and I didn't see any unwanted effects, but I haven't tested it extensively.

diff --git a/src/python/qgspythonutils.h b/src/python/qgspythonutils.h
index ff2ab8d29e..821e7657cd 100644
--- a/src/python/qgspythonutils.h
+++ b/src/python/qgspythonutils.h
@@ -95,7 +95,8 @@ class PYTHON_EXPORT QgsPythonUtils
      * Runs a Python \a command. No error reporting is not performed.
      * \returns TRUE if no error occurred
      */
-    virtual bool runStringUnsafe( const QString &command, bool single = true ) = 0;
+    //virtual bool runStringUnsafe( const QString &command, bool single = true ) = 0;
+    virtual QString runStringUnsafe( const QString &command, bool single = true ) = 0; // QString.isEmpty() == true if no error
 
     /**
      * Evaluates a Python \a command and stores the result in a the \a result string.
diff --git a/src/python/qgspythonutilsimpl.cpp b/src/python/qgspythonutilsimpl.cpp
index 4cc8f3a7fd..eee4bb4798 100644
--- a/src/python/qgspythonutilsimpl.cpp
+++ b/src/python/qgspythonutilsimpl.cpp
@@ -294,30 +294,46 @@ void QgsPythonUtilsImpl::uninstallErrorHook()
   runString( QStringLiteral( "qgis.utils.uninstallErrorHook()" ) );
 }
 
-bool QgsPythonUtilsImpl::runStringUnsafe( const QString &command, bool single )
+QString QgsPythonUtilsImpl::runStringUnsafe( const QString &command, bool single )
 {
   // acquire global interpreter lock to ensure we are in a consistent state
   PyGILState_STATE gstate;
   gstate = PyGILState_Ensure();
+  QString ret;
 
   // TODO: convert special characters from unicode strings u"…" to \uXXXX
   // so that they're not mangled to utf-8
   // (non-unicode strings can be mangled)
   PyObject *obj = PyRun_String( command.toUtf8().constData(), single ? Py_single_input : Py_file_input, mMainDict, mMainDict );
-  bool res = nullptr == PyErr_Occurred();
+  PyObject *errobj = PyErr_Occurred();
+  //bool res = nullptr == PyErr_Occurred();
+  bool res = false;
+  if(nullptr == errobj)
+  {
+	  res = true;
+  }
+  else
+  {
+	  res = false;
+	  ret = getTracebackNoLock();
+  }
   Py_XDECREF( obj );
 
   // we are done calling python API, release global interpreter lock
   PyGILState_Release( gstate );
 
-  return res;
+  return ret;
 }
 
 bool QgsPythonUtilsImpl::runString( const QString &command, QString msgOnError, bool single )
 {
-  bool res = runStringUnsafe( command, single );
-  if ( res )
+  //bool res = runStringUnsafe( command, single );
+  bool res = true;
+  QString errstr = runStringUnsafe( command, single );
+  if ( errstr.isEmpty() )
     return true;
+  else
+	  res = false;
 
   if ( msgOnError.isEmpty() )
   {
@@ -327,7 +343,8 @@ bool QgsPythonUtilsImpl::runString( const QString &command, QString msgOnError,
 
   // TODO: use python implementation
 
-  QString traceback = getTraceback();
+  //QString traceback = getTraceback();
+  QString traceback = errstr;
   QString path, version;
   evalString( QStringLiteral( "str(sys.path)" ), path );
   evalString( QStringLiteral( "sys.version" ), version );
@@ -364,7 +381,7 @@ QString QgsPythonUtilsImpl::getTraceback()
   PyObject *obStringIO = nullptr;
   PyObject *obResult = nullptr;
 
-  PyObject *type, *value, *traceback;
+  PyObject *type = nullptr, *value = nullptr, *traceback = nullptr;
 
   PyErr_Fetch( &type, &value, &traceback );
   PyErr_NormalizeException( &type, &value, &traceback );
@@ -429,6 +446,80 @@ done:
   return result;
 }
 
+QString QgsPythonUtilsImpl::getTracebackNoLock()
+{
+#define TRACEBACK_FETCH_ERROR(what) {errMsg = what; goto done;}
+
+  QString errMsg;
+  QString result;
+
+  PyObject *modStringIO = nullptr;
+  PyObject *modTB = nullptr;
+  PyObject *obStringIO = nullptr;
+  PyObject *obResult = nullptr;
+
+  PyObject *type = nullptr, *value = nullptr, *traceback = nullptr;
+
+  PyErr_Fetch( &type, &value, &traceback );
+  PyErr_NormalizeException( &type, &value, &traceback );
+
+  const char *iomod = "io";
+
+  modStringIO = PyImport_ImportModule( iomod );
+  if ( !modStringIO )
+    TRACEBACK_FETCH_ERROR( QStringLiteral( "can't import %1" ).arg( iomod ) );
+
+  obStringIO = PyObject_CallMethod( modStringIO, reinterpret_cast< const char * >( "StringIO" ), nullptr );
+
+  /* Construct a cStringIO object */
+  if ( !obStringIO )
+    TRACEBACK_FETCH_ERROR( QStringLiteral( "cStringIO.StringIO() failed" ) );
+
+  modTB = PyImport_ImportModule( "traceback" );
+  if ( !modTB )
+    TRACEBACK_FETCH_ERROR( QStringLiteral( "can't import traceback" ) );
+
+  obResult = PyObject_CallMethod( modTB,  reinterpret_cast< const char * >( "print_exception" ),
+                                  reinterpret_cast< const char * >( "OOOOO" ),
+                                  type, value ? value : Py_None,
+                                  traceback ? traceback : Py_None,
+                                  Py_None,
+                                  obStringIO );
+
+  if ( !obResult )
+    TRACEBACK_FETCH_ERROR( QStringLiteral( "traceback.print_exception() failed" ) );
+
+  Py_DECREF( obResult );
+
+  obResult = PyObject_CallMethod( obStringIO,  reinterpret_cast< const char * >( "getvalue" ), nullptr );
+  if ( !obResult )
+    TRACEBACK_FETCH_ERROR( QStringLiteral( "getvalue() failed." ) );
+
+  /* And it should be a string all ready to go - duplicate it. */
+  if ( !PyUnicode_Check( obResult ) )
+    TRACEBACK_FETCH_ERROR( QStringLiteral( "getvalue() did not return a string" ) );
+
+  result = QString::fromUtf8( PyUnicode_AsUTF8( obResult ) );
+
+done:
+
+  // All finished - first see if we encountered an error
+  if ( result.isEmpty() && !errMsg.isEmpty() )
+  {
+    result = errMsg;
+  }
+
+  Py_XDECREF( modStringIO );
+  Py_XDECREF( modTB );
+  Py_XDECREF( obStringIO );
+  Py_XDECREF( obResult );
+  Py_XDECREF( value );
+  Py_XDECREF( traceback );
+  Py_XDECREF( type );
+
+  return result;
+}
+
 QString QgsPythonUtilsImpl::getTypeAsString( PyObject *obj )
 {
   if ( !obj )
diff --git a/src/python/qgspythonutilsimpl.h b/src/python/qgspythonutilsimpl.h
index a1cb7cb8fc..bb0b5c066b 100644
--- a/src/python/qgspythonutilsimpl.h
+++ b/src/python/qgspythonutilsimpl.h
@@ -44,7 +44,8 @@ class QgsPythonUtilsImpl : public QgsPythonUtils
     void exitPython() override;
     bool isEnabled() override;
     bool runString( const QString &command, QString msgOnError = QString(), bool single = true ) override;
-    bool runStringUnsafe( const QString &command, bool single = true ) override;
+    //bool runStringUnsafe( const QString &command, bool single = true ) override;
+    QString runStringUnsafe( const QString &command, bool single = true ) override; // returns error traceback on failure
     bool evalString( const QString &command, QString &result ) override;
     bool getError( QString &errorClassName, QString &errorText ) override;
 
@@ -117,6 +118,7 @@ class QgsPythonUtilsImpl : public QgsPythonUtils
     void uninstallErrorHook();
 
     QString getTraceback();
+    QString getTracebackNoLock();
 
     //! convert Python object to QString. If the object isn't unicode/str, it will be converted
     QString PyObjectToQString( PyObject *obj );

[gioman]

This works for me, and I didn't see any unwanted effects, but I haven't tested it extensively.

@espinafre Can you submit a patch with a pull request?

[espinafre]

@gioman I'll clean it up a little and try to do that. A quick grep shows that QgsPythonUtilsImpl::getTraceback() is only used within that class, so removing its locks should do no harm. The same goes for QgsPythonUtilsImpl::runStringUnsafe(), it is not called anywhere else in the QGIS tree, so it should be safe to change its return type (unless this violates some QGIS code of style which I'm not aware of; I'm new to this).