Merge lp:~benji/lazr.restful/bug-539070 into lp:lazr.restful

[Benji York]

Bug 539070 describes a case where an error message doesn't give much of a hint as to where to start looking for a problem. This branch enhances the error message with the interface in question so the developer knows where to look.

[lp:~benji/lazr.restful/bug-539070]

157. By Benji York <benji@benji-laptop> on 2010-11-03

checkpoint

158. By Benji York on 2010-11-04

finish up reworking the error reporting

159. By Benji York on 2010-11-04

remove some dead code that snuck in

[Edwin Grubbs (edwin-grubbs)]

Hi Benji,

The changes you made look good for showing which interface attribute tries to set the schema to a class that doesn't have an IEntry adapter. I just have one comment below.

-Edwin

>=== modified file 'src/lazr/restful/tales.py'
>--- src/lazr/restful/tales.py 2010-08-24 18:57:50 +0000
>+++ src/lazr/restful/tales.py 2010-11-04 19:25:16 +0000
>@@ -29,6 +29,7 @@
> from lazr.restful import (
> EntryResource, ResourceJSONEncoder, CollectionResource,
> EntryAdapterUtility, IObjectLink, RESTUtilityBase)
>+from lazr.restful._resource import UnknownEntryAdapter
> from lazr.restful.interfaces import (
> ICollection, ICollectionField, IEntry, IJSONRequestCache,
> IReferenceChoice, IResourceDELETEOperation, IResourceGETOperation,
>@@ -540,8 +541,15 @@
> raise TypeError("Field is not of a supported type.")
> assert schema is not IObject, (
> "Null schema provided for %s" % self.field.__name__)
>- return EntryAdapterUtility.forSchemaInterface(
>- schema, get_current_web_service_request())
>+ try:
>+ return EntryAdapterUtility.forSchemaInterface(
>+ schema, get_current_web_service_request())
>+ except UnknownEntryAdapter, e:
>+ e.whence = (
>+ 'Encountered as a result of the entry interface %r, field %r.'
>+ % (self.field.interface, self.field.getName()))
>+ raise e
>+ return adapter.entry_page_type_link

Can you remove this line that can never be reached?

review: Approve (code)

[Benji York (benji)]

I removed the dead code. Thanks.

[Jonathan Lange (jml)]

Thanks so much for working on this change Benji – it promises to save me much hair-pulling in the future.

Perhaps it would also be worth adding a similar try/except/whence thingummy around WadlTopLevelEntryLinkAPI.type_link? (pasted below for convenience)

class WadlTopLevelEntryLinkAPI(RESTUtilityBase):
    """Namespace for WADL functions that operate on top-level entry links."""

    def __init__(self, entry_link):
        self.entry_link = entry_link

    def type_link(self):
        return EntryAdapterUtility.forSchemaInterface(
            self.entry_link.entry_type,
            get_current_web_service_request()).type_link

=== modified file 'src/lazr/restful/_resource.py'
--- src/lazr/restful/_resource.py	2010-10-25 16:31:09 +0000
+++ src/lazr/restful/_resource.py	2010-11-04 19:39:42 +0000
@@ -2060,6 +2060,22 @@
                            request)
 
 
+class UnknownEntryAdapter(Exception):
+    """Exception that signals no adaper could be found for an interface."""
+
+    def __init__(self, adapted_interface_name, version):
+        self.adapted_interface_name = adapted_interface_name
+        self.version = version
+        self.whence = ''
+
+    def __str__(self):
+        message = ("No IEntry adapter found for %s (web service version: %s)."
+            % (self.adapted_interface_name, self.version))
+        if self.whence:
+            message += '  ' + self.whence
+        return message
+
+
 class EntryAdapterUtility(RESTUtilityBase):
     """Useful information about an entry's presence in the web service.
 
@@ -2078,9 +2094,8 @@
             IWebServiceVersion, name=request.version)
         entry_class = getGlobalSiteManager().adapters.lookup(
             (entry_interface, request_interface), IEntry)
-        assert entry_class is not None, (
-            ("No IEntry adapter found for %s (web service version: %s)."
-             % (entry_interface.__name__, request.version)))
+        if entry_class is None:
+            raise UnknownEntryAdapter(entry_interface.__name__, request.version)
         return EntryAdapterUtility(entry_class)
 
     @classmethod

=== modified file 'src/lazr/restful/tales.py'
--- src/lazr/restful/tales.py	2010-08-24 18:57:50 +0000
+++ src/lazr/restful/tales.py	2010-11-04 19:39:42 +0000
@@ -29,6 +29,7 @@
 from lazr.restful import (
     EntryResource, ResourceJSONEncoder, CollectionResource,
     EntryAdapterUtility, IObjectLink, RESTUtilityBase)
+from lazr.restful._resource import UnknownEntryAdapter
 from lazr.restful.interfaces import (
     ICollection, ICollectionField, IEntry, IJSONRequestCache,
     IReferenceChoice, IResourceDELETEOperation, IResourceGETOperation,
@@ -540,8 +541,14 @@
             raise TypeError("Field is not of a supported type.")
         assert schema is not IObject, (
             "Null schema provided for %s" % self.field.__name__)
-        return EntryAdapterUtility.forSchemaInterface(
-            schema, get_current_web_service_request())
+        try:
+            return EntryAdapterUtility.forSchemaInterface(
+                schema, get_current_web_service_request())
+        except UnknownEntryAdapter, e:
+            e.whence = (
+                'Encountered as a result of the entry interface %r, field %r.'
+                % (self.field.interface, self.field.getName()))
+            raise e
 
 
     @property

=== modified file 'src/lazr/restful/tests/test_error.py'
--- src/lazr/restful/tests/test_error.py	2010-09-06 19:25:17 +0000
+++ src/lazr/restful/tests/test_error.py	2010-11-04 19:39:42 +0000
@@ -10,8 +10,9 @@
 
 from van.testing.layer import zcml_layer
 from zope.component import getMultiAdapter
+from zope.interface import Interface
 
-from lazr.restful._resource import ReadWriteResource
+from lazr.restful._resource import ReadWriteResource, UnknownEntryAdapter
 from lazr.restful.error import expose, ClientErrorView, SystemErrorView
 from lazr.restful.interfaces import IClientErrorView
 from lazr.restful.testing.webservice import FakeRequest
@@ -72,6 +73,36 @@
         self.assertRaises(ValueError, expose, 'x')
         self.assertRaises(ValueError, expose, RuntimeError)
 
+    def test_missing_adapter(self):
+        # If there is no multi-adapter from the entry interface (IMyEntry) and
+        # a request to IEntry an exception is raised.
+        class IMyEntry(Interface):
+            pass
+
+        # Since the test wants to inspect the exception message (below) we're
+        # not using self.assertRaises).
+        try:
+            EntryAdapterUtility.forSchemaInterface(
+                IMyEntry, self.beta_request)
+        except Exception, e:
+            pass
+        self.assert_(isinstance(e, Exception))
+
+        # The exception's message explains what went wrong.
+        self.assertTrue(str(e),
+            'No IEntry adapter found for IMyEntry (web service version: beta).')
+
+    def test_missing_adapter_whence(self):
+        # The UnknownEntryAdapter exception has a "whence" attribute that
+        # higher-level code can set to give the reader of the exeption message
+        # a hint about where in the code the mistake was made that triggered
+        # the exception.
+        exception = UnknownEntryAdapter('IInterfaceInQuestion', '2.0')
+        exception.whence = 'Encounterd as a result of badness in foo.py.'
+        self.assertEquals(str(exception),
+            'No IEntry adapter found for IInterfaceInQuestion (web service '
+            'version: 2.0).  Encounterd as a result of badness in foo.py.')
+
 
 class FunctionalLayer:
     allow_teardown = False