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

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

Open
2 of 4 tasks
adworacz opened this issue Oct 23, 2021 · 8 comments

Comments

@adworacz
Copy link

Is there an existing issue for this?

  • 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
  • Linux

Other

No response

@micalevisk
Copy link
Member

micalevisk commented Oct 23, 2021

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

ts.createDecorator(
ts.createCall(
ts.createIdentifier(`${OPENAPI_NAMESPACE}.${ApiResponse.name}`),
undefined,
[
this.createDecoratorObjectLiteralExpr(
node,
typeChecker,
ts.createNodeArray(),
hostFilename
)
]
)
)

@micalevisk
Copy link
Member

micalevisk commented Oct 23, 2021

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?

@adworacz
Copy link
Author

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.

@micalevisk
Copy link
Member

oh right

@AndreasMaros
Copy link

Any news on this? We have a couple of routes that for instance return 204 codes; but in Swagger they're still listed with an additional 200 response, which is misleading. Maybe an option to skip adding the default response (via decorator?) could do the trick?

@micalevisk
Copy link
Member

We have a couple of routes that for instance return 204 codes

are they statically defined? please show us a code snippet of it

@AndreasMaros
Copy link

OpenAPI setup:

const config = new DocumentBuilder()
      .setTitle(process.env.npm_package_name)
      .setDescription(process.env.npm_package_description)
      .setVersion(process.env.npm_package_version)
      .addBearerAuth()
      .build();
    const document = SwaggerModule.createDocument(app, config);
    SwaggerModule.setup('api/doc', app, document, {
      swaggerOptions: { persistAuthorization: true },
    });

Controller:

@Controller({ path: '/user/check-eligibility', version: '2' })
export class EligibilityCheckerController {
  @Get()
  @UseGuards(AuthGuard)
  @ApiBearerAuth()
  @ApiNoContentResponse({
    description:
      "The user's email address domain is eligible.",
  })
  @ApiUnauthorizedResponse({
    description:
      'The request is not authenticated, user needs to send a valid bearer token.',
  })
  @ApiForbiddenResponse({
    description: 'This user is not eligible.',
  })
  public async checkEligibility(
    @User() user: AccessToken,
    @Res() response: Response,
  ) {
    const isEligible =
      await this.eligibilityCheckerService.checkUserEligibility(user);

    if (isEligible) {
      return response.status(204).send();
    }

    return response.status(403).send();
  }
}

Generated API documentation:

We're not currently using the CLI, and I couldn't find a generated JSON or YAML, so here's a screenshot while I try to figure out how to get the generated document

grafik


I don't understand where the 200 comes from, or how I can suppress it.

@micalevisk
Copy link
Member

micalevisk commented Jun 26, 2024

@AndreasMaros

I don't understand where the 200 comes from, or how I can suppress it.

from the @Get(). For @Post() it will be 201, and so on. I don't think we can avoid this behavior. And this is not related with this issue so please open another one.

const requestMethod: RequestMethod = Reflect.getMetadata(
METHOD_METADATA,
method
);
switch (requestMethod) {
case RequestMethod.POST:
return HttpStatus.CREATED;
default:
return HttpStatus.OK;
}

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

3 participants