Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Converting response to raise a ProblemException #955

Merged
merged 9 commits into from
Oct 24, 2019
Merged
3 changes: 2 additions & 1 deletion connexion/apis/aiohttp_api.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,8 @@ def problems_middleware(request, handler):
try:
response = yield from handler(request)
except ProblemException as exc:
response = exc.to_problem()
response = problem(status=exc.status, detail=exc.detail, title=exc.title,
type=exc.type, instance=exc.instance, headers=exc.headers, ext=exc.ext)
except (werkzeug_HTTPException, _HttpNotFoundError) as exc:
response = problem(status=exc.code, title=exc.name, detail=exc.description)
except web.HTTPError as exc:
Expand Down
7 changes: 4 additions & 3 deletions connexion/apps/abstract.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
class AbstractApp(object):
def __init__(self, import_name, api_cls, port=None, specification_dir='',
host=None, server=None, arguments=None, auth_all_paths=False, debug=False,
resolver=None, options=None):
resolver=None, options=None, skip_error_handlers=False):
"""
:param import_name: the name of the application package
:type import_name: str
Expand Down Expand Up @@ -63,8 +63,9 @@ def __init__(self, import_name, api_cls, port=None, specification_dir='',

logger.debug('Specification directory: %s', self.specification_dir)

logger.debug('Setting error handlers')
self.set_errors_handlers()
if not skip_error_handlers:
logger.debug('Setting error handlers')
self.set_errors_handlers()
badcure marked this conversation as resolved.
Show resolved Hide resolved

@abc.abstractmethod
def create_app(self):
Expand Down
5 changes: 4 additions & 1 deletion connexion/apps/flask_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ def common_error_handler(exception):
:type exception: Exception
"""
if isinstance(exception, ProblemException):
response = exception.to_problem()
response = problem(
status=exception.status, title=exception.title, detail=exception.detail,
type=exception.type, instance=exception.instance, headers=exception.headers,
ext=exception.ext)
else:
if not isinstance(exception, werkzeug.exceptions.HTTPException):
exception = werkzeug.exceptions.InternalServerError()
Expand Down
5 changes: 4 additions & 1 deletion connexion/decorators/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import time

from werkzeug.exceptions import HTTPException

from connexion.exceptions import ProblemException
try:
import uwsgi_metrics
HAS_UWSGI_METRICS = True # pragma: no cover
Expand Down Expand Up @@ -40,6 +40,9 @@ def wrapper(*args, **kwargs):
except HTTPException as http_e:
status = http_e.code
raise http_e
except ProblemException as prob_e:
status = prob_e.status
raise prob_e
finally:
end_time_s = time.time()
delta_s = end_time_s - start_time_s
Expand Down
16 changes: 5 additions & 11 deletions connexion/decorators/response.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

from ..exceptions import (NonConformingResponseBody,
NonConformingResponseHeaders)
from ..problem import problem
from ..utils import all_json, has_coroutine
from .decorator import BaseDecorator
from .validation import ResponseBodyValidator
Expand Down Expand Up @@ -86,16 +85,11 @@ def __call__(self, function):
"""

def _wrapper(request, response):
try:
connexion_response = \
self.operation.api.get_connexion_response(response, self.mimetype)
self.validate_response(
connexion_response.body, connexion_response.status_code,
connexion_response.headers, request.url)

except (NonConformingResponseBody, NonConformingResponseHeaders) as e:
response = problem(500, e.reason, e.message)
return self.operation.api.get_response(response)
connexion_response = \
self.operation.api.get_connexion_response(response, self.mimetype)
self.validate_response(
connexion_response.body, connexion_response.status_code,
connexion_response.headers, request.url)

return response

Expand Down
39 changes: 13 additions & 26 deletions connexion/decorators/validation.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,9 @@
from jsonschema.validators import extend
from werkzeug.datastructures import FileStorage

from ..exceptions import ExtraParameterProblem
from ..exceptions import ExtraParameterProblem, BadRequestProblem, UnsupportedMediaTypeProblem
from ..http_facts import FORM_CONTENT_TYPES
from ..json_schema import Draft4RequestValidator, Draft4ResponseValidator
from ..problem import problem
from ..utils import all_json, boolean, is_json_mimetype, is_null, is_nullable

_jsonschema_3_or_newer = pkg_resources.parse_version(
Expand Down Expand Up @@ -148,22 +147,17 @@ def wrapper(request):

if ctype_is_json:
# Content-Type is json but actual body was not parsed
return problem(400,
"Bad Request",
"Request body is not valid JSON"
)
raise BadRequestProblem(detail="Request body is not valid JSON")
else:
# the body has contents that were not parsed as JSON
return problem(415,
"Unsupported Media Type",
raise UnsupportedMediaTypeProblem(
"Invalid Content-type ({content_type}), expected JSON data".format(
content_type=request.headers.get("Content-Type", "")
))

logger.debug("%s validating schema...", request.url)
error = self.validate_schema(data, request.url)
if error and not self.has_default:
return error
if data is not None or not self.has_default:
self.validate_schema(data, request.url)
elif self.consumes[0] in FORM_CONTENT_TYPES:
data = dict(request.form.items()) or (request.body if len(request.body) > 0 else {})
data.update(dict.fromkeys(request.files, '')) # validator expects string..
Expand All @@ -185,11 +179,9 @@ def wrapper(request):
errs += [str(e)]
print(errs)
if errs:
return problem(400, 'Bad Request', errs)
raise BadRequestProblem(detail=errs)

error = self.validate_schema(data, request.url)
if error:
return error
self.validate_schema(data, request.url)

response = function(request)
return response
Expand All @@ -207,7 +199,7 @@ def validate_schema(self, data, url):
logger.error("{url} validation error: {error}".format(url=url,
error=exception.message),
extra={'validator': 'body'})
return problem(400, 'Bad Request', str(exception.message))
raise BadRequestProblem(detail=str(exception.message))

return None

Expand Down Expand Up @@ -358,32 +350,27 @@ def wrapper(request):
for param in self.parameters.get('query', []):
error = self.validate_query_parameter(param, request)
if error:
response = problem(400, 'Bad Request', error)
return self.api.get_response(response)
raise BadRequestProblem(detail=error)

for param in self.parameters.get('path', []):
error = self.validate_path_parameter(param, request)
if error:
response = problem(400, 'Bad Request', error)
return self.api.get_response(response)
raise BadRequestProblem(detail=error)

for param in self.parameters.get('header', []):
error = self.validate_header_parameter(param, request)
if error:
response = problem(400, 'Bad Request', error)
return self.api.get_response(response)
raise BadRequestProblem(detail=error)

for param in self.parameters.get('cookie', []):
error = self.validate_cookie_parameter(param, request)
if error:
response = problem(400, 'Bad Request', error)
return self.api.get_response(response)
raise BadRequestProblem(detail=error)

for param in self.parameters.get('formData', []):
error = self.validate_formdata_parameter(param["name"], param, request)
if error:
response = problem(400, 'Bad Request', error)
return self.api.get_response(response)
raise BadRequestProblem(detail=error)

return function(request)

Expand Down
31 changes: 30 additions & 1 deletion connexion/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import warnings
from jsonschema.exceptions import ValidationError
from werkzeug.exceptions import Forbidden, Unauthorized

Expand All @@ -24,6 +25,9 @@ def __init__(self, status=400, title=None, detail=None, type=None,
self.ext = ext

def to_problem(self):
warnings.warn(
"'to_problem' is planned to be removed in a future release. "
"Call connexion.problem.problem(..) instead to maintain the existing error response.", DeprecationWarning)
return problem(status=self.status, title=self.title, detail=self.detail,
type=self.type, instance=self.instance, headers=self.headers,
ext=self.ext)
Expand Down Expand Up @@ -52,12 +56,13 @@ class InvalidSpecification(ConnexionException, ValidationError):
pass


class NonConformingResponse(ConnexionException):
class NonConformingResponse(ProblemException):
def __init__(self, reason='Unknown Reason', message=None):
"""
:param reason: Reason why the response did not conform to the specification
:type reason: str
"""
super(NonConformingResponse, self).__init__(status=500, title=reason, detail=message)
self.reason = reason
self.message = message

Expand All @@ -68,6 +73,30 @@ def __repr__(self): # pragma: no cover
return '<NonConformingResponse: {}>'.format(self.reason)


class AuthenticationProblem(ProblemException):

def __init__(self, status, title, detail):
super(AuthenticationProblem, self).__init__(status=status, title=title, detail=detail)


class ResolverProblem(ProblemException):

def __init__(self, status, title, detail):
super(ResolverProblem, self).__init__(status=status, title=title, detail=detail)


class BadRequestProblem(ProblemException):

def __init__(self, title='Bad Request', detail=None):
super(BadRequestProblem, self).__init__(status=400, title=title, detail=detail)


class UnsupportedMediaTypeProblem(ProblemException):

def __init__(self, title="Unsupported Media Type", detail=None):
super(UnsupportedMediaTypeProblem, self).__init__(status=415, title=title, detail=detail)


class NonConformingResponseBody(NonConformingResponse):
def __init__(self, message, reason="Response body does not conform to specification"):
super(NonConformingResponseBody, self).__init__(reason=reason, message=message)
Expand Down
8 changes: 3 additions & 5 deletions connexion/handlers.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging

from .operations.secure import SecureOperation
from .problem import problem
from .exceptions import AuthenticationProblem, ResolverProblem

logger = logging.getLogger('connexion.handlers')

Expand Down Expand Up @@ -45,12 +45,11 @@ def handle(self, *args, **kwargs):
"""
Actual handler for the execution after authentication.
"""
response = problem(
raise AuthenticationProblem(
badcure marked this conversation as resolved.
Show resolved Hide resolved
title=self.exception.name,
detail=self.exception.description,
status=self.exception.code
)
return self.api.get_response(response)


class ResolverErrorHandler(SecureOperation):
Expand All @@ -68,12 +67,11 @@ def function(self):
return self.handle

def handle(self, *args, **kwargs):
response = problem(
raise ResolverProblem(
title='Not Implemented',
detail=self.exception.reason,
status=self.status_code
)
return self.api.get_response(response)

@property
def operation_id(self):
Expand Down
1 change: 0 additions & 1 deletion connexion/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import functools
import importlib
import re

import six
import yaml
Expand Down
4 changes: 4 additions & 0 deletions requirements-all.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
-r requirements-aiohttp.txt
-r requirements-devel.txt
openapi_spec_validator
pytest-aiohttp
26 changes: 25 additions & 1 deletion tests/aiohttp/test_aiohttp_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ def aiohttp_app(problem_api_spec_dir):


@asyncio.coroutine
def test_aiohttp_problems(aiohttp_app, aiohttp_client):
def test_aiohttp_problems_404(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
Expand All @@ -38,6 +38,12 @@ def test_aiohttp_problems(aiohttp_app, aiohttp_client):
assert error404['status'] == 404
assert 'instance' not in error404

@asyncio.coroutine
def test_aiohttp_problems_405(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

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
Expand All @@ -49,6 +55,12 @@ def test_aiohttp_problems(aiohttp_app, aiohttp_client):
assert error405['status'] == 405
assert 'instance' not in error405

@asyncio.coroutine
def test_aiohttp_problems_500(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

get500 = yield from app_client.get('/v1.0/except') # type: aiohttp.ClientResponse
assert get500.content_type == 'application/problem+json'
assert get500.status == 500
Expand All @@ -60,6 +72,12 @@ def test_aiohttp_problems(aiohttp_app, aiohttp_client):
assert error500['status'] == 500
assert 'instance' not in error500

@asyncio.coroutine
def test_aiohttp_problems_418(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

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
Expand All @@ -72,6 +90,12 @@ def test_aiohttp_problems(aiohttp_app, aiohttp_client):
assert error_problem['status'] == 418
assert error_problem['instance'] == 'instance1'

@asyncio.coroutine
def test_aiohttp_problems_misc(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

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'
Expand Down
Loading