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

Feature request: set HttpCode trough res.status() #1869

Closed
ToonvanStrijp opened this issue Mar 28, 2019 · 6 comments

Comments

@ToonvanStrijp
Copy link
Contributor

commented Mar 28, 2019

I'm submitting a...


[X] Feature request

Current behavior

Currently the only way to set a custom http status code is to use the @Res() or @HttpCode(). By default Nest is returning 200 or 201. Right now I'm trying to implement a interceptor that sets the status code but because Nest is overriding this in the reply phase it's not possible to set it. Or yeah it is but Nest overrides it.

Expected behavior

Wouldn't it be nice to be able to use the res.status(401) to set the status code. We could easily get this done by changing the adapters.

Right now we could do something like this, but this doesn't feel like a good solution since the response will be sent and other interceptors won't have effect anymore.

import { CallHandler, ExecutionContext, HttpStatus, NestInterceptor } from '@nestjs/common';
import { Observable } from 'rxjs';
import { map } from 'rxjs/operators';
import { classToPlain } from 'class-transformer';
import { ApiResponse } from '../models/apiResponse';
import { FastifyReply } from 'fastify';
import { ServerResponse } from 'http';

export class TransformInterceptor implements NestInterceptor {
  intercept(
    context: ExecutionContext,
    next: CallHandler<ApiResponse | any>,
  ): Observable<ApiResponse | any> {
    return next.handle().pipe(
      map(data => {
        const ctx = context.switchToHttp();
        const res = ctx.getResponse<FastifyReply<ServerResponse>>();
        const result = classToPlain(data);

        if(data instanceof ApiResponse) {
          res.code(data.status !== undefined ? data.status : HttpStatus.OK).send(result);
        }
      }),
    );
  }
}

Environment


Nest version: 6.0.2
@kamilmysliwiec

This comment has been minimized.

Copy link
Member

commented Mar 29, 2019

We could easily get this done by changing the adapters.

Would you like to create a PR?

@ToonvanStrijp

This comment has been minimized.

Copy link
Contributor Author

commented Mar 29, 2019

@kamilmysliwiec the only thing that I’m looking at is to make it so that we could set it by using reflect meta. I tried to set it but this doesn’t work because NestJS is running the interceptors after it received the status code.

Could we just switch this process without breaking changes?

So, first transform the data (run the interceptors etc) then pull the httpstatus code using the reflect metadata key?

@kamilmysliwiec

This comment has been minimized.

Copy link
Member

commented Mar 31, 2019

So I think that we could check whether .statusCode (in platform-express) or .res.statusCode (in platform-fastify) differs from the default one (for instance, 200 for GET) and depending on that don't override the status code that has been manually set through res.status(). That change would allow manipulating response status in interceptors.

@ToonvanStrijp

This comment has been minimized.

Copy link
Contributor Author

commented Mar 31, 2019

@kamilmysliwiec wouldn't it be better to just make sure nest sets the statusCode before running the interceptors etc? platform-express and platform-fastify both send a 200 by default. We could do something like:

if(res.statusCode === 200) {
   // set nest default response (201 for POST, 200 for everything else, or the status that is set via @HttpCode decorator)
}

But I think this will just complicate things more. The best solution is to set the statusCode on the response before we run the interceptors, this way you can also get the statusCode inside your interceptor if needed and we don't have to care about handling the status override on the end.

@kamilmysliwiec

This comment has been minimized.

Copy link
Member

commented Apr 10, 2019

Added in 6.1.0

Thanks for your contribution!!

@lock

This comment has been minimized.

Copy link

commented Sep 23, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 23, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
2 participants
You can’t perform that action at this time.