Improve error messages by including more detailed information.

Author: thet

CHANGES.rst

@@ -4,6 +4,9 @@ Changelog
 1.3 (unreleased)
 ----------------
 
+- Improve error messages by including more detailed information.
+  [thet]
+
 - Do not wrap resource ``__repr__`` output in ``<>`` to render tracebacks
   properly in browser.
   [lenadax]
@@ -68,7 +71,7 @@ Changelog
   Modernize setup.[py|cfg].
   [jensens]
 
-- Added ``GracefulResourceRenderer``. 
+- Added ``GracefulResourceRenderer``.
   Fixes #1.
   [jensens]
 

webresource/_api.py

@@ -85,14 +85,36 @@ def include(self, include):
     def remove(self):
         """Remove resource or resource group from parent group."""
         if not self.parent:
-            raise ResourceError('Object is no member of a resource group')
+            raise ResourceError(
+                'Cannot remove resource or resource group {}.'
+                ' It is no member of a resource group.'.format(self)
+            )
         self.parent.members.remove(self)
         self.parent = None
 
     def copy(self):
         """Return a deep copy of this object."""
         return copy.deepcopy(self)
 
+    def __repr__(self):
+        """Return a string representation of the resource or resource group."""
+
+        reprs = []
+        if getattr(self, "name", None):
+            reprs.append('name="{}"'.format(self.name))
+        elif getattr(self, "path", None):
+            reprs.append('path="{}"'.format(self.path))
+        elif getattr(self, "url", None):
+            reprs.append('url="{}"'.format(self.url))
+
+        if getattr(self, "depends", None):
+            reprs.append('depends="{}"'.format(self.depends))
+
+        return '<{} {}>'.format(
+            self.__class__.__name__,
+            ", ".join(reprs) if reprs else "unnamed"
+        )
+
 
 class Resource(ResourceMixin):
     """A web resource."""
@@ -135,12 +157,16 @@ def __init__(
             additional attributes on resource tag.
         :raise ResourceError: No resource and no url given.
         """
-        if resource is None and url is None:
-            raise ResourceError('Either resource or url must be given')
         super(Resource, self).__init__(
             name=name, directory=directory, path=path,
             include=include, group=group
         )
+        if resource is None and url is None:
+            raise ResourceError(
+                'Either resource or url must be given for resource {}'.format(
+                    self
+                )
+            )
         self.depends = (
             (depends if isinstance(depends, (list, tuple)) else [depends])
             if depends else None
@@ -169,7 +195,9 @@ def file_path(self):
         """Absolute resource file path depending on operation mode."""
         directory = self.directory
         if not directory:
-            raise ResourceError('No directory set on resource.')
+            raise ResourceError(
+                'No directory set on resource {}'.format(self)
+            )
         return os.path.join(directory, self.file_name)
 
     @property
@@ -235,15 +263,6 @@ def _render_tag(self, tag, closing_tag, **attrs):
             return u'<{tag}{attrs} />'.format(tag=tag, attrs=attrs_)
         return u'<{tag}{attrs}></{tag}>'.format(tag=tag, attrs=attrs_)
 
-    def __repr__(self):
-        return (
-            '{} name="{}", depends="{}"'
-        ).format(
-            self.__class__.__name__,
-            self.name,
-            self.depends
-        )
-
 
 class ScriptResource(Resource):
     """A Javascript resource."""
@@ -323,8 +342,10 @@ def integrity(self):
     def integrity(self, integrity):
         if integrity is True:
             if self.url is not None:
-                msg = 'Cannot calculate integrity hash from external resource'
-                raise ResourceError(msg)
+                raise ResourceError(
+                    'Cannot calculate integrity hash from external resource '
+                    '{}'.format(self)
+                )
             self._integrity_hash = None
         else:
             self._integrity_hash = integrity
@@ -564,8 +585,8 @@ def add(self, member):
         """
         if not isinstance(member, (ResourceGroup, Resource)):
             raise ResourceError(
-                'Resource group can only contain instances '
-                'of ``ResourceGroup`` or ``Resource``'
+                'Resource group {} can only contain instances '
+                'of ``ResourceGroup`` or ``Resource``'.format(self)
             )
         member.parent = self
         self._members.append(member)
@@ -584,12 +605,6 @@ def _filtered_resources(self, type_, members=None):
                 resources.append(member)
         return resources
 
-    def __repr__(self):
-        return '{} name="{}"'.format(
-            self.__class__.__name__,
-            self.name
-        )
-
 
 class ResourceConflictError(ResourceError):
     """Multiple resources declared with the same name."""
@@ -634,7 +649,7 @@ def __init__(self, members):
         for member in members:
             if not isinstance(member, (Resource, ResourceGroup)):
                 raise ResourceError(
-                    'members can only contain instances '
+                    'ResourceResolver members can only contain instances '
                     'of ``ResourceGroup`` or ``Resource``'
                 )
         self.members = members
@@ -726,10 +741,19 @@ class GracefulResourceRenderer(ResourceRenderer):
     def render(self):
         lines = []
         for resource in self.resolver.resolve():
+            error_message = None
             try:
                 lines.append(resource.render(self.base_url))
-            except (ResourceError, FileNotFoundError):
-                msg = u'Failure to render resource "{}"'.format(resource.name)
-                lines.append(u'<!-- {} - details in logs -->'.format(msg))
-                logger.exception(msg)
+            except FileNotFoundError:
+                error_message = u'File not found for resource {}'.format(
+                    resource
+                )
+            except ResourceError as e:
+                error_message = str(e)
+            finally:
+                if error_message:
+                    lines.append(u'<!-- {} - details in logs -->'.format(
+                        error_message
+                    ))
+                    logger.exception(error_message)
         return u'\n'.join(lines)

webresource/tests.py

@@ -111,7 +111,7 @@ def test_Resource(self, tempdir):
         self.assertEqual(resource.type_, None)
         self.assertEqual(
             repr(resource),
-            'Resource name="res", depends="None"'
+            '<Resource name="res">'
         )
 
         resource = Resource(name='res', resource='res.ext')
@@ -239,7 +239,7 @@ def test_ScriptResource(self, tempdir):
         self.assertEqual(script.nomodule, None)
         self.assertEqual(
             repr(script),
-            'ScriptResource name="js_res", depends="None"'
+            """<ScriptResource name="js_res">"""
         )
         self.assertEqual(
             script.render('https://tld.org'),
@@ -301,7 +301,7 @@ def test_LinkMixin(self):
         self.assertEqual(link.title, None)
         self.assertEqual(
             repr(link),
-            'LinkMixin name="link_res", depends="None"'
+            '<LinkMixin name="link_res">'
         )
         link.hreflang = 'en'
         link.media = 'screen'
@@ -327,7 +327,7 @@ def test_LinkResource(self):
         self.assertIsInstance(link, LinkMixin)
         self.assertEqual(
             repr(link),
-            'LinkResource name="icon_res", depends="None"'
+            '<LinkResource name="icon_res">'
         )
         link.rel = 'icon'
         link.type_ = 'image/png'
@@ -355,7 +355,7 @@ def test_StyleResource(self):
         self.assertEqual(style.rel, 'stylesheet')
         self.assertEqual(
             repr(style),
-            'StyleResource name="css_res", depends="None"'
+            """<StyleResource name="css_res">"""
         )
         self.assertEqual(style.render('https://tld.org'), (
             '<link href="https://tld.org/res.css" media="all" '
@@ -377,7 +377,7 @@ def test_ResourceGroup(self):
         self.assertIsInstance(group, ResourceMixin)
         self.assertEqual(group.name, 'groupname')
         self.assertEqual(group.members, [])
-        self.assertEqual(repr(group), 'ResourceGroup name="groupname"')
+        self.assertEqual(repr(group), '<ResourceGroup name="groupname">')
 
         res = wr.ScriptResource(name='name', resource='name.js')
         group.add(res)
@@ -442,6 +442,115 @@ def test_ResourceGroup(self):
         self.assertEqual(group.members, [])
         self.assertEqual(resource.parent, None)
 
+    def test_resource_repr(self):
+        res = ResourceMixin(name='res')
+        self.assertEqual(
+            repr(res),
+            '<ResourceMixin name="res">'
+        )
+
+        # Fallback, if nothing is set.
+        res = ResourceMixin()
+        self.assertEqual(
+            repr(res),
+            '<ResourceMixin unnamed>'
+        )
+
+        res = ResourceMixin(path="/a/b/c")
+        self.assertEqual(
+            repr(res),
+            '<ResourceMixin path="/a/b/c">'
+        )
+
+        res = Resource(url="https://example.com")
+        self.assertEqual(
+            repr(res),
+            '<Resource url="https://example.com">'
+        )
+
+        # Name takes precendence over all other attributes.
+        res = Resource(name='res', path="/a/b/c", url="https://example.com")
+        self.assertEqual(
+            repr(res),
+            '<Resource name="res">'
+        )
+
+        # Path takes precendence over url.
+        res = Resource(path="/a/b/c", url="https://example.com")
+        self.assertEqual(
+            repr(res),
+            '<Resource path="/a/b/c">'
+        )
+
+        # Example with depends
+        res = Resource(
+            name='res',
+            url="https://example.com",
+            depends="other.ext",
+        )
+        self.assertEqual(
+            repr(res),
+            """<Resource name="res", depends="['other.ext']">"""
+        )
+
+    def test_ResourceError(self):
+        with self.assertRaises(wr.ResourceError) as cm:
+            Resource()
+
+        self.assertEqual(
+            str(cm.exception),
+            'Either resource or url must be given for resource '
+            '<Resource unnamed>'
+        )
+
+        with self.assertRaises(wr.ResourceError) as cm:
+            res = ResourceMixin(name="res")
+            res.remove()
+
+        self.assertEqual(
+            str(cm.exception),
+            'Cannot remove resource or resource group '
+            '<ResourceMixin name="res">. It is no member of a resource group.'
+        )
+
+        with self.assertRaises(wr.ResourceError) as cm:
+            res = Resource(url='https://example.com')
+            res.file_path
+
+        self.assertEqual(
+            str(cm.exception),
+            'No directory set on resource <Resource url="https://example.com">'
+        )
+
+        with self.assertRaises(wr.ResourceError) as cm:
+            res = wr.ScriptResource(url='https://example.com')
+            res.integrity = True
+
+        self.assertEqual(
+            str(cm.exception),
+            'Cannot calculate integrity hash from external resource '
+            '<ScriptResource url="https://example.com">'
+        )
+
+        with self.assertRaises(wr.ResourceError) as cm:
+            res = wr.ResourceGroup(name='okay')
+            res.add(object())
+
+        self.assertEqual(
+            str(cm.exception),
+            'Resource group <ResourceGroup name="okay"> can only contain '
+            'instances of ``ResourceGroup`` or ``Resource``'
+        )
+
+        with self.assertRaises(wr.ResourceError) as cm:
+            res = wr.ResourceResolver(None)
+
+        self.assertEqual(
+            str(cm.exception),
+            'ResourceResolver members can only contain instances of '
+            '``ResourceGroup`` or ``Resource``'
+        )
+
     def test_ResourceConflictError(self):
         counter = Counter(['a', 'b', 'b', 'c', 'c'])
         err = wr.ResourceConflictError(counter)
@@ -452,15 +561,15 @@ def test_ResourceCircularDependencyError(self):
         err = wr.ResourceCircularDependencyError([resource])
         self.assertEqual(str(err), (
             'Resources define circular dependencies: '
-            '[Resource name="res1", depends="[\'res2\']"]'
+            """[<Resource name="res1", depends="['res2']">]"""
         ))
 
     def test_ResourceMissingDependencyError(self):
         resource = Resource(name='res', resource='res.ext', depends='missing')
         err = wr.ResourceMissingDependencyError(resource)
         self.assertEqual(str(err), (
             'Resource defines missing dependency: '
-            'Resource name="res", depends="[\'missing\']"'
+            """<Resource name="res", depends="['missing']">"""
         ))
 
     def test_ResourceResolver__flat_resources(self):
@@ -687,7 +796,7 @@ def test_GracefulResourceRenderer(self):
                 # check if its ours
                 self.assertEqual(
                     captured.records[0].getMessage().split('\n')[0],
-                    'Failure to render resource "js2"',
+                    """File not found for resource <ScriptResource name="js2", depends="['js']">"""
                 )
         else:  # pragma: nocover
             rendered = renderer.render()
@@ -699,7 +808,7 @@ def test_GracefulResourceRenderer(self):
             '<link href="https://ext.org/styles.css" media="all" '
             'rel="stylesheet" type="text/css" />\n'
             '<script src="https://tld.org/res/script.js"></script>\n'
-            '<!-- Failure to render resource "js2" - details in logs -->'
+            """<!-- File not found for resource <ScriptResource name="js2", depends="['js']"> - details in logs -->"""
         ))