fix(plugin): restore default 2xx response when only error Api*Response decorators present#3863
Conversation
…e decorators present PR nestjs#3803 added a `hasExplicitApiResponseDecorator` guard that suppressed the auto-generated default response whenever any `Api*Response` decorator (or `@ApiResponse`) appeared on a handler. The check did not distinguish 2xx from non-2xx responses, so handlers decorated with only error responses (`@ApiUnauthorizedResponse`, `@ApiNotFoundResponse`, `@ApiResponse({ status: 500 })`, etc.) silently lost their auto-inferred 201/200, producing specs that advertised only error status codes. The check is narrowed to treat a decorator as an explicit success declaration only when it is a 2xx `Api*Response` alias, `ApiDefaultResponse`, or an `ApiResponse` whose status argument is a 2xx numeric literal, `'2XX'`, or `'default'`. Non-success decorators (3xx, 4xx, 5xx) no longer suppress the inferred 2xx. A regression fixture with error-only decorators is added alongside updates to the existing fixtures that already implied the restored behavior. Fixes nestjs#3862
|
Heads up — this re-introduces #1639. The 2xx-only filter excludes (0, swagger_1.ApiFoundResponse)({ description: 'Redirects to a URL' }),
- (0, common_1.Redirect)()
+ (0, common_1.Redirect)(),
+ openapi.ApiResponse({ status: 200, type: Object }) |
The 2xx-only guard from the previous commit re-introduced nestjs#1639 for redirect handlers: @reDIrect + @ApiFoundResponse (302) was getting a spurious 200 auto-added because 3xx decorators were not recognised as explicit response declarations. Broadens the non-error decorator set (and the @apiresponse status-argument check) to cover 2xx and 3xx so the auto-inferred success response is suppressed for redirect-style handlers while error-only decorators still allow the default to be emitted (the nestjs#3862 behaviour).
|
Thanks @PeterTheOne — good catch, the fixture diff was indeed showing the bug, not the fix. Pushed f00a9fc. The underlying issue was that the "explicit declaration" guard only recognised 2xx decorators. For a handler with Fix: widen the non-error decorator set (renamed The |
|
fixed #3867 |
|
Thanks @kamilmysliwiec — happy this is fixed via #3867. Closing the loop on this one. |
What kind of change does this PR introduce?
Bug fix.
What is the current behavior?
A regression introduced in 11.2.7 (by #3803, fixing #1639) causes the plugin to drop the auto-inferred 2xx response whenever a controller method is decorated with only error
@Api*Responsedecorators. ThehasExplicitApiResponseDecoratorguard added in that PR treats any decorator whose name matchesApi*Responseas an explicit response, so handlers annotated with only@ApiUnauthorizedResponse(),@ApiNotFoundResponse(),@ApiResponse({ status: 500 }), etc. lose their auto-inferred 201/200. The resulting spec advertises only the error status codes, which breaks downstream client generators (NSwag, openapi-generator, etc.).Before 11.2.7:
After 11.2.7 (broken):
Closes #3862
What is the new behavior?
The plugin now treats a decorator as an explicit success declaration only when:
Api*Responsealias (e.g.ApiOkResponse,ApiCreatedResponse,ApiNoContentResponse,ApiAcceptedResponse, and the other 2xx aliases derived fromHttpStatus),ApiDefaultResponse, or@ApiResponsewhosestatusargument is a 2xx numeric literal,'2XX', or'default'.Non-success decorators (3xx/4xx/5xx, including
@ApiFoundResponse,@ApiUnauthorizedResponse, and friends) no longer suppress the inferred 2xx. The auto-generated default is emitted alongside the explicit error entries, and the OpenAPI spec once again contains both success and error status codes.Additional context
HttpStatusat module-load time, mirroring how the decorators themselves are generated inlib/decorators/api-response.decorator.ts, so any future additions stay in sync automatically.statusargument of@ApiResponseis a non-literal expression (e.g.HttpStatus.OK), the plugin cannot evaluate it at compile time; to preserve the original intent of Adding @ApiFoundResponse on a dynamic redirect method still renders a 200 response code in the OpenAPI contract #1639, such expressions continue to be treated as explicit success declarations.test/plugin/fixtures/app.controller-error-only-api-response.ts) exercises the error-only scenarios from the issue: a POST with@ApiUnauthorizedResponse, a GET with multiple error decorators, and a@ApiResponse({ status: 500 }). The existing#1639fixture is updated so the 302 redirect handler now carries the restored default alongside its explicit@ApiFoundResponse.npm test(274 unit tests) andnpm run test:e2e(114 tests) pass locally.