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

feat: Setup local user register & handle exception error to logger #6

Closed
wants to merge 20 commits into from

Conversation

a20688392
Copy link
Collaborator

@a20688392 a20688392 commented Mar 23, 2023

Setup local user register, handle exception error, and setup logger

Setup:

  • local user register:setup user's entity, register api,and handle error.
  • handle exception error:setup handle error and return error response.
  • morgan:record the error log and http request, api.log and error.log will be generated in the log folder.

@a20688392 a20688392 requested a review from moontai0724 March 23, 2023 19:33
@a20688392 a20688392 self-assigned this Mar 23, 2023

import { AppModule } from "./app.module";

async function bootstrap() {
const app = await NestFactory.create(AppModule);
app.use(morgan("default", { stream: logStream }));
Copy link
Member

@moontai0724 moontai0724 Mar 25, 2023

Choose a reason for hiding this comment

The 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

src/main.ts Outdated
setupSwagger(app);
await app.listen(3000);
}

const logStream = fs.createWriteStream("api.log", {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this mean to log into single file in root folder? I would recommend to reconsider about log file save path and segmentation policy like by size or by date etc...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I change in commit
42d0040
c19d7d3

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. generaly we would use model to represent database models, to use this word here would be confusing.
  2. I would recommend to reconsider how to organize these files, like use a exception-filter to organize all-exceptions and this type together.

description: "Data Conflict",
type: CreateConflictUserError,
})
@ApiUnprocessableEntityResponse({
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generally will use 401 instead of 422 to represent that user given data is not correct.


@ApiProperty({
description: "User showname",
example: "showname",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would be better to use a name or something

},
},
},
example: "email has been registered.",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be an array of message

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@a20688392 a20688392 force-pushed the feature/localuser-register branch from cc2fab5 to 0f4a61d Compare April 16, 2023 10:52
@a20688392 a20688392 closed this Jun 8, 2023
@a20688392 a20688392 deleted the feature/localuser-register branch November 26, 2023 13:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants