Respond with problems by default in aiohttp. (#952)

* Respond with problems by default in aiohttp.

connexion/apis/aiohttp_api.py

@@ -1,6 +1,8 @@
 import asyncio
 import logging
 import re
+import traceback
+from contextlib import suppress
 from urllib.parse import parse_qs
 
 import aiohttp_jinja2
@@ -9,10 +11,18 @@ from aiohttp import web
 from aiohttp.web_exceptions import HTTPNotFound, HTTPPermanentRedirect
 from aiohttp.web_middlewares import normalize_path_middleware
 from connexion.apis.abstract import AbstractAPI
-from connexion.exceptions import OAuthProblem, OAuthScopeProblem
+from connexion.exceptions import ProblemException
 from connexion.handlers import AuthErrorHandler
 from connexion.lifecycle import ConnexionRequest, ConnexionResponse
+from connexion.problem import problem
 from connexion.utils import Jsonifier, is_json_mimetype, yamldumper
+from werkzeug.exceptions import HTTPException as werkzeug_HTTPException
+
+try:
+    from http import HTTPStatus
+except ImportError:  # pragma: no cover
+    # httpstatus35 backport for python 3.4
+    from httpstatus import HTTPStatus
 
 try:
     import ujson as json
@@ -25,17 +35,57 @@ except ImportError:  # pragma: no cover
 logger = logging.getLogger('connexion.apis.aiohttp_api')
 
 
+def _generic_problem(http_status: HTTPStatus, exc: Exception = None):
+    extra = None
+    if exc is not None:
+        loop = asyncio.get_event_loop()
+        if loop.get_debug():
+            tb = None
+            with suppress(Exception):
+                tb = traceback.format_exc()
+            if tb:
+                extra = {"traceback": tb}
+
+    return problem(
+        status=http_status.value,
+        title=http_status.phrase,
+        detail=http_status.description,
+        ext=extra,
+    )
+
+
 @web.middleware
 @asyncio.coroutine
-def oauth_problem_middleware(request, handler):
+def problems_middleware(request, handler):
     try:
         response = yield from handler(request)
-    except (OAuthProblem, OAuthScopeProblem) as oauth_error:
-        return web.Response(
-            status=oauth_error.code,
-            body=json.dumps(oauth_error.description).encode(),
-            content_type='application/problem+json'
-        )
+    except ProblemException as exc:
+        response = exc.to_problem()
+    except (werkzeug_HTTPException, _HttpNotFoundError) as exc:
+        response = problem(status=exc.code, title=exc.name, detail=exc.description)
+    except web.HTTPError as exc:
+        if exc.text == "{}: {}".format(exc.status, exc.reason):
+            detail = HTTPStatus(exc.status).description
+        else:
+            detail = exc.text
+        response = problem(status=exc.status, title=exc.reason, detail=detail)
+    except (
+        web.HTTPException,  # eg raised HTTPRedirection or HTTPSuccessful
+        asyncio.CancelledError,  # skipped in default web_protocol
+    ):
+        # leave this to default handling in aiohttp.web_protocol.RequestHandler.start()
+        raise
+    except asyncio.TimeoutError as exc:
+        # overrides 504 from aiohttp.web_protocol.RequestHandler.start()
+        logger.debug('Request handler timed out.', exc_info=exc)
+        response = _generic_problem(HTTPStatus.GATEWAY_TIMEOUT, exc)
+    except Exception as exc:
+        # overrides 500 from aiohttp.web_protocol.RequestHandler.start()
+        logger.exception('Error handling request', exc_info=exc)
+        response = _generic_problem(HTTPStatus.INTERNAL_SERVER_ERROR, exc)
+
+    if isinstance(response, ConnexionResponse):
+        response = yield from AioHttpApi.get_response(response)
     return response
 
 
@@ -51,7 +101,7 @@ class AioHttpApi(AbstractAPI):
         )
         self.subapp = web.Application(
             middlewares=[
-                oauth_problem_middleware,
+                problems_middleware,
                 trailing_slash_redirect
             ]
         )

setup.py

@@ -51,6 +51,8 @@ tests_require = [
 ]
 
 if sys.version_info[0] >= 3:
+    if sys.version_info[1] <= 4:
+        aiohttp_require.append('httpstatus35')
     tests_require.extend(aiohttp_require)
     tests_require.append(ujson_require)
     tests_require.append('pytest-aiohttp')

tests/aiohttp/test_aiohttp_errors.py

@@ -0,0 +1,123 @@
+# coding: utf-8
+
+import asyncio
+import sys
+
+import pytest
+
+import aiohttp.test_utils
+from connexion import AioHttpApp
+from connexion.apis.aiohttp_api import HTTPStatus
+
+
+def is_valid_problem_json(json_body):
+    return all(key in json_body for key in ["type", "title", "detail", "status"])
+
+
+@pytest.fixture
+def aiohttp_app(problem_api_spec_dir):
+    app = AioHttpApp(__name__, port=5001,
+                     specification_dir=problem_api_spec_dir,
+                     debug=True)
+    options = {"validate_responses": True}
+    app.add_api('openapi.yaml', validate_responses=True, pass_context_arg_name='request_ctx', options=options)
+    return app
+
+
+@pytest.mark.skipif(sys.version_info < (3, 5), reason="In python3.4, aiohttp.ClientResponse.json() requires "
+                                                      "an exact content_type match in the content_types header, "
+                                                      "but, application/problem+json is not in there. "
+                                                      "Newer versions use a re.match to see if the content_type is "
+                                                      "json or not.")
+@asyncio.coroutine
+def test_aiohttp_problems(aiohttp_app, aiohttp_client):
+    # TODO: This is a based on test_errors.test_errors(). That should be refactored
+    #       so that it is parameterized for all web frameworks.
+    app_client = yield from aiohttp_client(aiohttp_app.app)  # type: aiohttp.test_utils.TestClient
+
+    greeting404 = yield from app_client.get('/v1.0/greeting')  # type: aiohttp.ClientResponse
+    assert greeting404.content_type == 'application/problem+json'
+    assert greeting404.status == 404
+    error404 = yield from greeting404.json()
+    assert is_valid_problem_json(error404)
+    assert error404['type'] == 'about:blank'
+    assert error404['title'] == 'Not Found'
+    assert error404['detail'] == HTTPStatus(404).description
+    assert error404['status'] == 404
+    assert 'instance' not in error404
+
+    get_greeting = yield from app_client.get('/v1.0/greeting/jsantos')  # type: aiohttp.ClientResponse
+    assert get_greeting.content_type == 'application/problem+json'
+    assert get_greeting.status == 405
+    error405 = yield from get_greeting.json()
+    assert is_valid_problem_json(error405)
+    assert error405['type'] == 'about:blank'
+    assert error405['title'] == 'Method Not Allowed'
+    assert error405['detail'] == HTTPStatus(405).description
+    assert error405['status'] == 405
+    assert 'instance' not in error405
+
+    get500 = yield from app_client.get('/v1.0/except')  # type: aiohttp.ClientResponse
+    assert get500.content_type == 'application/problem+json'
+    assert get500.status == 500
+    error500 = yield from get500.json()
+    assert is_valid_problem_json(error500)
+    assert error500['type'] == 'about:blank'
+    assert error500['title'] == 'Internal Server Error'
+    assert error500['detail'] == HTTPStatus(500).description
+    assert error500['status'] == 500
+    assert 'instance' not in error500
+
+    get_problem = yield from app_client.get('/v1.0/problem')  # type: aiohttp.ClientResponse
+    assert get_problem.content_type == 'application/problem+json'
+    assert get_problem.status == 418
+    assert get_problem.headers['x-Test-Header'] == 'In Test'
+    error_problem = yield from get_problem.json()
+    assert is_valid_problem_json(error_problem)
+    assert error_problem['type'] == 'http://www.example.com/error'
+    assert error_problem['title'] == 'Some Error'
+    assert error_problem['detail'] == 'Something went wrong somewhere'
+    assert error_problem['status'] == 418
+    assert error_problem['instance'] == 'instance1'
+
+    problematic_json = yield from app_client.get(
+        '/v1.0/json_response_with_undefined_value_to_serialize')  # type: aiohttp.ClientResponse
+    assert problematic_json.content_type == 'application/problem+json'
+    assert problematic_json.status == 500
+    problematic_json_body = yield from problematic_json.json()
+    assert is_valid_problem_json(problematic_json_body)
+
+    custom_problem = yield from app_client.get('/v1.0/customized_problem_response')  # type: aiohttp.ClientResponse
+    assert custom_problem.content_type == 'application/problem+json'
+    assert custom_problem.status == 403
+    problem_body = yield from custom_problem.json()
+    assert is_valid_problem_json(problem_body)
+    assert 'amount' in problem_body
+
+    problem_as_exception = yield from app_client.get('/v1.0/problem_exception_with_extra_args')  # type: aiohttp.ClientResponse
+    assert problem_as_exception.content_type == "application/problem+json"
+    assert problem_as_exception.status == 400
+    problem_as_exception_body = yield from problem_as_exception.json()
+    assert is_valid_problem_json(problem_as_exception_body)
+    assert 'age' in problem_as_exception_body
+    assert problem_as_exception_body['age'] == 30
+
+
+@pytest.mark.skip(reason="aiohttp_api.get_connexion_response uses _cast_body "
+                         "to stringify the dict directly instead of using json.dumps. "
+                         "This differs from flask usage, where there is no _cast_body.")
+@asyncio.coroutine
+def test_aiohttp_problem_with_text_content_type(aiohttp_app, aiohttp_client):
+    app_client = yield from aiohttp_client(aiohttp_app.app)  # type: aiohttp.test_utils.TestClient
+
+    get_problem2 = yield from app_client.get('/v1.0/other_problem')  # type: aiohttp.ClientResponse
+    assert get_problem2.content_type == 'application/problem+json'
+    assert get_problem2.status == 418
+    error_problem2 = yield from get_problem2.json()
+    assert is_valid_problem_json(error_problem2)
+    assert error_problem2['type'] == 'about:blank'
+    assert error_problem2['title'] == 'Some Error'
+    assert error_problem2['detail'] == 'Something went wrong somewhere'
+    assert error_problem2['status'] == 418
+    assert error_problem2['instance'] == 'instance1'
+

tox.ini

@@ -29,6 +29,7 @@ deps=pytest
 commands=
     pip install Requirements-Builder
     min: requirements-builder --level=min -o {toxworkdir}/requirements-min.txt setup.py
+    py34-min: pip install httpstatus35
     min: pip install -r {toxworkdir}/requirements-min.txt
     pypi: requirements-builder --level=pypi -o {toxworkdir}/requirements-pypi.txt setup.py
     pypi: pip install -r {toxworkdir}/requirements-pypi.txt