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][PHP] Code unreachable for responses other than 2xx and bad return types #7788

Closed
deluxetom opened this issue Oct 22, 2020 · 3 comments
Closed

Comments

@deluxetom
Copy link

deluxetom commented Oct 22, 2020

Description

The generated PHP code (case 400:) is unreachable when a response with HTTP code 400 is defined. It adds that return type to the PHPdoc but it's never returned as the RequestException is caught for anything different than a 2xx.
Tools like Psalm will expect a return type that will never be returned.

/**
 * Operation registerWithHttpInfo
 *
 * Add a User
 *
 * @param  \Eplay\ApiClient\Model\UserForm $userForm User form (required)
 *
 * @throws \Eplay\ApiClient\ApiException on non-2xx response
 * @throws \InvalidArgumentException
 * @return array of \Eplay\ApiClient\Model\User|\Eplay\ApiClient\Model\FormValidationErrorResponse, HTTP status code, HTTP response headers (array of strings)
 */
public function registerWithHttpInfo($userForm)
{
    $request = $this->registerRequest($userForm);

    try {
        $options = $this->createHttpClientOption();
        try {
            $response = $this->client->send($request, $options);
        } catch (RequestException $e) {
            throw new ApiException(
                "[{$e->getCode()}] {$e->getMessage()}",
                $e->getCode(),
                $e->getResponse() ? $e->getResponse()->getHeaders() : null,
                $e->getResponse() ? (string) $e->getResponse()->getBody() : null
            );
        }

        $statusCode = $response->getStatusCode();

        if ($statusCode < 200 || $statusCode > 299) {
            throw new ApiException(
                sprintf(
                    '[%d] Error connecting to the API (%s)',
                    $statusCode,
                    $request->getUri()
                ),
                $statusCode,
                $response->getHeaders(),
                $response->getBody()
            );
        }

        $responseBody = $response->getBody();
        switch($statusCode) {
            case 200:
                if ('\Eplay\ApiClient\Model\User' === '\SplFileObject') {
                    $content = $responseBody; //stream goes to serializer
                } else {
                    $content = (string) $responseBody;
                }

                return [
                    ObjectSerializer::deserialize($content, '\Eplay\ApiClient\Model\User', []),
                    $response->getStatusCode(),
                    $response->getHeaders()
                ];
            case 400:
                if ('\Eplay\ApiClient\Model\FormValidationErrorResponse' === '\SplFileObject') {
                    $content = $responseBody; //stream goes to serializer
                } else {
                    $content = (string) $responseBody;
                }

                return [
                    ObjectSerializer::deserialize($content, '\Eplay\ApiClient\Model\FormValidationErrorResponse', []),
                    $response->getStatusCode(),
                    $response->getHeaders()
                ];
        }
...
}
openapi-generator version

v5.0.0-beta2

OpenAPI declaration file content or url
"/user/{user}": {
    "put": {
        "tags": [
            "user"
        ],
        "summary": "Edit a user",
        "description": "Edit a user",
        "operationId": "edit",
        "parameters": [
            {
                "name": "user",
                "in": "path",
                "description": "User ID",
                "required": true,
                "schema": {
                    "type": "string"
                }
            }
        ],
        "requestBody": {
            "description": "User update form",
            "required": true,
            "content": {
                "application/json": {
                    "schema": {
                        "$ref": "#/components/schemas/UserUpdateForm"
                    }
                }
            }
        },
        "responses": {
            "200": {
                "description": "Returns the user on success",
                "content": {
                    "application/json": {
                        "schema": {
                            "$ref": "#/components/schemas/User"
                        }
                    }
                }
            },
            "400": {
                "description": "Returns a 400 on validation failure",
                "content": {
                    "application/json": {
                        "schema": {
                            "$ref": "#/components/schemas/FormValidationErrorResponse"
                        }
                    }
                }
            }
        }
    },
},
Generation Details
docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli:v5.0.0-beta2 generate \
     		-i /local/api.spec.json \
     		-g php \
     		-c /local/terraform/openapi-clients/php/config.json \
     		-o /local/tmp-client \
     		--skip-validate-spec

config.json

{
    "invokerPackage": "Eplay\\ApiClient",
    "srcBasePath": "lib",
    "phpLegacySupport": "false",
    "variableNamingConvention": "camelCase"
}
Steps to reproduce

Generate the client with a model defined for a response different than 2xx

Related issues/PRs
Suggest a fix
  • Don't add the return type for any response types other than 2xx
  • Don't deserialize responses other than 2xx
@deluxetom deluxetom changed the title [BUG] PHP Code unreachable for responses other than 2xx and bad return types [BUG][PHP] Code unreachable for responses other than 2xx and bad return types Oct 22, 2020
@archadamaster
Copy link

archadamaster commented Jan 18, 2022

Any update on this?
There is no way to catch correct response other that 2xx

Current solution is awkward

catch (\Throwable $ex)
        {
            $errorObject = $ex->getResponseObject();
            if ($errorObject instanceof \TTPrices\OpenAPI\Account\Model\JsonApiError) {
                die($errorObject->getErrors()[0]);
            } 

@kruegge82
Copy link
Contributor

same problem, have commented out:

        if ($statusCode < 200 || $statusCode > 299) {
            throw new ApiException(
                sprintf(
                    '[%d] Error connecting to the API (%s)',
                    $statusCode,
                    $request->getUri()
                ),
                $statusCode,
                $response->getHeaders(),
                $response->getBody()
            );
        }

and also on $apiInstance put off http errors off:
$apiInstance = new OPENApi\Client\Api\Class(new GuzzleHttp\Client(['http_errors'=>false]), $config);

wing328 pushed a commit that referenced this issue Oct 7, 2024
…ue 7788 (#19483)

* in case of defined status codes > 299 switch will have no effect. As described in issue 7788

so we get only an error if statusCode is not defined AND not between 200 and 299

#7788

* in case of defined status codes > 299 switch will have no effect. As described in issue 7788

so we get only an error if statusCode is not defined AND not between 200 and 299

#7788
@wing328
Copy link
Member

wing328 commented Oct 7, 2024

closed via #19483

thanks for the PR by @kruegge82

@wing328 wing328 closed this as completed Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants