-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: Setup local user register & handle exception error to logger #6
Changes from all commits
9ac6106
12f6c74
b229c73
230866a
4f6798e
0f9b94e
3a58c3d
b4dd35b
e37eae5
c9b3be9
d24f5b4
e8425f0
e9dc0e9
e438e0b
27758ed
40bd465
874d07c
42d0040
c19d7d3
0f4a61d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,20 +1,30 @@ | ||
import { Module } from "@nestjs/common"; | ||
import { ConfigModule } from "@nestjs/config"; | ||
import { APP_FILTER } from "@nestjs/core"; | ||
import { TypeOrmModule } from "@nestjs/typeorm"; | ||
|
||
import { AppController } from "./app.controller"; | ||
import { AppService } from "./app.service"; | ||
import { dataSourceOptions } from "./config/data-source"; | ||
import { validate } from "./config/env.validation"; | ||
import { AllExceptionsFilter } from "./filters/all-exception.filter"; | ||
import { UsersModule } from "./users/users.module"; | ||
|
||
@Module({ | ||
imports: [ | ||
ConfigModule.forRoot({ | ||
validate, | ||
}), | ||
TypeOrmModule.forRoot(dataSourceOptions), | ||
UsersModule, | ||
], | ||
controllers: [AppController], | ||
providers: [AppService], | ||
providers: [ | ||
AppService, | ||
{ | ||
provide: APP_FILTER, | ||
useClass: AllExceptionsFilter, | ||
}, | ||
], | ||
}) | ||
export class AppModule {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
import { MigrationInterface, QueryRunner } from "typeorm"; | ||
|
||
export class UserInitMigration1679539757893 implements MigrationInterface { | ||
name = "UserInitMigration1679539757893"; | ||
|
||
public async up(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query( | ||
`CREATE TABLE \`users\` | ||
( | ||
\`id\` int NOT NULL AUTO_INCREMENT, | ||
\`email\` varchar(255) NOT NULL, | ||
\`name\` varchar(255) NOT NULL, | ||
\`account\` varchar(255) NOT NULL, | ||
\`password\` varchar(255) NOT NULL, | ||
\`createAt\` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6), | ||
\`updateAt\` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6), | ||
UNIQUE INDEX \`IDX_97672ac88f789774dd47f7c8be\` (\`email\`), | ||
UNIQUE INDEX \`IDX_51b8b26ac168fbe7d6f5653e6c\` (\`name\`), | ||
UNIQUE INDEX \`IDX_dd44b05034165835d6dcc18d68\` (\`account\`), | ||
PRIMARY KEY (\`id\`) | ||
) ENGINE=InnoDB`, | ||
); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query( | ||
`DROP INDEX \`IDX_dd44b05034165835d6dcc18d68\` ON \`users\``, | ||
); | ||
await queryRunner.query( | ||
`DROP INDEX \`IDX_51b8b26ac168fbe7d6f5653e6c\` ON \`users\``, | ||
); | ||
await queryRunner.query( | ||
`DROP INDEX \`IDX_97672ac88f789774dd47f7c8be\` ON \`users\``, | ||
); | ||
await queryRunner.query(`DROP TABLE \`users\``); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,29 @@ | ||
import { MigrationInterface, QueryRunner } from "typeorm"; | ||
|
||
export class AdjustUserFieldMigration1681641687260 | ||
implements MigrationInterface | ||
{ | ||
name = "AdjustUserFieldMigration1681641687260"; | ||
|
||
public async up(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query( | ||
`DROP INDEX \`IDX_51b8b26ac168fbe7d6f5653e6c\` ON \`users\``, | ||
); | ||
await queryRunner.query( | ||
`ALTER TABLE \`users\` MODIFY \`password\` varchar(60) NOT NULL`, | ||
); | ||
await queryRunner.query(`ALTER TABLE \`users\` DROP COLUMN \`updateAt\``); | ||
} | ||
|
||
public async down(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query( | ||
`ALTER TABLE \`users\` ADD \`updateAt\` datetime(6) NOT NULL DEFAULT CURRENT_TIMESTAMP(6) ON UPDATE CURRENT_TIMESTAMP(6)`, | ||
); | ||
await queryRunner.query( | ||
`ALTER TABLE \`users\` MODIFY \`password\` varchar(255) NOT NULL`, | ||
); | ||
await queryRunner.query( | ||
`CREATE UNIQUE INDEX \`IDX_51b8b26ac168fbe7d6f5653e6c\` ON \`users\` (\`name\`)`, | ||
); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,92 @@ | ||
import { | ||
ArgumentsHost, | ||
Catch, | ||
ExceptionFilter, | ||
HttpException, | ||
HttpStatus, | ||
} from "@nestjs/common"; | ||
import { Request, Response } from "express"; | ||
import * as fs from "fs"; | ||
import { QueryFailedError } from "typeorm"; | ||
|
||
import { CustomHttpExceptionResponse } from "./models/http-exception-response.interface"; | ||
|
||
@Catch() | ||
export class AllExceptionsFilter implements ExceptionFilter { | ||
catch(exception: unknown, host: ArgumentsHost) { | ||
const ctx = host.switchToHttp(); | ||
const response = ctx.getResponse<Response>(); | ||
const request = ctx.getRequest<Request>(); | ||
|
||
let status: HttpStatus; | ||
let tinyErrorMessage: string; | ||
let fullErrorMessage: string; | ||
let errorMessage: string; | ||
if (exception instanceof HttpException) { | ||
status = exception.getStatus(); | ||
const errorResponse = exception.getResponse(); | ||
errorMessage = errorResponse["message"]; | ||
} else if (exception instanceof TypeError) { | ||
status = HttpStatus.BAD_REQUEST; | ||
errorMessage = exception.message | ||
.substring(exception.message.indexOf("\n\n\n") + 1) | ||
.trim(); | ||
} else if (exception instanceof QueryFailedError) { | ||
status = HttpStatus.INTERNAL_SERVER_ERROR; | ||
tinyErrorMessage = exception.message | ||
.substring(exception.message.indexOf("\n\n\n") + 1) | ||
.trim(); | ||
fullErrorMessage = exception["sql"]; | ||
errorMessage = "Critical internal server error occurred!"; | ||
} else { | ||
status = HttpStatus.INTERNAL_SERVER_ERROR; | ||
errorMessage = "Critical internal server error occurred!"; | ||
fullErrorMessage = JSON.stringify(exception); | ||
} | ||
const errorResponse = this.getErrorResponse(status, errorMessage, request); | ||
const errorLog = this.getErrorLog( | ||
tinyErrorMessage, | ||
fullErrorMessage, | ||
errorResponse, | ||
request, | ||
exception, | ||
); | ||
this.writeErrorLogToFile(errorLog); | ||
response.status(status).json(errorResponse); | ||
} | ||
|
||
private getErrorResponse = ( | ||
status: HttpStatus, | ||
errorMessage: string, | ||
request: Request, | ||
): CustomHttpExceptionResponse => ({ | ||
statusCode: status, | ||
error: errorMessage, | ||
path: request.url, | ||
method: request.method, | ||
timeStamp: new Date(), | ||
}); | ||
|
||
private getErrorLog = ( | ||
tinyErrorMessage: string, | ||
fullErrorMessage: string, | ||
errorResponse: CustomHttpExceptionResponse, | ||
request: Request, | ||
exception: unknown, | ||
): string => { | ||
const { statusCode, error } = errorResponse; | ||
const { method, url } = request; | ||
const errorLog = `Response Code: ${statusCode} - Method: ${method} - URL: ${url} | ||
${JSON.stringify(errorResponse)} | ||
${tinyErrorMessage != undefined ? tinyErrorMessage : ""} | ||
${fullErrorMessage != undefined ? fullErrorMessage : ""} | ||
${exception instanceof HttpException ? exception.stack : error}\n\n`; | ||
return errorLog; | ||
}; | ||
|
||
private writeErrorLogToFile = (errorLog: string): void => { | ||
fs.appendFile("./log/error.log", errorLog, "utf8", err => { | ||
if (err) throw err; | ||
}); | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
export interface CustomHttpExceptionResponse { | ||
statusCode: number; | ||
error: string; | ||
path: string; | ||
method: string; | ||
timeStamp: Date; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,22 @@ | ||
import { INestApplication } from "@nestjs/common"; | ||
import { NestFactory } from "@nestjs/core"; | ||
import { DocumentBuilder, SwaggerModule } from "@nestjs/swagger"; | ||
import * as fs from "fs"; | ||
import * as morgan from "morgan"; | ||
|
||
import { AppModule } from "./app.module"; | ||
|
||
async function bootstrap() { | ||
const app = await NestFactory.create(AppModule); | ||
app.use(morgan("default", { stream: logStream })); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does not described in PR content, should consider to add description about this |
||
setupSwagger(app); | ||
await app.listen(3000); | ||
} | ||
|
||
const logStream = fs.createWriteStream("./log/access.log", { | ||
flags: "a", // append | ||
}); | ||
|
||
function setupSwagger(app: INestApplication) { | ||
const builder = new DocumentBuilder(); | ||
const config = builder | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,62 @@ | ||
import { ApiProperty, PickType } from "@nestjs/swagger"; | ||
import { IsEmail, IsNotEmpty, Length } from "class-validator"; | ||
|
||
export class CreateUserDto { | ||
@ApiProperty({ | ||
description: "User email", | ||
example: "[email protected]", | ||
}) | ||
@IsEmail({}, { message: "email must be in mailbox format." }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use |
||
@IsNotEmpty({ | ||
message: "email is required field.", | ||
}) | ||
public readonly email: string; | ||
|
||
@ApiProperty({ | ||
description: "User showname", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use |
||
example: "showname", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it would be better to use a name or something |
||
}) | ||
@IsNotEmpty({ | ||
message: "name is required field.", | ||
}) | ||
public readonly name: string; | ||
|
||
@ApiProperty({ | ||
description: "User account", | ||
example: "account", | ||
}) | ||
@IsNotEmpty({ | ||
message: "account is required field.", | ||
}) | ||
public readonly account: string; | ||
Comment on lines
+24
to
+31
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. may need a length limit and character limit like only accept numbers, alphabets and underscores. |
||
|
||
@ApiProperty({ | ||
description: "User Password", | ||
example: "Password@123", | ||
}) | ||
@IsNotEmpty({ | ||
message: "password is required field.", | ||
}) | ||
@Length(8, 24, { | ||
message: "password's length must be between 8-24 characters.", | ||
}) | ||
Comment on lines
+40
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why limit password length? I would not limit this, or only limit minimum length |
||
public readonly password: string; | ||
|
||
@ApiProperty({ | ||
description: "check password again", | ||
example: "Password@123", | ||
}) | ||
@IsNotEmpty({ | ||
message: "confirm is required field.", | ||
}) | ||
@Length(8, 24, { | ||
message: "confirm's length must be between 8-24 characters.", | ||
}) | ||
public readonly confirm: string; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think backend can ignore password confirm field, this can be done by frontend |
||
} | ||
export class CreateUserParam extends PickType(CreateUserDto, [ | ||
"name", | ||
"email", | ||
"account", | ||
"password", | ||
] as const) {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
import { | ||
BaseEntity, | ||
Column, | ||
CreateDateColumn, | ||
Entity, | ||
PrimaryGeneratedColumn, | ||
} from "typeorm"; | ||
|
||
@Entity({ name: "users" }) | ||
export class UserEntity extends BaseEntity { | ||
@PrimaryGeneratedColumn() | ||
id: number; | ||
|
||
@Column({ unique: true }) | ||
email: string; | ||
|
||
@Column() | ||
name: string; | ||
|
||
@Column({ unique: true }) | ||
account: string; | ||
|
||
@Column({ | ||
length: 60, | ||
}) | ||
password: string; | ||
|
||
@CreateDateColumn() | ||
createAt: Date; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
import { ApiProperty } from "@nestjs/swagger"; | ||
|
||
export class CreateConflictUserError { | ||
@ApiProperty({ | ||
type: "number", | ||
description: "HTTP StatusCode", | ||
example: "409", | ||
}) | ||
public readonly StatusCode: number; | ||
|
||
@ApiProperty({ | ||
type: "array", | ||
description: "Error Message", | ||
items: { | ||
properties: { | ||
email: { | ||
description: "email has been registered. \n", | ||
type: "string", | ||
}, | ||
name: { | ||
description: "name has been registered. \n", | ||
type: "string", | ||
}, | ||
account: { | ||
description: "account has been registered. \n", | ||
type: "string", | ||
}, | ||
password: { | ||
description: | ||
"Confirm and password do not match, please try again. \n", | ||
|
||
type: "string", | ||
}, | ||
}, | ||
}, | ||
example: "email has been registered.", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should be an array of message |
||
}) | ||
public readonly error: string[]; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bcrypt hashed password should only need 60 char length
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I change in commit
0f4a61d