nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript šŸš€
https://nestjs.com
MIT License
66.81k stars 7.55k forks source link

gRPC exceptions and filter #1953

Open tychota opened 5 years ago

tychota commented 5 years ago

After discussion the goal is to implement gRPC exceptions and filter, and document it, to improve gRPC integration.

See https://github.com/nestjs/nest/issues/1953#issuecomment-480885674

-- OLD ISSUE --

I'm submitting a...

https://github.com/nestjs/nest/issues/1015 report a similar problem but the result did not helped me fixing my issue

See what i tried section.


[ ] Regression 
[ ] Bug report
[ ] Feature request
[x] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

I have an application that expose a public HTTP api. The "gateway" communicate with microservices through GRPC.

@Injectable()
export class AuthService {
  constructor(private readonly commandBus: CommandBus, private readonly queryBus: QueryBus) {}

  // ...

  async assertEmailDoesNotExist(email: string) {
    const emailAlreadyExist = await this.queryBus.execute(new DoesEmailExistQuery(email));
    if (emailAlreadyExist) {
      // >>>> Here <<<<
      // I would like to display an "ALREADY_EXIST"
      // https://github.com/grpc/grpc/blob/master/doc/statuscodes.md
      const exception = new RpcException({ message: 'Email already exist', status: 6 });
      throw exception;
    }
  }
}

but I get

{
  "level":50,
   /// ...
  "context":"ExceptionsHandler",
  "msg":"2 UNKNOWN: Email already exist",
  "v":1
}

in my log (custom pino logger).

Expected behavior

{
  "level":50,
   /// ...
  "context":"ExceptionsHandler",
  "msg":"6 ALREADY_EXISTS: Email already exist",
  "v":1
}

What i tried

import { ExceptionFilter, Catch, ArgumentsHost, HttpException, RpcExceptionFilter } from '@nestjs/common';
import { RpcException, BaseRpcExceptionFilter } from '@nestjs/microservices';
import { Observable, throwError } from 'rxjs';

export class HttpExceptionFilter extends BaseRpcExceptionFilter implements RpcExceptionFilter {
  catch(exception: RpcException, host: ArgumentsHost): Observable<unknown> {
    if (exception instanceof RpcException) {
      return throwError({status: 6})
    }
    return super.catch(exception, host);
  }
}

Ultimatly, without doc, I lack knowledge how to solve my problem. I did try but I guess not in the right direction

Minimal reproduction of the problem with instructions

Not really minimal (but close two, one api gateway, two microservices) but you can found the codebase here: https://github.com/tychota/shitake

To run it:

What is the motivation / use case for changing the behavior?

In http, it is super nice, you can just throw a ConflictException and boom, you have a 429.

In GRPC world it is not as nice :/

You have to play with message string (hardcoded stuff) and ultimatly not every status code are equal (https://github.com/grpc/grpc/blob/master/doc/statuscodes.md) (like a 401 in HTTp may be expected but a 500 isn't). Here you have just the equivalence of 500, with all what is implied (logging of a 500).

Environment


Nest version:
    "@nestjs/common": "^6.0.5",
    "@nestjs/cqrs": "^6.0.0",

For Tooling issues:
- Node version: v8.12.0
- Platform:  Linux, Gentoo

Closing

Let me know if it is clear enough.

tychota commented 5 years ago

Applying https://github.com/nestjs/nest/issues/764#issuecomment-394837873 solve part of my problem.

The GRPC status is now correct but i can't try catch in the gateway:

Expected

@Controller('auth')
export class AuthGatewayController implements OnModuleInit {
  @Client(authClientOptions)
  private readonly authClient!: ClientGrpc;

  private authCommand!: AuthCommand;

  onModuleInit() {
    this.authCommand = this.authClient.getService<AuthCommand>('Command');
  }

  @Post('/register')
  // @UseFilters(new GrpcExceptionFilter())
  @UsePipes(new ValidationPipe({ transform: true }))
  async register(@Body() dto: AuthClearTextCredentialsDto) {
    try {
      return this.authCommand.register(dto);
   } catch (e) {
       // do dmthg with e and raise HTPP status code accordingly
   }
  }
}
kamilmysliwiec commented 5 years ago

Please, use StackOverflow for such questions.

Btw, we will add dedicated exceptions for gRPC shortly.

tychota commented 5 years ago

Please, use StackOverflow for such questions.

Don't you think this issue was fitting the specific case:

[x] Documentation issue or request

(Goal is not to criticise the issue being closed but to understand a bit more as without explanation it just feel a bit harsh: "such questions" is a bit condescending, at least I understand it like so, and that does not reflect the time I put filling the issue).

Don't get me wrong, I love Nest and most of the lib is super well designed and documented. I'm also not mad at you and I know maintaining open source is involving and tiresome sometimes.

That being said, I still think GRPC is hard to get right with the current doc and tooling in Nest.

My goal is to help Nest and do some PR in th doc so it get easier in the future. Thus I wanted to discuss a real use case to figure out the good way to answer this problem:

When doing a microservice with nest that use GRPC, it is hard to make a micro service fail and get another status than 500 in the gateway

Should I open another issue without the complex example to track that use case ?

How would you like go forward on this ?

Btw, we will add dedicated exceptions for gRPC shortly.

That is nice šŸ˜

I started something like this here : https://github.com/tychota/shitake/blob/master/packages/utils-grpc/exception.ts I can PR if needed.

I also created a mapping https://github.com/tychota/shitake/blob/master/packages/utils-grpc/mapping.ts so it is easy to raise appropriate HTTP exception based on GRPC status.

Look at https://github.com/tychota/shitake/blob/master/packages/utils-grpc/formatHttpResponse.ts and usage here: https://github.com/tychota/shitake/blob/master/packages/api-gateway/controllers/auth.controller.ts#L55-L63

kamilmysliwiec commented 5 years ago

(Goal is not to criticise the issue being closed but to understand a bit more as without explanation it just feel a bit harsh: "such questions" is a bit condescending, at least I understand it like so, and that does not reflect the time I put filling the issue). Don't get me wrong, I love Nest and most of the lib is super well designed and documented. I'm also not mad at you and I know maintaining open source is involving and tiresome sometimes.

It's all about time @tychota. I'd love to write long posts. I can't do it though :) Anyway, thanks for getting me full context.

This: https://github.com/tychota/shitake/blob/master/packages/utils-grpc/exception.ts is awesome. And this: https://github.com/tychota/shitake/blob/master/packages/utils-grpc/mapping.ts would be useful as well.

It would be great to create a filter that maps these exceptions to gRPC error objects.

Would you like to create a PR with your files? :)

kamilmysliwiec commented 5 years ago

Let's track this here

tychota commented 5 years ago

Thank for answering and reopening: I understand and my issue was not super quality anyway.

I will create the PR with the above file and then we can see how to improve the doc.

tychota commented 5 years ago

Shall I edit the description of first post to reflect new title ?

kamilmysliwiec commented 5 years ago

I will create the PR with the above file and then we can see how to improve the doc.

Amazing.

Shall I edit the description of first post to reflect new title ?

This one is fine - at least outlines the problem and lack of the functionality very well :)

In order to make the integration better, we would need:

tychota commented 5 years ago

Here you are: the wip start of the PR https://github.com/nestjs/nest/pull/1957.

raikusy commented 4 years ago

Any update on this?

sadhakbj commented 4 years ago

Any updates on this?

moshest commented 2 years ago

If anyone needs a quick mapping from HttpException to gRPC:

import {
  Catch,
  ExceptionFilter,
  HttpException,
  HttpStatus,
} from '@nestjs/common';
import { Observable, throwError } from 'rxjs';
import { status } from '@grpc/grpc-js';

@Catch(HttpException)
export class HttpExceptionFilter implements ExceptionFilter {
  static HttpStatusCode: Record<number, number> = {
    // standard gRPC error mapping
    // https://cloud.google.com/apis/design/errors#handling_errors
    [HttpStatus.BAD_REQUEST]: status.INVALID_ARGUMENT,
    [HttpStatus.UNAUTHORIZED]: status.UNAUTHENTICATED,
    [HttpStatus.FORBIDDEN]: status.PERMISSION_DENIED,
    [HttpStatus.NOT_FOUND]: status.NOT_FOUND,
    [HttpStatus.CONFLICT]: status.ALREADY_EXISTS,
    [HttpStatus.GONE]: status.ABORTED,
    [HttpStatus.TOO_MANY_REQUESTS]: status.RESOURCE_EXHAUSTED,
    499: status.CANCELLED,
    [HttpStatus.INTERNAL_SERVER_ERROR]: status.INTERNAL,
    [HttpStatus.NOT_IMPLEMENTED]: status.UNIMPLEMENTED,
    [HttpStatus.BAD_GATEWAY]: status.UNKNOWN,
    [HttpStatus.SERVICE_UNAVAILABLE]: status.UNAVAILABLE,
    [HttpStatus.GATEWAY_TIMEOUT]: status.DEADLINE_EXCEEDED,

    // additional built-in http exceptions
    // https://docs.nestjs.com/exception-filters#built-in-http-exceptions
    [HttpStatus.HTTP_VERSION_NOT_SUPPORTED]: status.UNAVAILABLE,
    [HttpStatus.PAYLOAD_TOO_LARGE]: status.OUT_OF_RANGE,
    [HttpStatus.UNSUPPORTED_MEDIA_TYPE]: status.CANCELLED,
    [HttpStatus.UNPROCESSABLE_ENTITY]: status.CANCELLED,
    [HttpStatus.I_AM_A_TEAPOT]: status.UNKNOWN,
    [HttpStatus.METHOD_NOT_ALLOWED]: status.CANCELLED,
    [HttpStatus.PRECONDITION_FAILED]: status.FAILED_PRECONDITION,
  };

  catch(exception: HttpException): Observable<never> | void {
    const httpStatus = exception.getStatus();
    const httpRes = exception.getResponse() as { details?: unknown };

    return throwError(() => ({
      code: HttpExceptionFilter.HttpStatusCode[httpStatus] ?? status.UNKNOWN,
      message: exception.message,
      details: Array.isArray(httpRes.details) ? httpRes.details : undefined,
    }));
  }
}

Then connect it as a global filter on the microservice:

const microservice = await NestFactory.createMicroservice<MicroserviceOptions>(...);

microservice.useGlobalFilters(new HttpExceptionFilter());
single9 commented 2 years ago

@moshest You save my time!

Farenheith commented 9 months ago

I have a hybrid http1/grpc application. The grpc app have either unary and stream server methods, and returning throwError wasn't working to treat the stream server errors, so, I created this base class to do all the exception filters, and it worked in every scenario:

import { throwError } from 'rxjs';
import { FastifyReply } from 'fastify';
import { ArgumentsHost, ExceptionFilter, HttpStatus } from '@nestjs/common';
import { ServerUnaryCall, ServerWritableStream, status } from '@grpc/grpc-js';

const httpToGrpc: Record<number, number> = {
    [HttpStatus.BAD_REQUEST]: status.INVALID_ARGUMENT,
    [HttpStatus.UNAUTHORIZED]: status.UNAUTHENTICATED,
    [HttpStatus.FORBIDDEN]: status.PERMISSION_DENIED,
    [HttpStatus.NOT_FOUND]: status.NOT_FOUND,
    [HttpStatus.CONFLICT]: status.ALREADY_EXISTS,
    [HttpStatus.GONE]: status.ABORTED,
    [HttpStatus.TOO_MANY_REQUESTS]: status.RESOURCE_EXHAUSTED,
    499: status.CANCELLED,
    [HttpStatus.INTERNAL_SERVER_ERROR]: status.INTERNAL,
    [HttpStatus.NOT_IMPLEMENTED]: status.UNIMPLEMENTED,
    [HttpStatus.BAD_GATEWAY]: status.UNKNOWN,
    [HttpStatus.SERVICE_UNAVAILABLE]: status.UNAVAILABLE,
    [HttpStatus.GATEWAY_TIMEOUT]: status.DEADLINE_EXCEEDED,

    // additional built-in http exceptions
    // https://docs.nestjs.com/exception-filters#built-in-http-exceptions
    [HttpStatus.HTTP_VERSION_NOT_SUPPORTED]: status.UNAVAILABLE,
    [HttpStatus.PAYLOAD_TOO_LARGE]: status.OUT_OF_RANGE,
    [HttpStatus.UNSUPPORTED_MEDIA_TYPE]: status.CANCELLED,
    [HttpStatus.UNPROCESSABLE_ENTITY]: status.CANCELLED,
    [HttpStatus.I_AM_A_TEAPOT]: status.UNKNOWN,
    [HttpStatus.METHOD_NOT_ALLOWED]: status.CANCELLED,
    [HttpStatus.PRECONDITION_FAILED]: status.FAILED_PRECONDITION,
};
export function getStatusCode(host: ArgumentsHost, code: number) {
    return host.getType() === 'http' ? code : httpToGrpc[code];
}

function isGrpcStream(call: ServerWritableStream<unknown, unknown> | ServerUnaryCall<unknown, unknown>): call is ServerWritableStream<unknown, unknown> {
  return call.constructor.name.includes('Stream');
}

const RESULT_INDEX = 2;

export abstract class BaseCustomExceptionFilter implements ExceptionFilter {
    constructor(private statusCode: number) {}

    catch(error: Error, host: ArgumentsHost) {
        const { message } = error;
        const ctx = host.switchToHttp();
        const statusCode =
            getStatusCode(host, this.statusCode) ?? HttpStatus.INTERNAL_SERVER_ERROR;
        if (host.getType() === 'http') {
            return ctx.getResponse<FastifyReply>().status(statusCode).send({
                statusCode,
                message,
            });
        }
        const call =
            host.getArgByIndex<ServerWritableStream<unknown, unknown> | ServerUnaryCall<unknown, unknown>>(RESULT_INDEX);

        if (isGrpcStream(call)) {
            return call.emit('error', {
                code: statusCode,
                message,
            });
        }

        return throwError(() => ({
            code: statusCode,
            message,
            type: error.constructor.name,
        }));
    }
}