const movie = await this.movieService.getOne(movie_id);
if(!movie){
throw new Error(
JSON.stringify({
message:'some message',
status:'http status'
})
);
}
const rating = await this.ratingRepository.find({where:{movie});
return rating;
And after it use try catch in controller and throw HttpExeption.
async getAllByMovie(@Param('movie_id') movie_id:string):Promise<Rating[]>{
try{
const ratings = await this.ratingService.getAllRatingsByMovie(Number(movie_id));
return ratings;
}catch(err){
const {message,status} = JSON.parse(err.message);
throw new HttpExeption(message,status);
}
}
Is it good or not?
const movie = await this.movieService.getOne(movie_id);
if(!movie){
throw new Error(
JSON.stringify({
message:'some message',
status:'http status'
})
);
}
const rating = await this.ratingRepository.find({where:{movie});
return rating;
And after it use try catch in controller and throw HttpExeption.
async getAllByMovie(@Param('movie_id') movie_id:string):Promise<Rating[]>{
try{
const ratings = await this.ratingService.getAllRatingsByMovie(Number(movie_id));
return ratings;
}catch(err){
const {message,status} = JSON.parse(err.message);
throw new HttpExeption(message,status);
}
}
Is it good or not?
Share Improve this question asked Jan 12, 2022 at 12:00 VladVlad 3371 gold badge4 silver badges7 bronze badges2 Answers
Reset to default 8In general it's a good idea to throw business errors from your services and handle theses errors on controller layer. But there is room for improvement looking at your code:
To me it looks a bit odd to stringify the message
and status
in order to pass it to Error
. You could create a custom Error that contains these properties:
class MyBusinessError extends Error {
status: number;
constructor(message: string, status: number) {
super(message);
this.status = status;
}
}
But I suggest to decide on controller level which status should be returned from the API because this is http specific and should not be part of your business logic.
Also there are exception filters ing with NestJS that you can use to catch exceptions and transform them into http exceptions. With that you don't need to try-catch in every controller method.
You can check for specific Error type using instanceof
:
try {
// ...
}
catch(err) {
if(err instanceof MyBusinessError) {
// handle business error
}
throw err;
}
In NestJs we have all exception filters so that we don't need to handle errors in all places
you can refer https://docs.nestjs./exception-filters
all-exceptions.filter.ts
import {
ExceptionFilter,
Catch,
ArgumentsHost,
HttpException,
HttpStatus,
} from '@nestjs/mon';
import { object } from 'underscore';
@Catch()
export class AllExceptionsFilter implements ExceptionFilter {
catch(exception: any, host: ArgumentsHost) {
const ctx = host.switchToHttp();
const response = ctx.getResponse();
const request = ctx.getRequest<CustomRequest>();
let internalStatus;
let status =
exception instanceof HttpException
? exception.getStatus()
: HttpStatus.INTERNAL_SERVER_ERROR;
let message = exception.sqlMessage || exception.response || exception;
let error = exception.sqlState === 45000 ? 'Bad Request' : 'Bad Request';
request.log.timeTookToServe = Date.now() - request.log.timestamp;
request.log.message = message;
request.log.status = `${status}`;
if (exception instanceof TypeError) {
status = 400;
error = 'Bad Request';
message = exception.message
.substring(exception.message.indexOf('\n\n\n') + 1)
.trim();
}
if (status === 500) {
console.log(exception.sqlMessage, exception.sqlState, exception);
} else {
console.log(exception.sqlMessage, exception.sqlState, exception);
}
const errMessage = errJson[request.log['module']];
response.status(status).json({
status: exception.status || status,
error: error,
message: [
status === 403
? "Either you don't have the privilege or been logged out."
: message,
],
});
}
}