Fixed Python bug related to invalid return parameters, fixes #550

2 files changed, 37 insertions, 31 deletions

diff --git a/python/modules/IcePy/ObjectAdapter.cpp b/python/modules/IcePy/ObjectAdapter.cpp
index 03e6167182a..166ba647a53 100644
--- a/python/modules/IcePy/ObjectAdapter.cpp
+++ b/python/modules/IcePy/ObjectAdapter.cpp
@@ -17,6 +17,7 @@
 #include <Ice/ObjectAdapter.h>
 #include <Ice/Router.h>
 #include <Ice/ServantLocator.h>
+#include <Ice/Logger.h>
 
 #include <pythread.h>
 
@@ -177,7 +178,11 @@ IcePy::ServantLocatorWrapper::locate(const Ice::Current& current, Ice::LocalObje
     {
         if(PyTuple_GET_SIZE(res.get()) > 2)
         {
-            PyErr_WarnEx(PyExc_RuntimeWarning, STRCAST("invalid return value for ServantLocator::locate"), 1);
+            const Ice::CommunicatorPtr com = current.adapter->getCommunicator();
+            if(com->getProperties()->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0)
+            {
+                com->getLogger()->warning("invalid return value for ServantLocator::locate");
+            }
             return 0;
         }
         servantObj = PyTuple_GET_ITEM(res.get(), 0);
@@ -196,7 +201,11 @@ IcePy::ServantLocatorWrapper::locate(const Ice::Current& current, Ice::LocalObje
     //
     if(!PyObject_IsInstance(servantObj, _objectType))
     {
-        PyErr_WarnEx(PyExc_RuntimeWarning, STRCAST("return value of ServantLocator::locate is not an Ice object"), 1);
+        const Ice::CommunicatorPtr com = current.adapter->getCommunicator();
+        if(com->getProperties()->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0)
+        {
+            com->getLogger()->warning("return value of ServantLocator::locate is not an Ice object");
+        }
         return 0;
     }
 
diff --git a/python/modules/IcePy/Operation.cpp b/python/modules/IcePy/Operation.cpp
index d343d244c2a..0570640aca4 100644
--- a/python/modules/IcePy/Operation.cpp
+++ b/python/modules/IcePy/Operation.cpp
@@ -1371,9 +1371,7 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result)
     {
         ostringstream ostr;
         ostr << "operation `" << fixIdent(name) << "' should return a tuple of length " << numResults;
-        string str = ostr.str();
-        PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1);
-        throw Ice::MarshalException(__FILE__, __LINE__);
+        throw Ice::MarshalException(__FILE__, __LINE__, ostr.str());
     }
 
     //
@@ -1407,12 +1405,18 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result)
         PyObject* arg = PyTuple_GET_ITEM(t.get(), info->pos);
         if((!info->optional || arg != Unset) && !info->type->validate(arg))
         {
-            // TODO: Provide the parameter name instead?
-            ostringstream ostr;
-            ostr << "invalid value for out argument " << (info->pos + 1) << " in operation `" << dispatchName << "'";
-            string str = ostr.str();
-            PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1);
-            throw Ice::MarshalException(__FILE__, __LINE__);
+            try
+            {
+                PyException().raise();
+            }
+            catch(const Ice::UnknownException& ex)
+            {
+                // TODO: Provide the parameter name instead?
+                ostringstream ostr;
+                ostr << "invalid value for out argument " << (info->pos + 1) << " in operation `" << dispatchName;
+                ostr << "':\n" << ex.unknown;
+                throw Ice::MarshalException(__FILE__, __LINE__, ostr.str());
+            }
         }
     }
     if(returnType)
@@ -1420,11 +1424,16 @@ Operation::marshalResult(Ice::OutputStream& os, PyObject* result)
         PyObject* res = PyTuple_GET_ITEM(t.get(), 0);
         if((!returnType->optional || res != Unset) && !returnType->type->validate(res))
         {
-            ostringstream ostr;
-            ostr << "invalid return value for operation `" << dispatchName << "'";
-            string str = ostr.str();
-            PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1);
-            throw Ice::MarshalException(__FILE__, __LINE__);
+            try
+            {
+                PyException().raise();
+            }
+            catch(const Ice::UnknownException& ex)
+            {
+                ostringstream ostr;
+                ostr << "invalid return value for operation `" << dispatchName << "':\n" << ex.unknown;
+                throw Ice::MarshalException(__FILE__, __LINE__, ostr.str());
+            }
         }
     }
 
@@ -3777,7 +3786,6 @@ Upcall::dispatchImpl(PyObject* servant, const string& dispatchName, PyObject* ar
         ostr << "servant for identity " << communicator->identityToString(current.id)
              << " does not define operation `" << dispatchName << "'";
         string str = ostr.str();
-        PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1);
         Ice::UnknownException ex(__FILE__, __LINE__);
         ex.unknown = str;
         throw ex;
@@ -3793,7 +3801,6 @@ Upcall::dispatchImpl(PyObject* servant, const string& dispatchName, PyObject* ar
         ostr << "_iceDispatch method not found for identity " << communicator->identityToString(current.id)
              << " and operation `" << dispatchName << "'";
         string str = ostr.str();
-        PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1);
         Ice::UnknownException ex(__FILE__, __LINE__);
         ex.unknown = str;
         throw ex;
@@ -4108,9 +4115,7 @@ IcePy::BlobjectUpcall::response(PyObject* result)
         //
         if(!PyTuple_Check(result) || PyTuple_GET_SIZE(result) != 2)
         {
-            string str = "operation `ice_invoke' should return a tuple of length 2";
-            PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1);
-            throw Ice::MarshalException(__FILE__, __LINE__);
+            throw Ice::MarshalException(__FILE__, __LINE__, "operation `ice_invoke' should return a tuple of length 2");
         }
 
         PyObject* arg = PyTuple_GET_ITEM(result, 0);
@@ -4121,11 +4126,7 @@ IcePy::BlobjectUpcall::response(PyObject* result)
 #if PY_VERSION_HEX >= 0x03000000
         if(!PyBytes_Check(arg))
         {
-            ostringstream ostr;
-            ostr << "invalid return value for operation `ice_invoke'";
-            string str = ostr.str();
-            PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1);
-            throw Ice::MarshalException(__FILE__, __LINE__);
+            throw Ice::MarshalException(__FILE__, __LINE__, "invalid return value for operation `ice_invoke'");
         }
 
         Py_ssize_t sz = PyBytes_GET_SIZE(arg);
@@ -4139,11 +4140,7 @@ IcePy::BlobjectUpcall::response(PyObject* result)
 #else
         if(!PyBuffer_Check(arg))
         {
-            ostringstream ostr;
-            ostr << "invalid return value for operation `ice_invoke'";
-            string str = ostr.str();
-            PyErr_WarnEx(PyExc_RuntimeWarning, const_cast<char*>(str.c_str()), 1);
-            throw Ice::MarshalException(__FILE__, __LINE__);
+            throw Ice::MarshalException(__FILE__, __LINE__, "invalid return value for operation `ice_invoke'");
         }
 
         char* charBuf = 0;