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

[BUG][Python] Pydantic 2 generated client does not properly handle "204 No Content" if it is the first response in a path -- empty _response_types_map #16967

Closed
5 tasks done
BrianMcBrayer opened this issue Nov 2, 2023 · 2 comments · Fixed by #17038

Comments

@BrianMcBrayer
Copy link

BrianMcBrayer commented Nov 2, 2023

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
Description

When using a valid "204 No Content" response as the first response to a path, none of the responses for that path will be included in the _response_types_map for that endpoint.

Please see the provided spec for a reproducible example.

If I have an endpoint /will-fail that returns (in this order):

  • 204 No Content
  • 401 Some Content
  • 404 Some Content

None of the responses will be correctly handled by the generated python client.

One issue this causes:
When I get a 401, an open_api.exceptions.Unauthorized exception is thrown instead of getting my spec's 401 response.

openapi-generator version

latest as of 2023-11-01 and 2023-11-02. This also happens in v7.0.0 and v7.0.1

OpenAPI declaration file content or url
openapi: 3.0.0
info:
  version: 1.0.0
  title: Sample API
  description: A sample API to illustrate 204 No Content issue
paths:
  /will-not-fail:
    delete:
      description: Returns nothing
      responses:
        "200":
          description: Successful response
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/SuccessMessage"
        "404":
          description: Not found
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/NotFound"
  /will-fail:
    delete:
      description: Returns nothing
      responses:
        "204":
          description: Successful response
        "404":
          description: Not found
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/NotFound"
  /will-strangely-not-fail:
    delete:
      description: Returns nothing
      responses:
        "200":
          description: Successful response
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/SuccessMessage"
        "204":
          description: Successful response
        "404":
          description: Not found
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/NotFound"
components:
  schemas:
    NotFound:
      type: object
      properties:
        message:
          type: string
          example: Not found
    SuccessMessage:
      type: object
      properties:
        message:
          type: string
          example: Success

To generate, I'm using something like the following:

rm -rf out/python

docker run --rm -v "${PWD}:/local" openapitools/openapi-generator-cli generate \
    -i "/local/sample-spec.yaml" \
    -g python \
    -o /local/out/python
Steps to reproduce

If you run the generator on the spec and look at default_api.py, you will see that for the will-fail path, it has no responses in _response_types_map, which causes problems. If you look at the will-strangely-not-fail and will-not-fail paths, there are the proper responses in _response_types_map even though will-strangely-not-fail also has a 204 No Content response.

Suggest a fix

I will see if I can fix this myself, but I cannot guarantee it. Thank you all for all of your hard work.

@BrianMcBrayer BrianMcBrayer changed the title [BUG][Python] Pydantic 2 generated client does not properly handle "204 No Content" -- empty _response_types_map [BUG][Python] Pydantic 2 generated client does not properly handle "204 No Content" if it is the first response in a path -- empty _response_types_map Nov 2, 2023
@robertschweizer
Copy link
Contributor

robertschweizer commented Nov 7, 2023

We just merged #16999, which stops deserializing the response content on error status codes.

I believe exceptions must be raised on error status codes, but bodies should be deserialized and added to the exception object if possible.

Independent of error responses, I suspect your issue also appears if you have 204 as the first documented response status, and maybe 206 after that with a schema? For successful responses, the client should already deserialize all classes correctly. If not, that's a bug.

@BrianMcBrayer
Copy link
Author

BrianMcBrayer commented Nov 8, 2023

I am not sure what I think of #16999 (though I'm sure a lot of thought went into it).

My OpenAPI spec has detailed responses for each of the error codes, and I'd like to just get them back as regular requests instead of writing exception handlers. Had to rewrite a bunch of test cases that started failing over night 😆

However, if this behavior going forward is stable and unlikely to change, I can get behind it.

For 204's or anything without a schema -- yes, it will happen regardless. If I have a 206 or something and I want the client's pydantic schema validation to happen, it will not happen if the 204 is first.

EDIT: Just to be clear, thank you SO much for all your work on this project.

robertschweizer added a commit to robertschweizer/openapi-generator that referenced this issue Nov 11, 2023
robertschweizer added a commit to robertschweizer/openapi-generator that referenced this issue Nov 11, 2023
wing328 pushed a commit that referenced this issue Nov 15, 2023
* refactor: Clean up _response_types_map formatting

It matches black's behavior of having trailing commas now.

* test: Add test to reproduce #16967

* fix: deserialize responses even if no returnType

Closes #16967

* refactor: Simplify ApiException subclasses

* refactor: Move exception subtype choice to ApiException

* feat: Deserialize error responses and add to exceptions

* test: Add for error responses with model
lwj5 pushed a commit to lwj5/openapi-generator that referenced this issue Dec 5, 2023
* refactor: Clean up _response_types_map formatting

It matches black's behavior of having trailing commas now.

* test: Add test to reproduce OpenAPITools#16967

* fix: deserialize responses even if no returnType

Closes OpenAPITools#16967

* refactor: Simplify ApiException subclasses

* refactor: Move exception subtype choice to ApiException

* feat: Deserialize error responses and add to exceptions

* test: Add for error responses with model
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants