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

增加业务校验异常errorCode=403处理 #619 #626

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

yesAnd92
Copy link

目前sdk提供的api屏蔽了nacos server返回的业务校验异常提示
比如server开启安全认证后,client账密错误server的响应:
{"resultCode":500,"errorCode":403,"message":"user not found!","success":false}

比如当前用户只授权读权限,client发起写操作请求server的响应:
{"resultCode":500,"errorCode":403,"message":"authorization failed!","success":false}

等等这些错误提示被屏蔽掉对开发人员排查问题来说不太友好。
为此,增加了对errorCode=403业务异常的处理:

  • 增加日志输出
  • 将错误提示作为接口返回的err,让用开发者可以自行处理

@yesAnd92
Copy link
Author

see #619

@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2023

Codecov Report

Patch coverage: 36.36% and project coverage change: +0.20 🎉

Comparison is base (16975cb) 30.22% compared to head (6a32900) 30.43%.

❗ Current head 6a32900 differs from pull request most recent head b5d9c4b. Consider uploading reports for the commit b5d9c4b to get more accurate results

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #626      +/-   ##
==========================================
+ Coverage   30.22%   30.43%   +0.20%     
==========================================
  Files          40       41       +1     
  Lines        3060     3118      +58     
==========================================
+ Hits          925      949      +24     
- Misses       2069     2096      +27     
- Partials       66       73       +7     
Impacted Files Coverage Δ
clients/config_client/config_proxy.go 5.12% <0.00%> (-0.19%) ⬇️
clients/naming_client/naming_client.go 33.65% <0.00%> (-1.02%) ⬇️
...nts/naming_client/naming_grpc/naming_grpc_proxy.go 0.00% <0.00%> (ø)
clients/config_client/config_client.go 36.60% <17.64%> (-1.20%) ⬇️
util/naming.go 69.44% <69.44%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Member

@binbin0325 binbin0325 left a comment

Choose a reason for hiding this comment

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

不仅仅是有403,可以参考java实现一版(包括注册中心的response状态码校验)

@yesAnd92 yesAnd92 requested a review from binbin0325 June 29, 2023 14:44
@yesAnd92
Copy link
Author

参照java版,实现了注册中心和配置中心相关接口不同业务异常状态的处理逻辑,包括:

  • 增加相应异常日志输出
  • 将错误提示作为接口返回的err,让用开发者可以自行处理

同时,注册中心参照Java版实现,增加实例注册前的参数校验,并返回参数校验的异常提示:

  • 心跳超时间必须大于心跳周期,ip删除周期必须大于心跳周期;
  • clusterName命名的合法性检验;

@binbin0325 binbin0325 removed their request for review June 30, 2023 02:20
@CLAassistant
Copy link

CLAassistant commented Oct 23, 2023

CLA assistant check
All committers have signed the CLA.

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