Stop catching POSKeyError.

CHANGES.rst

@@ -6,6 +6,10 @@
 1.0.0a2 (unreleased)
 ====================
 
+- The low levels of externalization no longer catch and hide
+  POSKeyError. This indicates a problem with the database. See
+  https://github.com/NextThought/nti.externalization/issues/60
+
 - Remove support for ``object_hook`` in
   ``update_from_external_object``. See
   https://github.com/NextThought/nti.externalization/issues/29.

src/nti/externalization/externalization/fields.py

@@ -13,7 +13,6 @@
 from zope.security.management import system_user
 from zope.security.interfaces import IPrincipal
 
-from ZODB.POSException import POSKeyError
 
 from nti.externalization._base_interfaces import get_standard_external_fields
 
@@ -43,32 +42,26 @@ def choose_field(result, self, ext_name,
     # to document this and probably move it to a different module, or
     # provide a cleaner simpler replacement.
     for x in fields:
-        try:
-            value = getattr(self, x)
-        except AttributeError:
-            continue
-        except POSKeyError:
-            logger.exception("Could not get attribute %s for object %s",
-                             x, self)
+        value = getattr(self, x, None)
+        if value is None:
             continue
 
+        # If the creator is the system user, catch it here
+        # XXX: Document this behaviour.
+        if ext_name == StandardExternalFields.CREATOR:
+            if is_system_user(value):
+                value = SYSTEM_USER_NAME
+            else:
+                # This is a likely recursion point, we want to be
+                # sure we don't do that.
+                value = text_type(value)
+            result[ext_name] = value
+            return value
+        if converter is not None:
+            value = converter(value)
         if value is not None:
-            # If the creator is the system user, catch it here
-            # XXX: Document this behaviour.
-            if ext_name == StandardExternalFields.CREATOR:
-                if is_system_user(value):
-                    value = SYSTEM_USER_NAME
-                else:
-                    # This is a likely recursion point, we want to be
-                    # sure we don't do that.
-                    value = text_type(value)
-                result[ext_name] = value
-                return value
-            if converter is not None:
-                value = converter(value)
-            if value is not None:
-                result[ext_name] = value
-                return value
+            result[ext_name] = value
+            return value
 
     # Nothing. Can we adapt it?
     if sup_iface is not None and sup_fields:

src/nti/externalization/tests/test_externalization.py

@@ -253,15 +253,15 @@ def test_isSyntheticKey(self):
         assert_that(isSyntheticKey('OID'), is_true())
         assert_that(isSyntheticKey('key'), is_false())
 
-    def test_choose_field_POSKeyError_ignored(self):
+    def test_choose_field_POSKeyError_not_ignored(self):
         from ZODB.POSException import POSKeyError
         class Raises(object):
             def __getattr__(self, name):
                 raise POSKeyError(name)
 
-        assert_that(choose_field({}, Raises(), u'ext_name',
-                                 fields=('a', 'b')),
-                    is_(none()))
+        with self.assertRaises(POSKeyError):
+            choose_field({}, Raises(), u'ext_name',
+                         fields=('a', 'b'))
 
     def test_choose_field_system_user(self):
         from nti.externalization.externalization import SYSTEM_USER_NAME