HPCC-18723 Poor error message when Python code does not return a value

Signed-off-by: Richard Chapman <rchapman@hpccsystems.com>

plugins/py3embed/py3embed.cpp

@@ -552,82 +552,87 @@ static void typeError(const char *expected, const RtlFieldInfo *field)
     VStringBuffer msg("pyembed: type mismatch - %s expected", expected);
     if (field)
         msg.appendf(" for field %s", str(field->name));
+    else
+        msg.appendf(" for return value");
     rtlFail(0, msg.str());
 }
 
 static bool getBooleanResult(const RtlFieldInfo *field, PyObject *obj)
 {
-    assertex(obj && obj != Py_None);
-    if (!PyBool_Check(obj))
-        typeError("boolean", field);
-    return obj == Py_True;
+    if (obj && obj != Py_None)
+    {
+        if (PyBool_Check(obj))
+            return obj == Py_True;
+    }
+    typeError("boolean", field);
 }
 
 static void getDataResult(const RtlFieldInfo *field, PyObject *obj, size32_t &chars, void * &result)
 {
-    assertex(obj && obj != Py_None);
-    if (!PyByteArray_Check(obj))
+    if (obj && obj != Py_None && PyByteArray_Check(obj))
+        rtlStrToDataX(chars, result, PyByteArray_Size(obj), PyByteArray_AsString(obj));
+    else
         typeError("bytearray", field);
-    rtlStrToDataX(chars, result, PyByteArray_Size(obj), PyByteArray_AsString(obj));
 }
 
 static double getRealResult(const RtlFieldInfo *field, PyObject *obj)
 {
-    assertex(obj && obj != Py_None);
-    if (!PyFloat_Check(obj))
-        typeError("real", field);
-    return PyFloat_AsDouble(obj);
+    if (obj && obj != Py_None)
+    {
+        if (PyFloat_Check(obj))
+            return PyFloat_AsDouble(obj);
+    }
+    typeError("real", field);
 }
 
 static __int64 getSignedResult(const RtlFieldInfo *field, PyObject *obj)
 {
-    assertex(obj && obj != Py_None);
-    __int64 ret;
-    if (PyLong_Check(obj))
-        ret = (__int64) PyLong_AsLongLong(obj);
-    else
-        typeError("integer", field);
-    return ret;
+    if (obj && obj != Py_None)
+    {
+        if (PyLong_Check(obj))
+            return (__int64) PyLong_AsLongLong(obj);
+    }
+    typeError("integer", field);
 }
 
 static unsigned __int64 getUnsignedResult(const RtlFieldInfo *field, PyObject *obj)
 {
-    assertex(obj && obj != Py_None);
-    unsigned __int64 ret;
-    if (PyLong_Check(obj))
-        ret =  (unsigned __int64) PyLong_AsUnsignedLongLong(obj);
-    else
-        typeError("integer", field);
-    return ret;
+    if (obj && obj != Py_None)
+    {
+        if (PyLong_Check(obj))
+            return (unsigned __int64) PyLong_AsUnsignedLongLong(obj);
+    }
+    typeError("integer", field);
 }
 
 static void getStringResult(const RtlFieldInfo *field, PyObject *obj, size32_t &chars, char * &result)
 {
-    assertex(obj && obj != Py_None);
-    if (PyUnicode_Check(obj))
+    if (obj && obj != Py_None)
     {
-        OwnedPyObject temp_bytes = PyUnicode_AsEncodedString(obj, ASCII_LIKE_CODEPAGE, "ignore");
-        checkPythonError();
-        const char * text =  PyBytes_AsString(temp_bytes);
-        checkPythonError();
-        size_t lenBytes = PyBytes_Size(temp_bytes);
-        rtlStrToStrX(chars, result, lenBytes, text);
-    }
-    else if (PyBytes_Check(obj))
-    {
-        const char * text = PyBytes_AsString(obj);
-        checkPythonError();
-        size_t lenBytes = PyBytes_Size(obj);
-        rtlStrToStrX(chars, result, lenBytes, text);
+        if (PyUnicode_Check(obj))
+        {
+            OwnedPyObject temp_bytes = PyUnicode_AsEncodedString(obj, ASCII_LIKE_CODEPAGE, "ignore");
+            checkPythonError();
+            const char * text =  PyBytes_AsString(temp_bytes);
+            checkPythonError();
+            size_t lenBytes = PyBytes_Size(temp_bytes);
+            rtlStrToStrX(chars, result, lenBytes, text);
+        }
+        else if (PyBytes_Check(obj))
+        {
+            const char * text = PyBytes_AsString(obj);
+            checkPythonError();
+            size_t lenBytes = PyBytes_Size(obj);
+            rtlStrToStrX(chars, result, lenBytes, text);
+        }
+        return;
     }
-    else
-        typeError("string", field);
+    typeError("string", field);
 }
 
 static void getUTF8Result(const RtlFieldInfo *field, PyObject *obj, size32_t &chars, char * &result)
 {
-    assertex(obj && obj != Py_None);
-    if (PyUnicode_Check(obj))
+    if (obj && obj != Py_None && PyUnicode_Check(obj))
     {
         Py_ssize_t lenBytes;
         const char *text = PyUnicode_AsUTF8AndSize(obj, &lenBytes);
@@ -642,8 +647,7 @@ static void getUTF8Result(const RtlFieldInfo *field, PyObject *obj, size32_t &ch
 static void getSetResult(PyObject *obj, bool & isAllResult, size32_t & resultBytes, void * & result, int elemType, size32_t elemSize)
 {
     // MORE - should probably recode to use the getResultDataset mechanism
-    assertex(obj && obj != Py_None);
-    if (!PyList_Check(obj) && !PySet_Check(obj))
+    if (!obj || obj == Py_None || (!PyList_Check(obj) && !PySet_Check(obj)))
         rtlFail(0, "pyembed: type mismatch - list or set expected");
     rtlRowBuilder out;
     size32_t outBytes = 0;
@@ -784,8 +788,7 @@ static void getSetResult(PyObject *obj, bool & isAllResult, size32_t & resultByt
 
 static void getUnicodeResult(const RtlFieldInfo *field, PyObject *obj, size32_t &chars, UChar * &result)
 {
-    assertex(obj && obj != Py_None);
-    if (PyUnicode_Check(obj))
+    if (obj && obj != Py_None && PyUnicode_Check(obj))
     {
         Py_ssize_t lenBytes;
         const char *text = PyUnicode_AsUTF8AndSize(obj, &lenBytes);
@@ -858,8 +861,7 @@ public:
     {
         nextField(field);
         isAll = false;  // No concept of an 'all' set in Python
-        assertex(elem && elem != Py_None);
-        if (!PyList_Check(elem) && !PySet_Check(elem))
+        if (!elem || elem == Py_None || (!PyList_Check(elem) && !PySet_Check(elem)))
             typeError("list or set", field);
         push();
     }

plugins/pyembed/pyembed.cpp

@@ -541,63 +541,66 @@ static void typeError(const char *expected, const RtlFieldInfo *field)
     VStringBuffer msg("pyembed: type mismatch - %s expected", expected);
     if (field)
         msg.appendf(" for field %s", str(field->name));
+    else
+        msg.appendf(" for return value");
     rtlFail(0, msg.str());
 }
 
 static bool getBooleanResult(const RtlFieldInfo *field, PyObject *obj)
 {
-    assertex(obj && obj != Py_None);
-    if (!PyBool_Check(obj))
-        typeError("boolean", field);
-    return obj == Py_True;
+    if (obj && obj != Py_None)
+    {
+        if (PyBool_Check(obj))
+            return obj == Py_True;
+    }
+    typeError("boolean", field);
 }
 
 static void getDataResult(const RtlFieldInfo *field, PyObject *obj, size32_t &chars, void * &result)
 {
-    assertex(obj && obj != Py_None);
-    if (!PyByteArray_Check(obj))
+    if (obj && obj != Py_None && PyByteArray_Check(obj))
+        rtlStrToDataX(chars, result, PyByteArray_Size(obj), PyByteArray_AsString(obj));
+    else
         typeError("bytearray", field);
-    rtlStrToDataX(chars, result, PyByteArray_Size(obj), PyByteArray_AsString(obj));
 }
 
 static double getRealResult(const RtlFieldInfo *field, PyObject *obj)
 {
-    assertex(obj && obj != Py_None);
-    if (!PyFloat_Check(obj))
-        typeError("real", field);
-    return PyFloat_AsDouble(obj);
+    if (obj && obj != Py_None)
+    {
+        if (PyFloat_Check(obj))
+            return PyFloat_AsDouble(obj);
+    }
+    typeError("real", field);
 }
 
 static __int64 getSignedResult(const RtlFieldInfo *field, PyObject *obj)
 {
-    assertex(obj && obj != Py_None);
-    __int64 ret;
-    if (PyInt_Check(obj))
-        ret = PyInt_AsUnsignedLongLongMask(obj);
-    else if (PyLong_Check(obj))
-        ret = (__int64) PyLong_AsLongLong(obj);
-    else
-        typeError("integer", field);
-    return ret;
+    if (obj && obj != Py_None)
+    {
+        if (PyInt_Check(obj))
+            return PyInt_AsUnsignedLongLongMask(obj);
+         else if (PyLong_Check(obj))
+            return (__int64) PyLong_AsLongLong(obj);
+    }
+    typeError("integer", field);
 }
 
 static unsigned __int64 getUnsignedResult(const RtlFieldInfo *field, PyObject *obj)
 {
-    assertex(obj && obj != Py_None);
-    unsigned __int64 ret;
-    if (PyInt_Check(obj))
-        ret = PyInt_AsUnsignedLongLongMask(obj);
-    else if (PyLong_Check(obj))
-        ret =  (unsigned __int64) PyLong_AsUnsignedLongLong(obj);
-    else
-        typeError("integer", field);
-    return ret;
+    if (obj && obj != Py_None)
+    {
+        if (PyInt_Check(obj))
+            return PyInt_AsUnsignedLongLongMask(obj);
+        else if (PyLong_Check(obj))
+            return (unsigned __int64) PyLong_AsUnsignedLongLong(obj);
+    }
+    typeError("integer", field);
 }
 
 static void getStringResult(const RtlFieldInfo *field, PyObject *obj, size32_t &chars, char * &result)
 {
-    assertex(obj && obj != Py_None);
-    if (PyString_Check(obj))
+    if (obj && obj != Py_None && PyString_Check(obj))
     {
         const char * text =  PyString_AsString(obj);
         checkPythonError();
@@ -610,8 +613,7 @@ static void getStringResult(const RtlFieldInfo *field, PyObject *obj, size32_t &
 
 static void getUTF8Result(const RtlFieldInfo *field, PyObject *obj, size32_t &chars, char * &result)
 {
-    assertex(obj && obj != Py_None);
-    if (PyUnicode_Check(obj))
+    if (obj && obj != Py_None && PyUnicode_Check(obj))
     {
         OwnedPyObject utf8 = PyUnicode_AsUTF8String(obj);
         checkPythonError();
@@ -628,8 +630,7 @@ static void getUTF8Result(const RtlFieldInfo *field, PyObject *obj, size32_t &ch
 static void getSetResult(PyObject *obj, bool & isAllResult, size32_t & resultBytes, void * & result, int elemType, size32_t elemSize)
 {
     // MORE - should probably recode to use the getResultDataset mechanism
-    assertex(obj && obj != Py_None);
-    if (!PyList_Check(obj) && !PySet_Check(obj))
+    if (!obj || obj == Py_None || (!PyList_Check(obj) && !PySet_Check(obj)))
         rtlFail(0, "pyembed: type mismatch - list or set expected");
     rtlRowBuilder out;
     size32_t outBytes = 0;
@@ -770,8 +771,7 @@ static void getSetResult(PyObject *obj, bool & isAllResult, size32_t & resultByt
 
 static void getUnicodeResult(const RtlFieldInfo *field, PyObject *obj, size32_t &chars, UChar * &result)
 {
-    assertex(obj && obj != Py_None);
-    if (PyUnicode_Check(obj))
+    if (obj && obj != Py_None && PyUnicode_Check(obj))
     {
         OwnedPyObject utf8 = PyUnicode_AsUTF8String(obj);
         checkPythonError();
@@ -846,8 +846,7 @@ public:
     {
         nextField(field);
         isAll = false;  // No concept of an 'all' set in Python
-        assertex(elem && elem != Py_None);
-        if (!PyList_Check(elem) && !PySet_Check(elem))
+        if (!elem || elem == Py_None || (!PyList_Check(elem) && !PySet_Check(elem)))
             typeError("list or set", field);
         push();
     }