-
-
Notifications
You must be signed in to change notification settings - Fork 7.7k
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
gRPC exceptions and filter #1953
Comments
Applying #764 (comment) 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
}
}
} |
Please, use StackOverflow for such questions. Btw, we will add dedicated exceptions for gRPC shortly. |
Don't you think this issue was fitting the specific case:
(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. 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 ?
That is nice 😍 I started something like this here : https://github.com/tychota/shitake/blob/master/packages/utils-grpc/exception.ts 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 |
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. 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? :) |
Let's track this here |
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. |
Shall I edit the description of first post to reflect new title ? |
Amazing.
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:
|
Here you are: the wip start of the PR #1957. |
Any update on this? |
Any updates on this? |
If anyone needs a quick mapping from 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()); |
@moshest You save my time! |
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,
}));
}
} |
After discussion the goal is to implement gRPC exceptions and filter, and document it, to improve gRPC integration.
See #1953 (comment)
-- OLD ISSUE --
I'm submitting a...
#1015 report a similar problem but the result did not helped me fixing my issue
See what i tried section.
Current behavior
I have an application that expose a public HTTP api.
The "gateway" communicate with microservices through GRPC.
but I get
in my log (custom pino logger).
Expected behavior
What i tried
ex.code = xxx
)RpcException({ message: 'Email already exist', status: 6 })
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:
yarn
yarn webpack -f webpack.config.js
node dist/server.js
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
Closing
Let me know if it is clear enough.
The text was updated successfully, but these errors were encountered: