PYTHON-2287 Improve error message for invalid boolean option (#1236)

Author: sleepyStick

bson/codec_options.py

@@ -397,7 +397,7 @@ def __new__(
                     "subclass of collections.abc.MutableMapping"
                 )
             if not isinstance(tz_aware, bool):
-                raise TypeError("tz_aware must be True or False")
+                raise TypeError(f"tz_aware must be True or False, was: tz_aware={tz_aware}")
             if uuid_representation not in ALL_UUID_REPRESENTATIONS:
                 raise ValueError(
                     "uuid_representation must be a value from bson.binary.UuidRepresentation"

doc/contributors.rst

@@ -96,3 +96,4 @@ The following is a list of people who have contributed to
 - Jean-Christophe Fillion-Robin (jcfr)
 - Sean Cheah (thalassemia)
 - Dainis Gorbunovs (DainisGorbunovs)
+- Iris Ho (sleepyStick)

pymongo/common.py

@@ -50,7 +50,7 @@
 from pymongo.read_concern import ReadConcern
 from pymongo.read_preferences import _MONGOS_MODES, _ServerMode
 from pymongo.server_api import ServerApi
-from pymongo.write_concern import DEFAULT_WRITE_CONCERN, WriteConcern
+from pymongo.write_concern import DEFAULT_WRITE_CONCERN, WriteConcern, validate_boolean
 
 ORDERED_TYPES: Sequence[Type] = (SON, OrderedDict)
 
@@ -170,13 +170,6 @@ def raise_config_error(key: str, dummy: Any) -> NoReturn:
 }
 
 
-def validate_boolean(option: str, value: Any) -> bool:
-    """Validates that 'value' is True or False."""
-    if isinstance(value, bool):
-        return value
-    raise TypeError(f"{option} must be True or False")
-
-
 def validate_boolean_or_string(option: str, value: Any) -> bool:
     """Validates that value is True, False, 'true', or 'false'."""
     if isinstance(value, str):

pymongo/pyopenssl_context.py

@@ -37,6 +37,7 @@
 from pymongo.ocsp_support import _load_trusted_ca_certs, _ocsp_callback
 from pymongo.socket_checker import SocketChecker as _SocketChecker
 from pymongo.socket_checker import _errno_from_exception
+from pymongo.write_concern import validate_boolean
 
 try:
     import certifi
@@ -228,8 +229,7 @@ def __get_check_hostname(self):
         return self._check_hostname
 
     def __set_check_hostname(self, value):
-        if not isinstance(value, bool):
-            raise TypeError("check_hostname must be True or False")
+        validate_boolean("check_hostname", value)
         self._check_hostname = value
 
     check_hostname = property(__get_check_hostname, __set_check_hostname)
@@ -238,8 +238,7 @@ def __get_check_ocsp_endpoint(self):
         return self._callback_data.check_ocsp_endpoint
 
     def __set_check_ocsp_endpoint(self, value):
-        if not isinstance(value, bool):
-            raise TypeError("check_ocsp must be True or False")
+        validate_boolean("check_ocsp", value)
         self._callback_data.check_ocsp_endpoint = value
 
     check_ocsp_endpoint = property(__get_check_ocsp_endpoint, __set_check_ocsp_endpoint)

pymongo/write_concern.py

@@ -19,6 +19,14 @@
 from pymongo.errors import ConfigurationError
 
 
+# Moved here to avoid a circular import.
+def validate_boolean(option: str, value: Any) -> bool:
+    """Validates that 'value' is True or False."""
+    if isinstance(value, bool):
+        return value
+    raise TypeError(f"{option} must be True or False, was: {option}={value}")
+
+
 class WriteConcern:
     """WriteConcern
 
@@ -65,13 +73,11 @@ def __init__(
             self.__document["wtimeout"] = wtimeout
 
         if j is not None:
-            if not isinstance(j, bool):
-                raise TypeError("j must be True or False")
+            validate_boolean("j", j)
             self.__document["j"] = j
 
         if fsync is not None:
-            if not isinstance(fsync, bool):
-                raise TypeError("fsync must be True or False")
+            validate_boolean("fsync", fsync)
             if j and fsync:
                 raise ConfigurationError("Can't set both j and fsync at the same time")
             self.__document["fsync"] = fsync

test/test_common.py

@@ -165,6 +165,13 @@ def test_mongo_client(self):
         self.assertEqual(direct, direct2)
         self.assertFalse(direct != direct2)
 
+    def test_validate_boolean(self):
+        self.db.test.update_one({}, {"$set": {"total": 1}}, upsert=True)
+        with self.assertRaisesRegex(
+            TypeError, "upsert must be True or False, was: upsert={'upsert': True}"
+        ):
+            self.db.test.update_one({}, {"$set": {"total": 1}}, {"upsert": True})  # type: ignore
+
 
 if __name__ == "__main__":
     unittest.main()