PYTHON-1664 Include type in InvalidDocument error

Author: ShaneHarvey

bson/__init__.py

@@ -812,7 +812,7 @@ def _name_value_to_bson(name, value, check_keys, opts,
             in_fallback_call=True)
 
     raise InvalidDocument(
-        "cannot convert value of type %s to bson" % type(value))
+        "cannot encode object: %r, of type: %r" % (value, type(value)))
 
 
 def _element_to_bson(key, value, check_keys, opts):

bson/_cbsonmodule.c

@@ -593,37 +593,53 @@ _fix_java(const char* in, char* out) {
 
 static void
 _set_cannot_encode(PyObject* value) {
+    PyObject* type = NULL;
     PyObject* InvalidDocument = _error("InvalidDocument");
-    if (InvalidDocument) {
-        PyObject* repr = PyObject_Repr(value);
-        if (repr) {
-#if PY_MAJOR_VERSION >= 3
-            PyObject* errmsg = PyUnicode_FromString("Cannot encode object: ");
-#else
-            PyObject* errmsg = PyString_FromString("Cannot encode object: ");
-#endif
-            if (errmsg) {
+    if (InvalidDocument == NULL) {
+        goto error;
+    }
+
+    type = PyObject_Type(value);
+    if (type == NULL) {
+        goto error;
+    }
 #if PY_MAJOR_VERSION >= 3
-                PyObject* error = PyUnicode_Concat(errmsg, repr);
-                if (error) {
-                    PyErr_SetObject(InvalidDocument, error);
-                    Py_DECREF(error);
-                }
-                Py_DECREF(errmsg);
-                Py_DECREF(repr);
+    PyErr_Format(InvalidDocument, "cannot encode object: %R, of type: %R",
+                 value, type);
 #else
-                PyString_ConcatAndDel(&errmsg, repr);
-                if (errmsg) {
-                    PyErr_SetObject(InvalidDocument, errmsg);
-                    Py_DECREF(errmsg);
-                }
-#endif
-            } else {
-                Py_DECREF(repr);
-            }
+     else {
+        PyObject* value_repr = NULL;
+        PyObject* type_repr = NULL;
+        char* value_str = NULL;
+        char* type_str = NULL;
+
+        value_repr = PyObject_Repr(value);
+        if (value_repr == NULL) {
+            goto py2error;
         }
-        Py_DECREF(InvalidDocument);
+        value_str = PyString_AsString(value_repr);
+        if (value_str == NULL) {
+            goto py2error;
+        }
+        type_repr = PyObject_Repr(type);
+        if (type_repr == NULL) {
+            goto py2error;
+        }
+        type_str = PyString_AsString(type_repr);
+        if (type_str == NULL) {
+            goto py2error;
+        }
+
+        PyErr_Format(InvalidDocument, "cannot encode object: %s, of type: %s",
+                     value_str, type_str);
+py2error:
+        Py_XDECREF(type_repr);
+        Py_XDECREF(value_repr);
     }
+#endif
+error:
+    Py_XDECREF(type);
+    Py_XDECREF(InvalidDocument);
 }
 
 /*

doc/examples/custom_type.rst

@@ -39,7 +39,7 @@ to save an instance of ``Decimal`` with PyMongo, results in an
   >>> db.test.insert_one({'num': num})
   Traceback (most recent call last):
   ...
-  bson.errors.InvalidDocument: Cannot encode object: <__main__.Decimal object at ...>
+  bson.errors.InvalidDocument: cannot encode object: Decimal('45.321'), of type: <class 'decimal.Decimal'>
 
 
 .. _custom-type-type-codec:
@@ -179,7 +179,7 @@ codec for it, we get an error:
   >>> collection.insert_one({'num': DecimalInt("45.321")})
   Traceback (most recent call last):
   ...
-  bson.errors.InvalidDocument: Cannot encode object: Decimal('45.321')
+  bson.errors.InvalidDocument: cannot encode object: Decimal('45.321'), of type: <class 'decimal.Decimal'>
 
 In order to proceed further, we must define a type codec for ``DecimalInt``.
 This is trivial to do since the same transformation as the one used for

test/test_bson.py

@@ -923,6 +923,20 @@ def target(i):
         for t in threads:
             self.assertIsNone(t.exc)
 
+    def test_raise_invalid_document(self):
+        class Wrapper(object):
+            def __init__(self, val):
+                self.val = val
+
+            def __repr__(self):
+                return repr(self.val)
+
+        self.assertEqual('1', repr(Wrapper(1)))
+        with self.assertRaisesRegex(
+                InvalidDocument,
+                "cannot encode object: 1, of type: " + repr(Wrapper)):
+            BSON.encode({'t': Wrapper(1)})
+
 
 class TestCodecOptions(unittest.TestCase):
     def test_document_class(self):