AlexStocks commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4043601764
代码改动总体上已响应大部分建议。逐条确认:
✓ **问题 1:TLS 互斥分支**(protocol/grpc/client.go)
改为先设置 `transportCreds = insecure.NewCredentials()`,仅在有合法 TLS config 时覆盖,只
append 一次。逻辑清晰,消除了 fail-open 风险。
✓ **问题 2:consumerService nil guard**(protocol/grpc/client.go)
添加了两道防护:
1. 从 RpcServiceKey 读取后立即检查,nil 则 close conn + return error
2. getInvoker 返回后也检查,nil 则 log + close + return error
这确实消除了延迟 panic 的问题。
✓ **问题 3:ExporterMap 污染**(protocol/dubbo3/dubbo3_protocol.go)
改为先完成所有 service/serialization/tracing 校验,全部通过后才 SetExporterMap。失败时直接 panic
而不是 return,更符合 fail-fast 原则。
✓ **问题 5:RegisterService 类型安全**(protocol/grpc/server.go)
改为 `server.RegisterService(ds.ServiceDesc(), ds)` 而不是传 `any`,消除类型不匹配风险。
---
**⚠️ 问题 4:RequestTimeout 仍有缺陷**(protocol/dubbo3/dubbo3_invoker.go)
当前逻辑:
```go
rt = global.DefaultConsumerConfig().RequestTimeout // 硬编码默认"3s"
if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey); ok {
if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok {
rt = consumerConf.RequestTimeout // 覆盖
}
}
timeout := url.GetParamDuration(constant.TimeoutKey, rt)
```
问题在于:**丢失了全局配置的读取能力**。原来的逻辑:
```go
rt = config.GetConsumerConfig().RequestTimeout // 从全局配置系统读用户设置
```
新逻辑用了硬编码默认值,没有办法读取全局配置。这是向后兼容的回归,因为:
1. 删除 config 包前,用户可以通过全局配置(如 dubbo.yaml)设置默认 RequestTimeout
2. 现在只能通过 URL attribute(ConsumerConfigKey)设置
3. 这样会破坏现有部署
建议改为:
```go
// 优先级:URL 参数 > ConsumerConfig > 全局默认值
rt := url.GetParamDuration(constant.TimeoutKey, "")
if rt == "" {
if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey);
ok {
if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok
&& consumerConf.RequestTimeout != "" {
rt = consumerConf.RequestTimeout
}
}
}
if rt == "" {
rt = global.DefaultConsumerConfig().RequestTimeout // 最后才用硬编码默认
}
```
或者,如果设计上要求必须通过 URL 传递所有配置,那需要明确说明这是 breaking change,在 CHANGELOG 中标注,给用户迁移时间。
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]