Improve dbm/gdbm error messages

Improve dbm/gdbm error messages

Author: furkanonder

Lib/shelve.py

@@ -114,8 +114,8 @@ def _validate_serialized_value(serialized_value, original_value):
                 invalid_type = "None"
             else:
                 invalid_type = type(serialized_value).__name__
-            msg = (f"Serializer returned {invalid_type} for value "
-                   f"{original_value!r} But database values must be "
+            msg = (f"Serializer returned {serialized_value!r} for value "
+                   f"{original_value!r}, but database values must be "
                    f"bytes or str, not {invalid_type}")
             raise ShelveError(msg)
 

Lib/test/test_dbm_gnu.py

@@ -217,6 +217,25 @@ def test_localized_error(self):
             create_empty_file(os.path.join(d, 'test'))
             self.assertRaises(gdbm.error, gdbm.open, filename, 'r')
 
+    def test_type_errors(self):
+        self.g = gdbm.open(filename, 'c')
+        with self.assertRaisesRegex(
+            TypeError, "^a bytes-like object is required, not 'int'$",
+        ):
+            self.g[123]
+        with self.assertRaisesRegex(
+            TypeError, "^gdbm key must be bytes or str, not 'int'$",
+        ):
+            123 in self.g
+        with self.assertRaisesRegex(
+            TypeError, "^gdbm key must be bytes or str, not 'NoneType'$",
+        ):
+            self.g[None] = 123
+        with self.assertRaisesRegex(
+            TypeError, "^gdbm value must be bytes or str, not 'int'$",
+        ):
+            self.g['foo'] = 123
+
 
 if __name__ == '__main__':
     unittest.main()

Lib/test/test_dbm_ndbm.py

@@ -160,6 +160,25 @@ def test_clear(self):
                 self.assertNotIn(k, db)
             self.assertEqual(len(db), 0)
 
+    def test_type_errors(self):
+        with dbm.ndbm.open(self.filename, 'c') as db:
+            with self.assertRaisesRegex(
+                TypeError, "^a bytes-like object is required, not 'int'$",
+            ):
+                db[123]
+            with self.assertRaisesRegex(
+                TypeError, "^dbm key must be bytes or str, not 'int'$",
+            ):
+                123 in db
+            with self.assertRaisesRegex(
+                TypeError, "^dbm key must be bytes or str, not 'NoneType'$",
+            ):
+                db[None] = 123
+            with self.assertRaisesRegex(
+                TypeError, "^dbm value must be bytes or str, not 'int'$",
+            ):
+                db['foo'] = 123
+
 
 if __name__ == '__main__':
     unittest.main()

Lib/test/test_shelve.py

@@ -468,16 +468,16 @@ def deserializer(data):
                              deserializer=deserializer) as s:
                 with self.assertRaises(shelve.ShelveError) as cm:
                     s["key"] = "value"
-                self.assertEqual("Serializer returned None for value 'value' "
-                                 "But database values must be bytes or str, not None",
+                self.assertEqual("Serializer returned None for value 'value', but "
+                                 "database values must be bytes or str, not None",
                                  f"{cm.exception}")
 
             with shelve.open(self.fname, serializer=int_serializer,
                              deserializer=deserializer,) as s:
                 with self.assertRaises(shelve.ShelveError) as cm:
                     s["key"] = "value"
-                self.assertEqual("Serializer returned int for value 'value' "
-                                 "But database values must be bytes or str, not int",
+                self.assertEqual("Serializer returned 3 for value 'value', but "
+                                 "database values must be bytes or str, not int",
                                  f"{cm.exception}")
 
     def test_shelve_type_compatibility(self):

Modules/_dbmmodule.c

@@ -232,8 +232,8 @@ dbm_ass_sub_lock_held(PyObject *self, PyObject *v, PyObject *w)
 
     if ( !PyArg_Parse(v, "s#", &krec.dptr, &tmp_size) ) {
         PyErr_Format(PyExc_TypeError,
-                     "dbm key returned %.100s for value %R But database keys must be bytes or str, not %.100s",
-                     Py_TYPE(v)->tp_name, v, Py_TYPE(v)->tp_name);
+                     "dbm key must be bytes or str, not '%T'",
+                     v);
         return -1;
     }
     _dbm_state *state = PyType_GetModuleState(Py_TYPE(dp));
@@ -260,8 +260,8 @@ dbm_ass_sub_lock_held(PyObject *self, PyObject *v, PyObject *w)
     } else {
         if ( !PyArg_Parse(w, "s#", &drec.dptr, &tmp_size) ) {
             PyErr_Format(PyExc_TypeError,
-                         "dbm value returned %.100s for value %R But database values must be bytes or str, not %.100s",
-                         Py_TYPE(w)->tp_name, w, Py_TYPE(w)->tp_name);
+                         "dbm value must be bytes or str, not '%T'",
+                         w);
             return -1;
         }
         drec.dsize = tmp_size;
@@ -371,8 +371,7 @@ dbm_contains_lock_held(PyObject *self, PyObject *arg)
     }
     else if (!PyBytes_Check(arg)) {
         PyErr_Format(PyExc_TypeError,
-                     "dbm key must be bytes or string, not %.100s",
-                     Py_TYPE(arg)->tp_name);
+                     "dbm key must be bytes or str, not '%T'", arg);
         return -1;
     }
     else {

Modules/_gdbmmodule.c

@@ -237,12 +237,14 @@ gdbm_bool(PyObject *op)
 // This function is needed to support PY_SSIZE_T_CLEAN.
 // Return 1 on success, same to PyArg_Parse().
 static int
-parse_datum(PyObject *o, datum *d, const char *failmsg)
+parse_datum(PyObject *o, datum *d, const char *item_name)
 {
     Py_ssize_t size;
     if (!PyArg_Parse(o, "s#", &d->dptr, &size)) {
-        if (failmsg != NULL) {
-            PyErr_Format(PyExc_TypeError, failmsg, Py_TYPE(o)->tp_name, o, Py_TYPE(o)->tp_name);
+        if (item_name) {
+            PyErr_Format(PyExc_TypeError,
+                         "gdbm %s must be bytes or str, not '%T'",
+                         item_name, o);
         }
         return 0;
     }
@@ -318,12 +320,10 @@ static int
 gdbm_ass_sub_lock_held(PyObject *op, PyObject *v, PyObject *w)
 {
     datum krec, drec;
-    const char *key_failmsg = "dbm key returned %.100s for value %R But database keys must be bytes or str, not %.100s";
-    const char *value_failmsg = "dbm value returned %.100s for value %R But database keys must be bytes or str, not %.100s";
     gdbmobject *dp = _gdbmobject_CAST(op);
     _gdbm_state *state = PyType_GetModuleState(Py_TYPE(dp));
 
-    if (!parse_datum(v, &krec, key_failmsg)) {
+    if (!parse_datum(v, &krec, "key")) {
         return -1;
     }
     if (dp->di_dbm == NULL) {
@@ -344,7 +344,7 @@ gdbm_ass_sub_lock_held(PyObject *op, PyObject *v, PyObject *w)
         }
     }
     else {
-        if (!parse_datum(w, &drec, value_failmsg)) {
+        if (!parse_datum(w, &drec, "value")) {
             return -1;
         }
         errno = 0;
@@ -492,8 +492,7 @@ gdbm_contains_lock_held(PyObject *self, PyObject *arg)
     }
     else if (!PyBytes_Check(arg)) {
         PyErr_Format(PyExc_TypeError,
-                     "gdbm key must be bytes or string, not %.100s",
-                     Py_TYPE(arg)->tp_name);
+                     "gdbm key must be bytes or str, not '%T'", arg);
         return -1;
     }
     else {