Converting response to raise a ProblemException

[badcure]

The goal of this to convert all possible error responses to be a raised ProblemException subclass instead. Also for flask, allow for the option to skip the error handling.

The goal is to allow the end application handle the exceptions, as well as customize the response that aligns with the application's payload structure.

[cognifloyd]

Left a comment on why you're getting the aiohttp error in tests.

The following errors are not related to aiohttp, and I don't know why your change is triggering them.
Apparently the default object in the spec is not replacing the None body.
test_default_object_body[swagger.yaml]
test_default_object_body[openapi.yaml]

[badcure]

Side note: In reviewing some of the errors "test_default_object_body", I plan on removing those tests.

While I have no idea how it was passing before, reviewing json-schema/json-schema#5 seems to suggest that default should NOT be applied when something is required. Below is the exception I am seeing for this case:

Traceback (most recent call last):
  File "/connexion/connexion/decorators/validation.py", line 178, in validate_schema
    self.validator.validate(data)
  File "/connexion/venv/py27/lib/python2.7/site-packages/jsonschema/validators.py", line 123, in validate
    raise error
ValidationError: None is not of type 'object'

Failed validating 'type' in schema:
    {'components': {'examples': {'justAnExample': {'summary': 'a basic example.',
                                                   'value': 'Good evening, doctor.'}},
                    'parameters': {'OptionalName': {'description': 'Name of the person to greet.',
                                                    'in': 'path',
                                                    'name': 'name',
                                                    'required': False,
                                                    'schema': {'type': 'string'}}},
                    'requestBodies': {'fakeapi.hello.test_body_sanitization_body': {'content': {'application/json': {'schema': {'components': <Recursion on dict with id=140022751344632>,
                                                                                                                                'properties': {'body1': {'type': 'string'},
                                                                                                                                               'body2': {'type': 'string'}},
                                                                                                                                'type': 'object'}}},
                                                                                    'description': 'Just a testing parameter in the body',
                                                                                    'required': True}},
                    'schemas': {'new_stack': {'properties': {'image_version': {'description': 'Docker image version to deploy',
                                                                               'type': 'string'}},
                                              'required': ['image_version'],
                                              'type': 'object'}}},
     'default': {'image_version': 'default_image'},
     'properties': {'image_version': {'description': 'Docker image version to deploy',
                                      'type': 'string'}},
     'required': ['image_version'],
     'type': 'object',
     'x-body-name': 'stack',
     'x-scope': ['']}

[cognifloyd]

I would love to see this merged so that core connexion raises ProblemException everywhere instead of returning a problem response. That feels much cleaner and more consistent.

I have a few comments about how this might move the PR forward.

[phillipmadlan]

Any update on this PR and possibility of merging @cognifloyd? Thanks!

[cognifloyd]

I do not work for Zalando, so I can't merge any PRs here.
But, I would love to see this included, so I've been reviewing trying to help move it along so that when the Zalando people have some time, they can just merge it.

To that end, this PR has a merge conflict since #952 was merged. I would recommend rebasing.

[ConstantinoSchillebeeckxSimpleRose]

Any movement on this, I'd love to see it merged?

[hjacobs]

Could you rebase the PR and fix the conflicts? We will take a look afterwards.

[ConstantinoSchillebeeckxSimpleRose]

All set.

[badcure]

@ConstantinoSchillebeeckxSimpleRose and @hjacobs Sorry for the MIA-ness. I have updated the PR with the changes. Let me know what you think

[cognifloyd]

There's the note I left about deprecating ProblemException.to_problem, and a couple of questions about tests changes.

[hjacobs]

See comments

[cognifloyd]

LGTM 👍

[hjacobs]

👍