PYCBC-1823: Fix pending-exception leak in add_* dict helpers

Changes
--------
* Check the return value of PyDict_SetItem/PyDict_SetItemString in
  add_field (all 3 overloads), add_string_field_if_not_empty, and
  add_duration_field, reporting a failure via PyErr_WriteUnraisable
  instead of leaving the exception pending on the thread
* Apply the same fix to add_bool_field and add_cpp_core_span_field,
  changing their return type from void to int to match the rest of
  the family
* Add a shared banner comment above the helper block documenting the
  return 0/-1 and self-reporting convention for all six functions

Change-Id: I786dd7421d857541a0c9adfee7c7c83797370e05
Reviewed-on: https://review.couchbase.org/c/couchbase-python-client/+/250034
Tested-by: Build Bot <build@couchbase.com>
Reviewed-by: Dimitris Christodoulou <dimitris.christodoulou@couchbase.com>

Author: thejcfactor

src/utils.hxx

@@ -228,11 +228,10 @@ extract_legacy_span_field(PyObject* dict,
   }
 }
 
-// Add a field to result dict (auto-converts C++ value to Python).
-// Returns 0 on success, -1 on failure. Most callers (generated code) don't
-// check the return value, so on failure we report the pending exception via
-// PyErr_WriteUnraisable (matches the pattern used in logger.hxx) rather than
-// leaving it set on the thread or silently dropping it.
+// Shared convention for every add_* helper below (add_field, add_bool_field,
+// add_string_field_if_not_empty, add_duration_field, add_cpp_core_span_field): return 0 on
+// success, -1 on failure, self-reported via PyErr_WriteUnraisable (matches logger.hxx) since
+// most callers (generated code) never check the return value.
 template<typename T>
 inline int
 add_field(PyObject* dict, const char* key, const T& value)
@@ -244,6 +243,9 @@ add_field(PyObject* dict, const char* key, const T& value)
   }
   int rv = PyDict_SetItemString(dict, key, pyObj);
   Py_DECREF(pyObj);
+  if (rv < 0) {
+    PyErr_WriteUnraisable(dict);
+  }
   return rv;
 }
 
@@ -258,6 +260,9 @@ add_field(PyObject* dict, PyObject* interned_key, const T& value)
   }
   int rv = PyDict_SetItem(dict, interned_key, pyObj);
   Py_DECREF(pyObj);
+  if (rv < 0) {
+    PyErr_WriteUnraisable(dict);
+  }
   return rv;
 }
 
@@ -271,13 +276,20 @@ add_field(PyObject* dict, const char* key, PyObject* value)
   }
   int rv = PyDict_SetItemString(dict, key, value);
   Py_DECREF(value);
+  if (rv < 0) {
+    PyErr_WriteUnraisable(dict);
+  }
   return rv;
 }
 
-inline void
+inline int
 add_bool_field(PyObject* dict, const char* key, bool value)
 {
-  PyDict_SetItemString(dict, key, value ? Py_True : Py_False);
+  int rv = PyDict_SetItemString(dict, key, value ? Py_True : Py_False);
+  if (rv < 0) {
+    PyErr_WriteUnraisable(dict);
+  }
+  return rv;
 }
 
 inline int
@@ -293,6 +305,9 @@ add_string_field_if_not_empty(PyObject* dict, const char* key, const std::string
   }
   int rv = PyDict_SetItemString(dict, key, pyObj);
   Py_DECREF(pyObj);
+  if (rv < 0) {
+    PyErr_WriteUnraisable(dict);
+  }
   return rv;
 }
 
@@ -307,20 +322,29 @@ add_duration_field(PyObject* dict, const char* key, const std::chrono::milliseco
   }
   int rv = PyDict_SetItemString(dict, key, pyObj);
   Py_DECREF(pyObj);
+  if (rv < 0) {
+    PyErr_WriteUnraisable(dict);
+  }
   return rv;
 }
 
-inline void
+inline int
 add_cpp_core_span_field(
   PyObject* dict,
   const char* key,
   const std::shared_ptr<couchbase::core::tracing::wrapper_sdk_span>& wrapperSpan)
 {
   PyObject* pyObj = cbpp_wrapper_span_to_py(wrapperSpan);
-  if (pyObj != nullptr) {
-    PyDict_SetItemString(dict, key, pyObj);
-    Py_DECREF(pyObj);
+  if (pyObj == nullptr) {
+    PyErr_WriteUnraisable(dict);
+    return -1;
+  }
+  int rv = PyDict_SetItemString(dict, key, pyObj);
+  Py_DECREF(pyObj);
+  if (rv < 0) {
+    PyErr_WriteUnraisable(dict);
   }
+  return rv;
 }
 
 // ======================================================================