Fix: Make error initializers more consistent

docs/_newsfragments/777-refactor-errors.feature.rst

@@ -0,0 +1,2 @@
+Update to the error classes in ``falcon.errors`` to have ``title`` and
+``description`` keyword arguments

docs/_newsfragments/777-refactor-errors.removal.rst

@@ -0,0 +1,2 @@
+The class :class:`~.falcon.http_error.NoRepresentation` was removed since
+all :class:`~.falcon.HTTPError` have optional representation

docs/api/errors.rst

@@ -79,7 +79,7 @@ Base Class
 Mixins
 ------
 
-.. autoclass:: falcon.http_error.NoRepresentation
+.. autoclass:: falcon.http_error.OptionalRepresentation
     :members:
 
 .. _predefined_errors:

falcon/app.py

@@ -729,7 +729,8 @@ def set_error_serializer(self, serializer):
             The default serializer will not render any response body for
             :class:`~.HTTPError` instances where the `has_representation`
             property evaluates to ``False`` (such as in the case of types
-            that subclass :class:`falcon.http_error.NoRepresentation`).
+            that subclass :class:`falcon.http_error.OptionalRepresentation`
+            and do not provide a description).
             However a custom serializer will be called regardless of the
             property value, and it may choose to override the
             representation logic.

falcon/http_error.py

@@ -79,7 +79,7 @@ class HTTPError(Exception):
             returns ``True``, but child classes may override it
             in order to return ``False`` when an empty HTTP body is desired.
 
-            (See also: :class:`falcon.http_error.NoRepresentation`)
+            (See also: :class:`falcon.http_error.OptionalRepresentation`)
 
             Note:
                 A custom error serializer
@@ -203,30 +203,6 @@ def to_xml(self):
                 et.tostring(error_element, encoding='utf-8'))
 
 
-class NoRepresentation:
-    """Mixin for ``HTTPError`` child classes that have no representation.
-
-    This class can be mixed in when inheriting from ``HTTPError``, in order
-    to override the `has_representation` property such that it always
-    returns ``False``. This, in turn, will cause Falcon to return an empty
-    response body to the client.
-
-    You can use this mixin when defining errors that either should not have
-    a body (as dictated by HTTP standards or common practice), or in the
-    case that a detailed error response may leak information to an attacker.
-
-    Note:
-        This mixin class must appear before ``HTTPError`` in the base class
-        list when defining the child; otherwise, it will not override the
-        `has_representation` property as expected.
-
-    """
-
-    @property
-    def has_representation(self):
-        return False
-
-
 class OptionalRepresentation:
     """Mixin for ``HTTPError`` child classes that may have a representation.
 
@@ -247,4 +223,4 @@ class OptionalRepresentation:
     """
     @property
     def has_representation(self):
-        return super(OptionalRepresentation, self).description is not None
+        return super().description is not None

tests/test_error.py

@@ -1,17 +1,24 @@
 import pytest
 
 import falcon
+import falcon.errors as errors
 import falcon.status_codes as status
 
 
 @pytest.mark.parametrize('err, title', [
     (falcon.HTTPBadRequest, status.HTTP_400),
+    (falcon.HTTPUnauthorized, status.HTTP_401),
     (falcon.HTTPForbidden, status.HTTP_403),
+    (falcon.HTTPNotFound, status.HTTP_404),
+    (errors.HTTPRouteNotFound, status.HTTP_404),
+    (falcon.HTTPNotAcceptable, status.HTTP_406),
     (falcon.HTTPConflict, status.HTTP_409),
+    (falcon.HTTPGone, status.HTTP_410),
     (falcon.HTTPLengthRequired, status.HTTP_411),
     (falcon.HTTPPreconditionFailed, status.HTTP_412),
     (falcon.HTTPPayloadTooLarge, status.HTTP_413),
     (falcon.HTTPUriTooLong, status.HTTP_414),
+    (falcon.HTTPUnsupportedMediaType, status.HTTP_415),
     (falcon.HTTPUnprocessableEntity, status.HTTP_422),
     (falcon.HTTPLocked, status.HTTP_423),
     (falcon.HTTPFailedDependency, status.HTTP_424),
@@ -40,17 +47,40 @@ def test_with_default_title_and_desc(err, title):
         assert 'Retry-After' not in e.value.headers
 
 
+@pytest.mark.parametrize('err, title, args', (
+    (falcon.HTTPMethodNotAllowed, status.HTTP_405, (['GET'], )),
+    (falcon.HTTPRangeNotSatisfiable, status.HTTP_416, (11,)),
+))
+def test_with_default_title_and_desc_args(err, title, args):
+    with pytest.raises(err) as e:
+        raise err(*args)
+
+    assert e.value.title == title
+    assert e.value.description is None
+
+    if e.value.headers:
+        assert 'Retry-After' not in e.value.headers
+
+
 @pytest.mark.parametrize('err', [
     falcon.HTTPBadRequest,
+    falcon.HTTPUnauthorized,
     falcon.HTTPForbidden,
+    falcon.HTTPNotFound,
+    errors.HTTPRouteNotFound,
+    falcon.HTTPNotAcceptable,
     falcon.HTTPConflict,
+    falcon.HTTPGone,
     falcon.HTTPLengthRequired,
     falcon.HTTPPreconditionFailed,
-    falcon.HTTPPreconditionRequired,
+    falcon.HTTPPayloadTooLarge,
     falcon.HTTPUriTooLong,
+    falcon.HTTPUnsupportedMediaType,
     falcon.HTTPUnprocessableEntity,
     falcon.HTTPLocked,
     falcon.HTTPFailedDependency,
+    falcon.HTTPPreconditionRequired,
+    falcon.HTTPTooManyRequests,
     falcon.HTTPRequestHeaderFieldsTooLarge,
     falcon.HTTPUnavailableForLegalReasons,
     falcon.HTTPInternalServerError,
@@ -76,6 +106,23 @@ def test_with_title_desc_and_headers(err):
     assert e.value.headers['foo'] == 'bar'
 
 
+@pytest.mark.parametrize('err, args', (
+    (falcon.HTTPMethodNotAllowed, (['GET'], )),
+    (falcon.HTTPRangeNotSatisfiable, (11,)),
+))
+def test_with_title_desc_and_headers_args(err, args):
+    title = 'trace'
+    desc = 'boom'
+    headers = {'foo': 'bar'}
+
+    with pytest.raises(err) as e:
+        raise err(*args, title=title, description=desc, headers=headers)
+
+    assert e.value.title == title
+    assert e.value.description == desc
+    assert e.value.headers['foo'] == 'bar'
+
+
 @pytest.mark.parametrize('err', [
     falcon.HTTPServiceUnavailable,
     falcon.HTTPTooManyRequests,
@@ -101,13 +148,6 @@ def test_with_retry_after_and_headers(err):
     assert e.value.headers['foo'] == 'bar'
 
 
-def test_http_range_not_satisfiable_with_headers():
-    try:
-        raise falcon.HTTPRangeNotSatisfiable(resource_length=0, headers={'foo': 'bar'})
-    except falcon.HTTPRangeNotSatisfiable as e:
-        assert e.headers['foo'] == 'bar'
-
-
 def test_http_unauthorized_no_title_and_desc_and_challenges_and_headers():
     try:
         raise falcon.HTTPUnauthorized()
@@ -132,49 +172,23 @@ def test_http_unauthorized_with_title_and_desc_and_challenges_and_headers():
         assert 'bar' == e.headers['foo']
 
 
-def test_http_not_acceptable_no_title_and_desc_and_challenges():
-    try:
-        raise falcon.HTTPNotAcceptable()
-    except falcon.HTTPNotAcceptable as e:
-        assert e.description is None
-
-
-def test_http_not_acceptable_with_title():
-    try:
-        raise falcon.HTTPNotAcceptable(title='test title')
-    except falcon.HTTPNotAcceptable as e:
-        assert e.title == 'test title'
-
-
-def test_http_not_acceptable_with_title_and_desc_and_challenges():
-    try:
-        raise falcon.HTTPNotAcceptable(description='Testdescription')
-    except falcon.HTTPNotAcceptable as e:
-        assert 'Testdescription' == e.description
-
-
-def test_http_unsupported_media_type_no_title_and_desc_and_challenges():
-    try:
-        raise falcon.HTTPUnsupportedMediaType()
-    except falcon.HTTPUnsupportedMediaType as e:
-        assert e.description is None
-
-
-def test_http_unsupported_media_type_with_title():
-    try:
-        raise falcon.HTTPUnsupportedMediaType(title='test title')
-    except falcon.HTTPUnsupportedMediaType as e:
-        assert e.title == 'test title'
-
-
-def test_http_unsupported_media_type_with_title_and_desc_and_challenges():
-    try:
-        raise falcon.HTTPUnsupportedMediaType(description='boom')
-    except falcon.HTTPUnsupportedMediaType as e:
-        assert e.description == 'boom'
-
-
 def test_http_error_repr():
     error = falcon.HTTPBadRequest()
     _repr = '<%s: %s>' % (error.__class__.__name__, error.status)
     assert error.__repr__() == _repr
+
+
+@pytest.mark.parametrize('err, args, title, desc', (
+    (falcon.HTTPInvalidHeader, ('foo', 'bar'), 'Invalid header value',
+     'The value provided for the "bar" header is invalid. foo'),
+    (falcon.HTTPMissingHeader, ('foo',), 'Missing header value', 'The "foo" header is required.'),
+    (falcon.HTTPInvalidParam, ('foo', 'bar'), 'Invalid parameter',
+     'The "bar" parameter is invalid. foo'),
+    (falcon.HTTPMissingParam, ('foo',), 'Missing parameter', 'The "foo" parameter is required.'),
+))
+def test_custom_400(err, args, title, desc):
+    with pytest.raises(err) as e:
+        raise err(*args)
+
+    assert e.value.title == title
+    assert e.value.description == desc

tests/test_headers.py

@@ -349,7 +349,7 @@ def test_required_header(self, client):
         except falcon.HTTPMissingHeader as ex:
             assert isinstance(ex, falcon.HTTPBadRequest)
             assert ex.title == 'Missing header value'
-            expected_desc = 'The X-Not-Found header is required.'
+            expected_desc = 'The "X-Not-Found" header is required.'
             assert ex.description == expected_desc
 
     @pytest.mark.parametrize('status', (falcon.HTTP_204, falcon.HTTP_304))

tests/test_httperror.py

@@ -782,7 +782,7 @@ def test_invalid_header(self, client):
         client.app.add_route('/400', InvalidHeaderResource())
         response = client.simulate_request(path='/400')
 
-        expected_desc = ('The value provided for the X-Auth-Token '
+        expected_desc = ('The value provided for the "X-Auth-Token" '
                          'header is invalid. Please provide a valid token.')
 
         expected_body = {
@@ -800,7 +800,7 @@ def test_missing_header(self, client):
 
         expected_body = {
             'title': 'Missing header value',
-            'description': 'The X-Auth-Token header is required.',
+            'description': 'The "X-Auth-Token" header is required.',
         }
 
         assert response.status == falcon.HTTP_400

tests/test_request_attrs.py

@@ -520,7 +520,7 @@ def test_range_invalid(self, asgi):
             req.range
 
         headers = {'Range': 'bytes=-'}
-        expected_desc = ('The value provided for the Range header is '
+        expected_desc = ('The value provided for the "Range" header is '
                          'invalid. The range offsets are missing.')
         self._test_error_details(headers, 'range',
                                  falcon.HTTPInvalidHeader,
@@ -583,7 +583,7 @@ def test_range_invalid(self, asgi):
             req.range
 
         headers = {'Range': 'bytes=x-y'}
-        expected_desc = ('The value provided for the Range header is '
+        expected_desc = ('The value provided for the "Range" header is '
                          'invalid. It must be a range formatted '
                          'according to RFC 7233.')
         self._test_error_details(headers, 'range',
@@ -592,7 +592,7 @@ def test_range_invalid(self, asgi):
                                  asgi)
 
         headers = {'Range': 'bytes=0-0,-1'}
-        expected_desc = ('The value provided for the Range '
+        expected_desc = ('The value provided for the "Range" '
                          'header is invalid. The value must be a '
                          'continuous range.')
         self._test_error_details(headers, 'range',
@@ -601,7 +601,7 @@ def test_range_invalid(self, asgi):
                                  asgi)
 
         headers = {'Range': '10-'}
-        expected_desc = ('The value provided for the Range '
+        expected_desc = ('The value provided for the "Range" '
                          'header is invalid. The value must be '
                          "prefixed with a range unit, e.g. 'bytes='")
         self._test_error_details(headers, 'range',
@@ -628,7 +628,7 @@ def test_content_length(self, asgi):
     def test_bogus_content_length_nan(self, asgi):
         headers = {'content-length': 'fuzzy-bunnies'}
         expected_desc = ('The value provided for the '
-                         'Content-Length header is invalid. The value '
+                         '"Content-Length" header is invalid. The value '
                          'of the header must be a number.')
         self._test_error_details(headers, 'content_length',
                                  falcon.HTTPInvalidHeader,
@@ -637,7 +637,7 @@ def test_bogus_content_length_nan(self, asgi):
 
     def test_bogus_content_length_neg(self, asgi):
         headers = {'content-length': '-1'}
-        expected_desc = ('The value provided for the Content-Length '
+        expected_desc = ('The value provided for the "Content-Length" '
                          'header is invalid. The value of the header '
                          'must be a positive number.')
         self._test_error_details(headers, 'content_length',
@@ -667,7 +667,7 @@ def test_date_invalid(self, asgi, header, attr):
 
         # Date formats don't conform to RFC 1123
         headers = {header: 'Thu, 04 Apr 2013'}
-        expected_desc = ('The value provided for the {} '
+        expected_desc = ('The value provided for the "{}" '
                          'header is invalid. It must be formatted '
                          'according to RFC 7231, Section 7.1.1.1')