hey-api / openapi-ts

✨ Turn your OpenAPI specification into a beautiful TypeScript client
https://heyapi.vercel.app
Other
969 stars 77 forks source link

Handling Error Responses in API with OpenAPI Specification #812

Open sagardwivedi opened 1 month ago

sagardwivedi commented 1 month ago

Description

I wrote a login API, and when I enter the wrong password, it sends back a status code 400 along with a message. However, I know that the OpenAPI specification doesn't directly include the raise HTTPException feature. Is there a workaround for this?

Problem Details

Example Code

Here is a simplified example of the current implementation:

from fastapi import FastAPI, HTTPException

app = FastAPI()

@router.post("/login/access-token")
def login_access_token(
    session: SessionDep,
    form_data: Annotated[OAuth2PasswordRequestForm, Depends()],
) -> Token:
    """
    Get an access token for authentication.

    This endpoint authenticates a user's credentials and returns an access token
    if the credentials are valid.

    - `session`: SQLModel database session dependency.
    - `form_data`: OAuth2PasswordRequestForm object containing username and password.

    ## Returns:
    - `Token`: A Token object containing the access token and token type.

    ## Raises:
    - `HTTPException`: If the provided username or password is incorrect.
    """
    user = authenticate(
        session=session, username=form_data.username, password=form_data.password
    )
    if not user:
        raise HTTPException(
            status_code=status.HTTP_400_BAD_REQUEST,
            detail="Incorrect email or password",
        )
    access_token_expires = timedelta(minutes=settings.ACCESS_TOKEN_EXPIRE_MINUTES)
    access_token = create_access_token(user.user_id, access_token_expires)
    return Token(access_token=access_token, token_type="Bearer")

Desired Outcome

To have the OpenAPI documentation accurately reflect the API's behavior, including the handling of incorrect passwords and the resulting status code 400 response.

Question

Is there a workaround to properly document this behavior in the OpenAPI specification?

Reproducible example or configuration

No response

OpenAPI specification (optional)

"responses": {
          "200": {
            "description": "Successful Response",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/Token"
                }
              }
            }
          },
          "422": {
            "description": "Validation Error",
            "content": {
              "application/json": {
                "schema": {
                  "$ref": "#/components/schemas/HTTPValidationError"
                }
              }
            }
          }
        }

System information (optional)

No response

mrlubos commented 1 month ago

Yes @sagardwivedi, but that is a FastAPI question. Had the OpenAPI specification been correct, then it would be an issue with this package

sagardwivedi commented 1 month ago

Okay, I will raise the question with FastAPI. @mrlubos, thank you for your response!

sagardwivedi commented 1 month ago

Should I close the issue here?

sagardwivedi commented 1 month ago

I am trying to custom set the error message if i get the response 400

if (response.status === 400) {
      return setError(error?.detail)
}

but i am getting (property) detail?: ValidationError[] | undefined in error?.detail but I am getting from the backend

image

mrlubos commented 1 month ago

It doesn't matter what is returned from the backend. OpenAPI specification is the contract describing the API. It's up to the server to then value that contract. In your case, there are scenarios you're not describing in the contract as you can see from your spec

IgorKha commented 1 month ago

same problem.... @sagardwivedi Will you bring up the FastAPI issue? Please provide a link

sagardwivedi commented 1 month ago

@IgorKha https://github.com/fastapi/fastapi/discussions/11882#discussion-6973409

IgorKha commented 1 month ago

I think it would be correct to always return the server's response status code in the case of an error. This makes more sense, as it provides the actual response status from the server.

sagardwivedi commented 1 month ago

@IgorKha I understand your point, but after thoroughly checking the FastAPI documentation, I couldn't find an alternative method. The @hey-api/openapi-ts library relies on openapi.json to generate client, and unfortunately, the openapi.json doesn't include the raise HTTPException.

IgorKha commented 1 month ago

Yes, I understand; I read the discussion about FastAPI. I realize this is an issue specific to their implementation. However, I would still like to ask the author of this library to consider providing an option to return the actual server response status code in case of an error, regardless of what's in the OpenAPI specification. In my opinion, this would be quite versatile and convenient for everyone. Thank you :)

mrlubos commented 1 month ago

@IgorKha if you're using the new standalone client packages, you have access to the raw response object. Is that what you need?

IgorKha commented 1 month ago

I use @hey-api/client-axios, but I also tested @hey-api/client-fetch. Indeed, the raw response contains the server status, but the issue is that you have to remember this additional step while working. This workaround does address the problem, though it's not as native as I'd like. It seems I'll have to adjust the backend 🥲

Anyway, thank you for the great tool! 😄

mrlubos commented 1 month ago

@IgorKha feel free to propose a different implementation!