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

Returning JSON null value from API handler inconsistent #1219

Closed
Budovi opened this issue Apr 18, 2020 · 2 comments
Closed

Returning JSON null value from API handler inconsistent #1219

Budovi opened this issue Apr 18, 2020 · 2 comments

Comments

@Budovi
Copy link

Budovi commented Apr 18, 2020

Description

A single JSON value (literal, i.e. string, number, true, false, null) is considered a valid JSON document. I have an endpoint returning nullable number (either a number or null) and I return None from the API handler method in applicable cases:

def api_handler(...)
   ...
   return None, 200

Expected behaviour

The serialization engine handles this situation and produces a "null" JSON document.

I can confirm that the used JSON serializer handles this situation correctly, the issue being it is not called at all in this case.

Actual behaviour

An empty response is generated which immediately fails the response validation step (and would be a wrong response anyway).

Implementation and workaround notes

The current handling of the returned tuples is, afaik, the following:

  • Since the response is neither of the type ConnexionResponse, nor one of the framework responses, it is handled by AbstractAPI._response_from_handler (called from here).
  • Tuples of lenght two are then unpacked and None is passed for argument data to the AbstractAPI._build_response.
  • From both Flask and AioHttp implementations, AbstractAPI._prepare_body_and_status_code is called with None data.
  • AbstractAPI._serialize_data is only called in case the data is not None.

From the context it is clear that the None has an "empty body" semantics. From the top of my head an emtpy string would suit the purpose better, but it seems that this would break other code paths and in turn it may break users' codes.

A different approach would be to add logic that would check for a combination status_code == 200 and data == None or even include a check for a mimetype, but feels like an ugly hack. Or better, to remove the "premature conversion" of the NoContent object and by doing so enforce users to use this special class to return empty messages, and check whether the rest of the library is consistent in this matter.

A workaround in my case could be to return ConnexionResponse instance with status_code=200, mimetype='application/json' and body='null'. This is poorly documented; I'm not sure how content_type is used and whether I need to call some "fill up the response headers properly" code somewhere.

Steps to reproduce

  1. Create an API that returns a single JSON nullable object.
  2. Set validate_responses=True while adding API to the connexion, if you want.
  3. Return None along with 200 status code from the corresponding API handler.

Additional info

  • Python interpreter version: 3.6.9
  • Connexion version: 2.6.0
@Budovi
Copy link
Author

Budovi commented Apr 18, 2020

So I've tried the mentioned workaround with ConnexionResponse and it also fails, because the body is converted to a string (and a quoted "null" is returned in the answer, which is an invalid response that also fails the validation step in this case). Not sure if this is working as intended or a separate bug.

A working workaround seems to be to return Response(status=200, mimetype='application/json', response='null') from flask directly. I don't like this since it assumes the use of Flask as an underlying library.

@RobbeSneyders
Copy link
Member

This part of the code base was reworked for Connexion 3, which will be released tomorrow. Feel free to reopen including a reproducible example if this is still an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants