Merge pull request #488 from arctype-co/error-json

[server] Return HTTP errors in JSON format if Accept: contains application/json

CHANGES.rst

@@ -19,6 +19,7 @@ Changelog for Isso
 - Re-enable ``no-intra-emphasis`` misaka extension in default config.
 - Allow ``sup`` and ``sub`` HTML elements by default
 - Move ``isso-dev.cfg`` to ``contrib/`` folder
+- wsgi: Return HTTP errors as JSON if client prefers it (`#488`_, sundbry)
 - Add ``data-isso-page-author-hashes`` option to client which makes it possible
   to style comments and replies made by the page's author(s).
 - Add Ukrainian localisation (`#878`_, okawo80085)
@@ -28,6 +29,7 @@ Changelog for Isso
 
 .. _Gravatar: Image requests: http://en.gravatar.com/site/implement/images/
 .. _879: https://github.com/posativ/isso/pull/879
+.. _488: https://github.com/posativ/isso/pull/488
 
 0.12.6 (2022-03-06)
 -------------------

isso/__init__.py

@@ -64,7 +64,7 @@
 from isso import config, db, migrate, wsgi, ext, views
 from isso.core import ThreadedMixin, ProcessMixin, uWSGIMixin
 from isso.wsgi import origin, urlsplit
-from isso.utils import http, JSONRequest, html, hash
+from isso.utils import http, JSONRequest, JSONResponse, html, hash
 from isso.views import comments
 
 from isso.ext.notifications import Stdout, SMTP
@@ -77,6 +77,14 @@
 logger = logging.getLogger("isso")
 
 
+def error_handler(env, request, error):
+    if request.accept_mimetypes.best == "application/json":
+        data = {'message': str(error)}
+        code = 500 if error.code is None else error.code
+        return JSONResponse(data, code)
+    return error
+
+
 class ProxyFixCustom(ProxyFix):
     def __init__(self, app):
         # This is needed for werkzeug.wsgi.get_current_url called in isso/views/comments.py
@@ -139,16 +147,16 @@ def dispatch(self, request):
         try:
             handler, values = adapter.match()
         except HTTPException as e:
-            return e
+            return error_handler(request.environ, request, e)
         else:
             try:
                 response = handler(request.environ, request, **values)
             except HTTPException as e:
-                return e
+                return error_handler(request.environ, request, e)
             except Exception:
                 logger.exception("%s %s", request.method,
                                  request.environ["PATH_INFO"])
-                return InternalServerError()
+                return error_handler(request.environ, request, InternalServerError())
             else:
                 return response
 

isso/tests/test_wsgi.py

@@ -1,8 +1,14 @@
 # -*- encoding: utf-8 -*-
 
+import json
 import unittest
 
-from isso import wsgi
+from werkzeug.exceptions import BadRequest
+from werkzeug.test import create_environ
+from werkzeug.wrappers import Request
+
+from isso import error_handler, wsgi
+from isso.utils import JSONResponse
 
 
 class TestWSGIUtilities(unittest.TestCase):
@@ -43,3 +49,42 @@ def test_origin(self):
         self.assertEqual(origin({"HTTP_ORIGIN": "http://spam.baz"}),
                          "http://foo.bar")
         self.assertEqual(origin({}), "http://foo.bar")
+
+    def test_errorhandler(self):
+        """
+        Test out MIME type accept parsing:
+
+        >>> from werkzeug.datastructures import MIMEAccept
+        >>> a = MIMEAccept([('text/html', 0.5), ('application/json', 1),])
+        >>> a.best
+        'application/json'
+        """
+
+        # Client prefers response with `text/html` MIME type
+        env = create_environ(headers=((
+            'Accept', 'text/html, application/xml;q=0.9'),
+        ))
+        req = Request(env)
+        error = BadRequest
+        # Error is simply passed through
+        self.assertEqual(error_handler(env, req, error), BadRequest)
+
+        # Client prefers response with `application/json` MIME type
+        env = create_environ(headers=((
+            'Accept', 'text/html;q=0.7, application/json;q=0.9, */*;q=0.8'),
+        ))
+        req = Request(env)
+        error = BadRequest('invalid data')
+        self.assertEqual(req.accept_mimetypes.best, "application/json")
+        # Error is converted to JSONResponse
+        self.assertIsInstance(error_handler(env, req, error), JSONResponse)
+        # Error code is retained by JSONResponse
+        self.assertEqual(error_handler(env, req, error).status_code, 400)
+
+        # Missing error codes get converted to 500
+        error.code = None
+        self.assertEqual(json.loads(
+            error_handler(env, req, error).response[0])['message'],
+            '??? Unknown Error: invalid data'
+        )
+        self.assertEqual(error_handler(env, req, error).status_code, 500)