Merge pull request #383 from zalando/fix-problem-expt

Problem as exceptions should convert properly to problem responses

connexion/app.py

@@ -4,11 +4,11 @@
 import flask
 import werkzeug.exceptions
 
-from connexion.decorators.produces import JSONEncoder as ConnexionJSONEncoder
-from connexion.resolver import Resolver
-
 from .api import Api
+from .decorators.produces import JSONEncoder as ConnexionJSONEncoder
+from .exceptions import ProblemException
 from .problem import problem
+from .resolver import Resolver
 
 logger = logging.getLogger('connexion.app')
 
@@ -85,11 +85,15 @@ def common_error_handler(exception):
         """
         :type exception: Exception
         """
-        if not isinstance(exception, werkzeug.exceptions.HTTPException):
-            exception = werkzeug.exceptions.InternalServerError()
+        if isinstance(exception, ProblemException):
+            response_container = exception.to_problem()
+        else:
+            if not isinstance(exception, werkzeug.exceptions.HTTPException):
+                exception = werkzeug.exceptions.InternalServerError()
+
+            response_container = problem(title=exception.name, detail=exception.description,
+                                         status=exception.code)
 
-        problem_title = getattr(exception, 'title', exception.name)
-        response_container = problem(title=problem_title, detail=exception.description, status=exception.code)
         return response_container.flask_response_object()
 
     def add_api(self, specification, base_path=None, arguments=None, auth_all_paths=None, swagger_json=None,

connexion/exceptions.py

@@ -1,21 +1,31 @@
-from werkzeug.exceptions import (BadRequest, Forbidden, HTTPException,
-                                 Unauthorized)
+from werkzeug.exceptions import Forbidden, Unauthorized
+
+from .problem import problem
 
 
 class ConnexionException(Exception):
     pass
 
 
-class ProblemException(ConnexionException, HTTPException):
-    def __init__(self, title=None, description=None, response=None):
+class ProblemException(ConnexionException):
+    def __init__(self, status=400, title=None, detail=None, type=None,
+                 instance=None, headers=None, ext=None):
         """
-        :param title: Title of the problem.
-        :type title: str
+        This exception is holds arguments that are going to be passed to the
+        `connexion.problem` function to generate a propert response.
         """
-        ConnexionException.__init__(self)
-        HTTPException.__init__(self, description, response)
+        self.status = status
+        self.title = title
+        self.detail = detail
+        self.type = type
+        self.instance = instance
+        self.headers = headers
+        self.ext = ext
 
-        self.title = title or self.name
+    def to_problem(self):
+        return problem(status=self.status, title=self.title, detail=self.detail,
+                       type=self.type, instance=self.instance, headers=self.headers,
+                       ext=self.ext)
 
 
 class ResolverError(LookupError):
@@ -78,38 +88,37 @@ def __init__(self, message, reason="Response headers do not conform to specifica
         super(NonConformingResponseHeaders, self).__init__(reason=reason, message=message)
 
 
-class OAuthProblem(ProblemException, Unauthorized):
-    def __init__(self, title=None, **kwargs):
-        super(OAuthProblem, self).__init__(title=title, **kwargs)
+class OAuthProblem(Unauthorized):
+    pass
 
 
-class OAuthResponseProblem(ProblemException, Unauthorized):
-    def __init__(self, token_response, title=None, **kwargs):
+class OAuthResponseProblem(Unauthorized):
+    def __init__(self, token_response, **kwargs):
         self.token_response = token_response
-        super(OAuthResponseProblem, self).__init__(title=title, **kwargs)
+        super(OAuthResponseProblem, self).__init__(**kwargs)
 
 
-class OAuthScopeProblem(ProblemException, Forbidden):
-    def __init__(self, token_scopes, required_scopes, title=None, **kwargs):
+class OAuthScopeProblem(Forbidden):
+    def __init__(self, token_scopes, required_scopes, **kwargs):
         self.required_scopes = required_scopes
         self.token_scopes = token_scopes
         self.missing_scopes = required_scopes - token_scopes
 
-        super(OAuthScopeProblem, self).__init__(title=title, **kwargs)
+        super(OAuthScopeProblem, self).__init__(**kwargs)
 
 
-class ExtraParameterProblem(ProblemException, BadRequest):
-    def __init__(self, formdata_parameters, query_parameters, title=None, description=None, **kwargs):
+class ExtraParameterProblem(ProblemException):
+    def __init__(self, formdata_parameters, query_parameters, title=None, detail=None, **kwargs):
         self.extra_formdata = formdata_parameters
         self.extra_query = query_parameters
 
         # This keep backwards compatibility with the old returns
-        if description is None:
+        if detail is None:
             if self.extra_query:
-                description = "Extra {parameter_type} parameter(s) {extra_params} not in spec"\
+                detail = "Extra {parameter_type} parameter(s) {extra_params} not in spec"\
                     .format(parameter_type='query', extra_params=', '.join(self.extra_query))
             elif self.extra_formdata:
-                description = "Extra {parameter_type} parameter(s) {extra_params} not in spec"\
+                detail = "Extra {parameter_type} parameter(s) {extra_params} not in spec"\
                     .format(parameter_type='formData', extra_params=', '.join(self.extra_formdata))
 
-        super(ExtraParameterProblem, self).__init__(title=title, description=description, **kwargs)
+        super(ExtraParameterProblem, self).__init__(title=title, detail=detail, **kwargs)

tests/api/test_errors.py

@@ -66,3 +66,9 @@ def test_errors(problem_app):
     problem_body = json.loads(custom_problem.data.decode('utf-8'))
     assert 'amount' in problem_body
     assert problem_body['amount'] == 23.
+
+    problem_as_exception = app_client.get('/v1.0/problem_exception_with_extra_args')
+    assert problem_as_exception.status_code == 400
+    problem_as_exception_body = json.loads(problem_as_exception.data.decode('utf-8'))
+    assert 'age' in problem_as_exception_body
+    assert problem_as_exception_body['age'] == 30

tests/fakeapi/hello.py

@@ -2,7 +2,7 @@
 
 from flask import redirect
 
-from connexion import NoContent, problem, request
+from connexion import NoContent, ProblemException, problem, request
 
 
 class DummyClass(object):
@@ -13,7 +13,7 @@ def test_classmethod(cls):
     def test_method(self):
         return self.__class__.__name__
 
-class_instance = DummyClass()
+class_instance = DummyClass()  # noqa
 
 
 def get():
@@ -348,6 +348,14 @@ def get_custom_problem_response():
                    ext={'amount': 23.0})
 
 
+def throw_problem_exception():
+    raise ProblemException(
+        title="As Exception",
+        detail="Something wrong or not!",
+        ext={'age': 30}
+    )
+
+
 def unordered_params_response(first, path_param, second):
     return dict(first=int(first), path_param=str(path_param), second=int(second))
 

tests/fixtures/problem/swagger.yaml

@@ -91,3 +91,13 @@ paths:
       responses:
         200:
           description: Custom problem response
+
+  /problem_exception_with_extra_args:
+    get:
+      description: Using problem as exception
+      operationId: fakeapi.hello.throw_problem_exception
+      produces:
+        - application/json
+      responses:
+        200:
+          description: Problem exception