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

fix: add addressing error example in examroom module #130

Merged
merged 7 commits into from
Dec 12, 2024

Conversation

SchwarzSail
Copy link
Member

@SchwarzSail SchwarzSail commented Dec 7, 2024

自查 PR 结构

  • PR 标题符合这个格式: <type>(optional scope): <description>

  • 此 PR 标题的描述以用户为导向,足够清晰,其他人可以理解。

  • 我已经对所有 commit 提供了签名(GPG 密钥签名、SSH 密钥签名)

  • 这个 PR 属于强制变更/破坏性更改

如果是,请在 PR 标题中添加 BREAKING CHANGE 前缀,并在 PR 描述中详细说明。

这个 PR 的类型是什么?

这个 PR 做了什么 / 我们为什么需要这个 PR?

在 examroom 模块添加了错误的处理流程:

  1. 在原始错误的产生处(根源)使用 errno.Errorf 进行定义,包含了错误码和错误信息
  2. 之后只会上抛错误,并附带相关信息
  3. 在 handler(controller)层使用 logger.Info 打印出业务错误
  4. api 模块与其他模块是使用 RPC 进行交互的,如果出现 err(rpc 调度错误,并非业务错误),使用 logger.Errorf
  5. api 模块属于业务的错误统一不做任何处理,不断上抛至 pack.RespError(c, err)

(可选)这个 PR 解决了哪个/些 issue?

对 Reviewer 预留的一些提醒

@SchwarzSail SchwarzSail requested review from jiuxia211, ozline and a team as code owners December 7, 2024 07:14
@@ -84,7 +84,7 @@ func (s *ClassroomServiceImpl) GetExamRoomInfo(ctx context.Context, req *classro
resp = classroom.NewExamRoomInfoResponse()
rooms, err := service.NewClassroomService(ctx, s.ClientSet).GetExamRoomInfo(req)
if err != nil {
logger.Infof("Classroom.GetExamRoomInfo: Get exam room info fail %v", err)
logger.Errorf("Classroom.GetExamRoomInfo: Get exam room info fail %v", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

cookie 过期的错误是经常出现的,用 logger.Errorf 是不是不合适?

Copy link
Member Author

Choose a reason for hiding this comment

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

统一一下吧:业务错误使用 Info,其他的错误用 error 吧

Copy link
Member

Choose a reason for hiding this comment

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

的确,这样普通运行的时候v 设置小一点,使得只记录必要的错误

调试的时候增大 v 就可以显示更多信息,没有毛病

不过你有没有想过这里用 Debug 和用 Info 的区别?@SchwarzSail

Copy link
Member Author

Choose a reason for hiding this comment

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

的确,这样普通运行的时候v 设置小一点,使得只记录必要的错误

调试的时候增大 v 就可以显示更多信息,没有毛病

不过你有没有想过这里用 Debug 和用 Info 的区别?@SchwarzSail

没有)求科普

Copy link
Member

Choose a reason for hiding this comment

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

的确,这样普通运行的时候v 设置小一点,使得只记录必要的错误
调试的时候增大 v 就可以显示更多信息,没有毛病
不过你有没有想过这里用 Debug 和用 Info 的区别?@SchwarzSail

没有)求科普

在日志初始化的时候 Logger 对象有一个变量叫 LogLevel,他只会打印大于这个 Level 的日志。
比如你的 logger 的 LogLevel 为 Info,那么他会打印出 info。
但是你的 LogLevel 为 debug 的话,因为他会打印出 info 和 debug,因为 debugLevel 等级更低,所以他会打印所有大于这个等级的日志。
同理,如果你的 Level 为 Error,那么下面这个将什么都不会输出,因为 Error > Warning > Info > Debug

logger.Info("info")
logger.Debug("debug")

@jiuxia211
Copy link
Contributor

@mutezebra cc

Copy link
Member

@mutezebra mutezebra left a comment

Choose a reason for hiding this comment

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

我认为没有问题

@ozline ozline merged commit 26bc4c3 into west2-online:main Dec 12, 2024
6 checks passed
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.

4 participants