swagger icon indicating copy to clipboard operation
swagger copied to clipboard

Adding @ApiFoundResponse on a dynamic redirect method still renders a 200 response code in the OpenAPI contract

Open adworacz opened this issue 2 years ago • 8 comments

Is there an existing issue for this?

  • [X] I have searched the existing issues

Current behavior

It appears that the Swagger CLI is adding an extra 200 response code to a method that will never return a 200 response. Specifically, for a method that always returns a dynamic redirect that's been labeled with @ApiFoundResponse

Example:

  interface RedirectResponse {
    url: string
    statusCode?: number
  }

  @Get()
  @ApiFoundResponse({ description: 'Redirects to a URL' })
  @Redirect()
  getRedirect(): RedirectResponse {
    return {
      url: `https://example.com`,
      statusCode: HttpStatus.FOUND,
    }
  }

Which produces:

  "/example": {
      "get": {
        ...
        "responses": {
          "200": {
            "description": "",
            "content": {
              "application/json": {
                "schema": {
                  "type": "object"
                }
              }
            }
          },
          "302": {
            "description": "Redirects to a URL"
          }
        }
      }

Minimum reproduction code

See above.

Steps to reproduce

See above.

Expected behavior

Only a single response is rendered - no 200 for a redirect.

Package version

5.1.3

NestJS version

8.0.11

Node.js version

14

In which operating systems have you tested?

  • [ ] macOS
  • [ ] Windows
  • [X] Linux

Other

No response

adworacz avatar Oct 23 '21 02:10 adworacz

yeah

it will be 201 for @Post(), btw

Basically, @ApiFoundResponse()'s metadata is not taking in count when using @nestjs/swagger plugin

I guess this happen due to the following

https://github.com/nestjs/swagger/blob/491b168cbff3003191e55ee96e77e69d8c1deb66/lib/plugin/visitors/controller-class.visitor.ts#L71-L84

micalevisk avatar Oct 23 '21 02:10 micalevisk

actually, isn't this the intended behavior? The spec will have that 200 OK entry but it doesn't mean that it is reachable, right?

micalevisk avatar Oct 23 '21 03:10 micalevisk

Hmmm, it would seem very strange for this to be intended behavior. It seems incorrect to list a response value in the OpenAPI spec that will never happen. Thinking about this as if an engineer was manually writing this schema, it seems wrong for them to include a 200 for a route that never returns such a status code. Also, any clients will be confused as to why they have to handle a 200.

adworacz avatar Oct 23 '21 19:10 adworacz

oh right

micalevisk avatar Oct 23 '21 20:10 micalevisk