Merge pull request #2489 from Pylons/feature/json_exceptions

Feature: JSON exceptions

3 files changed, 153 insertions, 25 deletions

diff --git a/CHANGES.txt b/CHANGES.txt
index da59c3e6f..96d8d8236 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -1,6 +1,13 @@
 unreleased
 ==========
 
+- Pyramid HTTPExceptions will now take into account the best match for the
+  clients Accept header, and depending on what is requested will return
+  text/html, application/json or text/plain. The default for */* is still
+  text/html, but if application/json is explicitly mentioned it will now
+  receive a valid JSON response. See:
+  https://github.com/Pylons/pyramid/pull/2489
+
 - (Deprecation) Support for Python 3.3 will be removed in Pyramid 1.8.
   https://github.com/Pylons/pyramid/issues/2477
 
diff --git a/pyramid/httpexceptions.py b/pyramid/httpexceptions.py
index 8bf9a0a72..e76f43c8a 100644
--- a/pyramid/httpexceptions.py
+++ b/pyramid/httpexceptions.py
@@ -123,12 +123,14 @@ The subclasses of :class:`~_HTTPMove`
 field. Reflecting this, these subclasses have one additional keyword argument:
 ``location``, which indicates the location to which to redirect.
 """
+import json
 
 from string import Template
 
 from zope.interface import implementer
 
 from webob import html_escape as _html_escape
+from webob.acceptparse import MIMEAccept
 
 from pyramid.compat import (
     class_types,
@@ -214,7 +216,7 @@ ${body}''')
     empty_body = False
 
     def __init__(self, detail=None, headers=None, comment=None,
-                 body_template=None, **kw):
+                 body_template=None, json_formatter=None, **kw):
         status = '%s %s' % (self.code, self.title)
         Response.__init__(self, status=status, **kw)
         Exception.__init__(self, detail)
@@ -225,6 +227,8 @@ ${body}''')
         if body_template is not None:
             self.body_template = body_template
             self.body_template_obj = Template(body_template)
+        if json_formatter is not None:
+            self._json_formatter = json_formatter
 
         if self.empty_body:
             del self.content_type
@@ -233,18 +237,48 @@ ${body}''')
     def __str__(self):
         return self.detail or self.explanation
 
+    def _json_formatter(self, status, body, title, environ):
+        return {'message': body,
+                'code': status,
+                'title': self.title}
+
     def prepare(self, environ):
         if not self.body and not self.empty_body:
             html_comment = ''
             comment = self.comment or ''
-            accept = environ.get('HTTP_ACCEPT', '')
-            if accept and 'html' in accept or '*/*' in accept:
+            accept_value = environ.get('HTTP_ACCEPT', '')
+            accept = MIMEAccept(accept_value)
+            # Attempt to match text/html or application/json, if those don't
+            # match, we will fall through to defaulting to text/plain
+            match = accept.best_match(['text/html', 'application/json'])
+
+            if match == 'text/html':
                 self.content_type = 'text/html'
                 escape = _html_escape
                 page_template = self.html_template_obj
                 br = '<br/>'
                 if comment:
                     html_comment = '<!-- %s -->' % escape(comment)
+            elif match == 'application/json':
+                self.content_type = 'application/json'
+                self.charset = None
+                escape = _no_escape
+                br = '\n'
+                if comment:
+                    html_comment = escape(comment)
+
+                class JsonPageTemplate(object):
+                    def __init__(self, excobj):
+                        self.excobj = excobj
+
+                    def substitute(self, status, body):
+                        jsonbody = self.excobj._json_formatter(
+                            status=status,
+                            body=body, title=self.excobj.title,
+                            environ=environ)
+                        return json.dumps(jsonbody)
+
+                page_template = JsonPageTemplate(self)
             else:
                 self.content_type = 'text/plain'
                 escape = _no_escape
@@ -253,11 +287,11 @@ ${body}''')
                 if comment:
                     html_comment = escape(comment)
             args = {
-                'br':br,
+                'br': br,
                 'explanation': escape(self.explanation),
                 'detail': escape(self.detail or ''),
                 'comment': escape(comment),
-                'html_comment':html_comment,
+                'html_comment': html_comment,
                 }
             body_tmpl = self.body_template_obj
             if HTTPException.body_template_obj is not body_tmpl:
@@ -274,7 +308,7 @@ ${body}''')
             body = body_tmpl.substitute(args)
             page = page_template.substitute(status=self.status, body=body)
             if isinstance(page, text_type):
-                page = page.encode(self.charset)
+                page = page.encode(self.charset if self.charset else 'UTF-8')
             self.app_iter = [page]
             self.body = page
 
@@ -1001,8 +1035,8 @@ class HTTPInternalServerError(HTTPServerError):
     code = 500
     title = 'Internal Server Error'
     explanation = (
-      'The server has either erred or is incapable of performing '
-      'the requested operation.')
+        'The server has either erred or is incapable of performing '
+        'the requested operation.')
 
 class HTTPNotImplemented(HTTPServerError):
     """
diff --git a/pyramid/tests/test_httpexceptions.py b/pyramid/tests/test_httpexceptions.py
index b94ef30e4..6c6e16d55 100644
--- a/pyramid/tests/test_httpexceptions.py
+++ b/pyramid/tests/test_httpexceptions.py
@@ -28,9 +28,9 @@ class Test_exception_response(unittest.TestCase):
         self.assertTrue(isinstance(self._callFUT(201), HTTPCreated))
 
     def test_extra_kw(self):
-        resp = self._callFUT(404,  headers=[('abc', 'def')])
+        resp = self._callFUT(404, headers=[('abc', 'def')])
         self.assertEqual(resp.headers['abc'], 'def')
-        
+
 class Test_default_exceptionresponse_view(unittest.TestCase):
     def _callFUT(self, context, request):
         from pyramid.httpexceptions import default_exceptionresponse_view
@@ -129,7 +129,7 @@ class TestHTTPException(unittest.TestCase):
     def test_ctor_sets_body_template_obj(self):
         exc = self._makeOne(body_template='${foo}')
         self.assertEqual(
-            exc.body_template_obj.substitute({'foo':'foo'}), 'foo')
+            exc.body_template_obj.substitute({'foo': 'foo'}), 'foo')
 
     def test_ctor_with_empty_body(self):
         cls = self._getTargetSubclass(empty_body=True)
@@ -160,7 +160,7 @@ class TestHTTPException(unittest.TestCase):
         self.assertTrue(b'200 OK' in body)
         self.assertTrue(b'explanation' in body)
         self.assertTrue(b'detail' in body)
-        
+
     def test_ctor_with_body_sets_default_app_iter_text(self):
         cls = self._getTargetSubclass()
         exc = cls('detail')
@@ -173,7 +173,7 @@ class TestHTTPException(unittest.TestCase):
         exc = self._makeOne()
         exc.detail = 'abc'
         self.assertEqual(str(exc), 'abc')
-        
+
     def test__str__explanation(self):
         exc = self._makeOne()
         exc.explanation = 'def'
@@ -212,6 +212,9 @@ class TestHTTPException(unittest.TestCase):
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
+        for header in start_response.headerlist:
+            if header[0] == 'Content-Type':
+                self.assertEqual(header[1], 'text/plain; charset=UTF-8')
         self.assertEqual(body, b'200 OK\n\nexplanation\n\n\n\n\n')
 
     def test__default_app_iter_with_comment_plain(self):
@@ -220,26 +223,78 @@ class TestHTTPException(unittest.TestCase):
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
+        for header in start_response.headerlist:
+            if header[0] == 'Content-Type':
+                self.assertEqual(header[1], 'text/plain; charset=UTF-8')
         self.assertEqual(body, b'200 OK\n\nexplanation\n\n\n\ncomment\n')
-        
+
     def test__default_app_iter_no_comment_html(self):
         cls = self._getTargetSubclass()
         exc = cls()
         environ = _makeEnviron()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
+        for header in start_response.headerlist:
+            if header[0] == 'Content-Type':
+                self.assertEqual(header[1], 'text/plain; charset=UTF-8')
         self.assertFalse(b'<!-- ' in body)
 
-    def test__default_app_iter_with_comment_html(self):
+    def test__content_type(self):
         cls = self._getTargetSubclass()
-        exc = cls(comment='comment & comment')
+        exc = cls()
+        environ = _makeEnviron()
+        start_response = DummyStartResponse()
+        exc(environ, start_response)
+        for header in start_response.headerlist:
+            if header[0] == 'Content-Type':
+                self.assertEqual(header[1], 'text/plain; charset=UTF-8')
+
+    def test__content_type_default_is_html(self):
+        cls = self._getTargetSubclass()
+        exc = cls()
         environ = _makeEnviron()
         environ['HTTP_ACCEPT'] = '*/*'
         start_response = DummyStartResponse()
+        exc(environ, start_response)
+        for header in start_response.headerlist:
+            if header[0] == 'Content-Type':
+                self.assertEqual(header[1], 'text/html; charset=UTF-8')
+
+    def test__content_type_text_html(self):
+        cls = self._getTargetSubclass()
+        exc = cls()
+        environ = _makeEnviron()
+        environ['HTTP_ACCEPT'] = 'text/html'
+        start_response = DummyStartResponse()
+        exc(environ, start_response)
+        for header in start_response.headerlist:
+            if header[0] == 'Content-Type':
+                self.assertEqual(header[1], 'text/html; charset=UTF-8')
+
+    def test__content_type_application_json(self):
+        cls = self._getTargetSubclass()
+        exc = cls()
+        environ = _makeEnviron()
+        environ['HTTP_ACCEPT'] = 'application/json'
+        start_response = DummyStartResponse()
+        exc(environ, start_response)
+        for header in start_response.headerlist:
+            if header[0] == 'Content-Type':
+                self.assertEqual(header[1], 'application/json')
+
+    def test__default_app_iter_with_comment_ampersand(self):
+        cls = self._getTargetSubclass()
+        exc = cls(comment='comment & comment')
+        environ = _makeEnviron()
+        environ['HTTP_ACCEPT'] = 'text/html'
+        start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
+        for header in start_response.headerlist:
+            if header[0] == 'Content-Type':
+                self.assertEqual(header[1], 'text/html; charset=UTF-8')
         self.assertTrue(b'<!-- comment &amp; comment -->' in body)
 
-    def test__default_app_iter_with_comment_html2(self):
+    def test__default_app_iter_with_comment_html(self):
         cls = self._getTargetSubclass()
         exc = cls(comment='comment & comment')
         environ = _makeEnviron()
@@ -248,6 +303,38 @@ class TestHTTPException(unittest.TestCase):
         body = list(exc(environ, start_response))[0]
         self.assertTrue(b'<!-- comment &amp; comment -->' in body)
 
+    def test__default_app_iter_with_comment_json(self):
+        cls = self._getTargetSubclass()
+        exc = cls(comment='comment & comment')
+        environ = _makeEnviron()
+        environ['HTTP_ACCEPT'] = 'application/json'
+        start_response = DummyStartResponse()
+        body = list(exc(environ, start_response))[0]
+        import json
+        retval = json.loads(body.decode('UTF-8'))
+        self.assertEqual(retval['code'], '200 OK')
+        self.assertEqual(retval['title'], 'OK')
+
+    def test__default_app_iter_with_custom_json(self):
+        def json_formatter(status, body, title, environ):
+            return {'message': body,
+                    'code': status,
+                    'title': title,
+                    'custom': environ['CUSTOM_VARIABLE']
+                    }
+        cls = self._getTargetSubclass()
+        exc = cls(comment='comment', json_formatter=json_formatter)
+        environ = _makeEnviron()
+        environ['HTTP_ACCEPT'] = 'application/json'
+        environ['CUSTOM_VARIABLE'] = 'custom!'
+        start_response = DummyStartResponse()
+        body = list(exc(environ, start_response))[0]
+        import json
+        retval = json.loads(body.decode('UTF-8'))
+        self.assertEqual(retval['code'], '200 OK')
+        self.assertEqual(retval['title'], 'OK')
+        self.assertEqual(retval['custom'], 'custom!')
+
     def test_custom_body_template(self):
         cls = self._getTargetSubclass()
         exc = cls(body_template='${REQUEST_METHOD}')
@@ -261,7 +348,8 @@ class TestHTTPException(unittest.TestCase):
         exc = cls(body_template='${REQUEST_METHOD}')
         environ = _makeEnviron()
         class Choke(object):
-            def __str__(self): raise ValueError
+            def __str__(self): # pragma nocover
+                raise ValueError
         environ['gardentheory.user'] = Choke()
         start_response = DummyStartResponse()
         body = list(exc(environ, start_response))[0]
@@ -293,7 +381,7 @@ class TestRenderAllExceptionsWithoutArguments(unittest.TestCase):
                 self.assertTrue(bytes_(exc.status) in result)
             L.append(result)
         self.assertEqual(len(L), len(status_map))
-            
+
     def test_it_plain(self):
         self._doit('text/plain')
 
@@ -367,12 +455,11 @@ class DummyStartResponse(object):
     def __call__(self, status, headerlist):
         self.status = status
         self.headerlist = headerlist
-        
+
 def _makeEnviron(**kw):
-    environ = {'REQUEST_METHOD':'GET',
-               'wsgi.url_scheme':'http',
-               'SERVER_NAME':'localhost',
-               'SERVER_PORT':'80'}
+    environ = {'REQUEST_METHOD': 'GET',
+               'wsgi.url_scheme': 'http',
+               'SERVER_NAME': 'localhost',
+               'SERVER_PORT': '80'}
     environ.update(kw)
     return environ
-