[BUG][PHP] 204 response causes exception when path also has 200 response

jhogervorst commented 5 months ago

Bug Report Checklist

[x] Have you provided a full/minimal spec to reproduce the issue?
[x] Have you validated the input using an OpenAPI validator (example)?
[x] Have you tested with the latest master to confirm the issue still exists?
[x] Have you searched for related issues/PRs?
[x] What's the actual output vs expected output?
[ ] [Optional] Sponsorship to speed up the bug fix or feature request (example)

Description

When I have a path that can either return a 200 response (with some JSON body) or a 204 response (with no content), an exception is thrown when the 204 response is received by the generated PHP API client, even though it's a valid (according to my API schema) and successful response.

openapi-generator version

7.7.0 (latest)

OpenAPI declaration file content or url

openapi: 3.0.0
paths:
  "/pet":
    patch:
      responses:
        "200":
          description: Pet updated
          content:
            application/json:
              schema:
                type: object
                properties:
                  id:
                    type: integer
        "204":
          description: Pet deleted
        "404":
          description: Pet not found

Generation Details

docker run --rm -v "${PWD}:/local" openapitools/openapi-generator-cli generate \
  -i https://gist.github.com/jhogervorst/3e56b05c092a8b0845dc765471928892/raw/8a7f3bc982d34077861b489f6d31b446756dccaf/patch-200-204.yaml \
  -g php \
  -o /local/out/php

Steps to reproduce

Call the patch method on the generated API client, while getting a 204 response:

<?php

require 'vendor/autoload.php';

$config = new OpenAPI\Client\Configuration();
$config->setHost('https://httpstat.us/204'); // returns 204 for any request

$api = new OpenAPI\Client\Api\DefaultApi(config: $config);

$api->petPatch();

Expected output

I would expect this to return without errors, since the 204 response is valid (according to my API schema) and successful (HTTP

Actual output

Fatal error: Uncaught OpenAPI\Client\ApiException: Error JSON decoding server response (https://httpstat.us/204/pet) in lib/Api/DefaultApi.php:227
Stack trace:
#0 lib/Api/DefaultApi.php(136): OpenAPI\Client\Api\DefaultApi->petPatchWithHttpInfo('application/jso...')
#1 test.php(10): OpenAPI\Client\Api\DefaultApi->petPatch()
#2 {main}
  thrown in lib/Api/DefaultApi.php on line 227

Related issues/PRs

https://github.com/OpenAPITools/openapi-generator/issues/125

Suggest a fix

The problem seems that the generated switch statement only contains a case 200 (for the 200 response). The 204 response is not handled here.

As a result, later code tries to parse the response as JSON, but that causes an exception, since the response is empty (which is expected for 204).

I think the generated switch should include a case 204 to handle this situation properly?

wing328 commented 5 months ago

I think the generated switch should include a case 204 to handle this situation properly?

yes, would you have time to contribute a PR to fix it? I can show you some good starting points.

we also have a php-nextgen client generator with some improvements and refactoring that you can give it a try (my guess is that likely multi response code is not yet handled)

jhogervorst commented 5 months ago

we also have a php-nextgen client generator with some improvements and refactoring that you can give it a try (my guess is that likely multi response code is not yet handled)

@wing328 That's interesting! Didn't know about it. Where can I read more about it? Like, is it stable, or still in development/beta phase?

I think the generated switch should include a case 204 to handle this situation properly?

yes, would you have time to contribute a PR to fix it? I can show you some good starting points.

Yes, we could take a look at it, either for the php-nextgen version or the 'old' one. Some starting points would be great, since we're not familiar with the code of the generator itself.

bardkalbakk commented 1 month ago

FYI, this issue is also present in php-nextgen. It runs json_decode on the empty response even if http status code is 204, which results in an exception

OpenAPITools / openapi-generator