You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "Snow-kal (via GitHub)" <gi...@apache.org> on 2026/03/06 05:26:39 UTC
[PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Snow-kal opened a new pull request, #3231:
URL: https://github.com/apache/dubbo-go/pull/3231
更改概览和 TODO 更改情况
protocol/dubbo3/dubbo3_invoker.go 已移除
- TODO RequestTimeout：默认超时读取路径变更，改为从 global.DefaultConsumerConfig() 读取，经 url 的 ConsumerConfigKey 覆盖后，统一用 TimeoutKey 解析最终值
- TODO consumerService：删除旧兼容逻辑，仅从url的RpcServiceKey 属性获取服务实例
- TODO tracing：不再从 config 读取，改为直接读取 url 的 TracingConfigKey，并补充类型校验和空值兜底
- TODO TLSConfig：移除全局配置分支，仅保留从 url 的 TLSConfigKey 属性初始化 TLS
protocol/dubbo3/dubbo3_protocol.go 已移除
- TODO deprecated：删除废弃的注释代码，保留初始化逻辑
- TODO Export()：删除旧API兼容逻辑，移除 config.GetProviderService，改为仅从url的 RpcServiceKey 属性获取
- TODO tracing：不再从 config 读取，移除对 config.GetTracingConfig/GetApplicationConfig 的依赖
- TODO TLSConfig：移除 root-config 的全局配置分支，保留配置链路
protocol/grpc/client.go 已移除
- TODO: remove config TLSConfig：已删除 config TLS 分支
- TODO: remove this else：已移除该 else 分支。
- TODO: Temporary compatibility...（consumerService）：已移除旧 API 查找，保留 RpcServiceKey attribute。
- TODO: Temporary compatibility...（protocolConf）：已移除 rootConfig/dubbo compat 路径，保留 ProtocolConfigKey attribute
protocol/grpc/server.go
- TODO:remove config TLSConfig：已删除 root-config TLS 分支，保留TLS 路径。
- TODO:remove this else：已去掉原 else 分支
- TODO #2741 old config compatibility（getProviderServices）：已去掉 config 读取与兼容转换，保留 ProviderConfigKey
- TODO #2741 old config compatibility（waitGrpcExporter）：兼容注释已清理，逻辑保留为等待 exporter
- TODO #2741 old config compatibility（registerService）：已去掉旧 config 服务查找链路
- Temporary compatibility with old APIs：已删除 config.GetProviderService(key)，改从 invoker URL 的 RpcServiceKey 获取服务对象
### Description
issue https://github.com/apache/dubbo-go/issues/3204
### Checklist
- [ √] I confirm the target branch is `develop`
- [ √] Code has passed local testing
- [ √] I have added tests that prove my fix is effective or that my feature works
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901399379
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
+ if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
+ consumerService = rpcService
}
invoker := getInvoker(consumerService, conn)
Review Comment:
感谢建议 已更改
<img width="1032" height="677" alt="image" src="https://github.com/user-attachments/assets/162f0eb5-e2c9-4446-b4a5-3fa4bfd3703b" />
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4062106820
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_coverage&view=list)
 [30.8% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2899091507
##########
protocol/dubbo3/dubbo3_protocol.go:
##########
@@ -80,17 +77,16 @@ func NewDubboProtocol() *DubboProtocol {
func (dp *DubboProtocol) Export(invoker base.Invoker) base.Exporter {
url := invoker.GetURL()
serviceKey := url.ServiceKey()
- exporter := NewDubboExporter(serviceKey, invoker, dp.ExporterMap(), dp.serviceMap)
- dp.SetExporterMap(serviceKey, exporter)
- logger.Infof("[Triple Protocol] Export service: %s", url.String())
key := url.GetParam(constant.BeanNameKey, "")
var service any
- //TODO: Temporary compatibility with old APIs, can be removed later
- service = config.GetProviderService(key)
if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
service = rpcService
}
+ if service == nil {
+ logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
+ return nil
Review Comment:
`Export` 提前 `return nil` 导致 exporter 没有写入 map，`grpc/server.go` 的 `waitGrpcExporter` 会等待 exporter 数量与 providerServices 数量一致，因超时最终触发 `panic("wait grpc exporter timeout")`，真正原因（缺少 `RpcServiceKey`）被完全隐藏，排查极难。
调用方（`server/action.go`、`config/service_config.go`）会把 nil 当错误向上传递，但中间层 filter chain 不一定做了 nil 检查，存在隐患。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2899090807
##########
protocol/grpc/client.go:
##########
@@ -93,24 +88,10 @@ func NewClient(url *common.URL) (*Client, error) {
grpc.MaxCallRecvMsgSize(maxCallRecvMsgSize),
grpc.MaxCallSendMsgSize(maxCallSendMsgSize),
),
+ grpc.WithTransportCredentials(insecure.NewCredentials()),
Review Comment:
`insecure.NewCredentials()` 被无条件加入 `dialOpts`，之后若命中 TLS 分支，又 append 了 `credentials.NewTLS(cfg)`。gRPC 不允许同一连接同时配置两套 transport credentials，TLS 场景**完全失效**。原代码是互斥的 `else if ... else` 结构，PR 把互斥性破坏了。`server.go` 存在同样问题。
建议恢复互斥结构：
```go
if tlsConfRaw, ok := url.GetAttribute(constant.TLSConfigKey); ok {
// TLS 路径
} else {
dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901377268
##########
protocol/grpc/client.go:
##########
@@ -127,12 +108,7 @@ func NewClient(url *common.URL) (*Client, error) {
logger.Infof("Grpc Client initialized the TLSConfig configuration")
dialOpts = append(dialOpts, grpc.WithTransportCredentials(credentials.NewTLS(cfg)))
}
- } else {
- dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
- } else {
- // TODO: remove this else
- dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
Review Comment:
已经更改 感谢建议 这个和以上的 server 的类似
<img width="1416" height="1055" alt="image" src="https://github.com/user-attachments/assets/6bcb347d-5bea-422e-8d3f-11bbef471b2b" />
总结一下就是
```
transportCreds = insecure.NewCredentials() // 默认值：无 TLS
if url 有 TLSConfigKey {
if tlsConf 类型断言失败 {
return error // 配置异常，直接返回
}
if IsClientTLSValid(tlsConf) {
if GetClientTlSConfig 出错 {
return error // 获取 TLS 配置失败，直接返回
}
if cfg != nil {
transportCreds = credentials.NewTLS(cfg) // 覆盖为 TLS
}
}
}
dialOpts = append(dialOpts, grpc.WithTransportCredentials(transportCreds)) // 只 append 一次
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4053617440
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4018581748
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2893943234
##########
protocol/dubbo3/dubbo3_protocol.go:
##########
@@ -86,11 +83,13 @@ func (dp *DubboProtocol) Export(invoker base.Invoker) base.Exporter {
key := url.GetParam(constant.BeanNameKey, "")
var service any
- //TODO: Temporary compatibility with old APIs, can be removed later
- service = config.GetProviderService(key)
if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
service = rpcService
}
+ if service == nil {
+ logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
+ return nil
+ }
Review Comment:
已经改：
现在先完成 service 和序列化校验，全部通过后才 SetExporterMap。
效果：失败时不再污染 ExporterMap
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2899091151
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
+ if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
+ consumerService = rpcService
}
invoker := getInvoker(consumerService, conn)
Review Comment:
`getInvoker` 内部：`reflect.ValueOf(nil).MethodByName("GetDubboStub")` 当 `consumerService` 为 nil 时会直接 **panic**，根本不会返回 nil 供调用方检查。`if invoker != nil` 这个 nil guard 加在 panic 之后，毫无意义。另外返回 `reflect.Value{}` 后续调用 `invoker.Call(...)` 同样会 panic。
建议：在调用 `getInvoker` 前检查：
```go
if consumerService == nil {
conn.Close()
return nil, fmt.Errorf("grpc: no rpc service found for interface=%s", key)
}
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks merged PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4009673616
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_coverage&view=list)
 [46.9% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2893919865
##########
protocol/dubbo3/dubbo3_protocol.go:
##########
@@ -86,11 +83,13 @@ func (dp *DubboProtocol) Export(invoker base.Invoker) base.Exporter {
key := url.GetParam(constant.BeanNameKey, "")
var service any
- //TODO: Temporary compatibility with old APIs, can be removed later
- service = config.GetProviderService(key)
if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
service = rpcService
}
+ if service == nil {
+ logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
+ return nil
+ }
Review Comment:
Export() stores the exporter into dp.ExporterMap() before verifying that the service instance exists (RpcServiceKey). If the attribute is missing, the function returns nil but leaves a stale exporter entry in the map, which can cause inconsistent state and later panics/timeouts. Consider validating the service first (before SetExporterMap), or removing the exporter from the map on this failure path, and ensure Export() never returns nil after partially mutating protocol state.
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2899092435
##########
protocol/grpc/server.go:
##########
@@ -243,6 +198,14 @@ func registerService(providerServices map[string]*global.ServiceConfig, server *
if invoker == nil {
panic(fmt.Sprintf("no invoker found for servicekey: %v", serviceKey))
}
+ service, ok := invoker.GetURL().GetAttribute(constant.RpcServiceKey)
+ if !ok {
+ panic(fmt.Sprintf("no rpc service found in url attribute %s for servicekey: %v", constant.RpcServiceKey, serviceKey))
+ }
+ ds, ok := service.(DubboGrpcService)
+ if !ok {
+ panic("illegal service type registered")
+ }
ds.SetProxyImpl(invoker)
server.RegisterService(ds.ServiceDesc(), service)
Review Comment:
类型断言 `ds, ok := service.(DubboGrpcService)` 已经得到了 `ds`，但最终传给 `server.RegisterService` 的仍是 `service`（类型 `any`）而非 `ds`。gRPC 的 `RegisterService` 需要第二个参数满足 service descriptor handler 签名，传 `any` 类型不一定满足，运行时可能失败。原代码传的是经过 `config.GetProviderService(key)` 拿到的具体对象，类型一致性有保障。
建议改为：
```go
server.RegisterService(ds.ServiceDesc(), ds)
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901369049
##########
protocol/grpc/server.go:
##########
@@ -139,12 +121,7 @@ func (s *Server) Start(url *common.URL) {
logger.Infof("gRPC Server initialized the TLSConfig configuration")
serverOpts = append(serverOpts, grpc.Creds(credentials.NewTLS(cfg)))
}
- } else {
- serverOpts = append(serverOpts, grpc.Creds(insecure.NewCredentials()))
}
- } else {
- // TODO: remove this else
- serverOpts = append(serverOpts, grpc.Creds(insecure.NewCredentials()))
}
Review Comment:
OK 感谢建议 已经更改
<img width="1416" height="1047" alt="image" src="https://github.com/user-attachments/assets/2f179169-2b8f-44c1-bea4-3c0e3595614e" />
新逻辑总结一下是这样的 可以吗
```
transportCreds = insecure.NewCredentials() // 默认值：无 TLS
if url 有 TLSConfigKey && TLS 有效 && cfg != nil {
transportCreds = credentials.NewTLS(cfg) // 覆盖为 TLS
}
serverOpts = append(serverOpts, grpc.Creds(transportCreds)) // 只 append 一次
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901365969
##########
protocol/dubbo3/dubbo3_protocol.go:
##########
@@ -80,17 +77,16 @@ func NewDubboProtocol() *DubboProtocol {
func (dp *DubboProtocol) Export(invoker base.Invoker) base.Exporter {
url := invoker.GetURL()
serviceKey := url.ServiceKey()
- exporter := NewDubboExporter(serviceKey, invoker, dp.ExporterMap(), dp.serviceMap)
- dp.SetExporterMap(serviceKey, exporter)
- logger.Infof("[Triple Protocol] Export service: %s", url.String())
key := url.GetParam(constant.BeanNameKey, "")
var service any
- //TODO: Temporary compatibility with old APIs, can be removed later
- service = config.GetProviderService(key)
if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
service = rpcService
}
+ if service == nil {
+ logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
Review Comment:
OK 已按照建议更改
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4018597723
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901488172
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
+ if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
+ consumerService = rpcService
}
invoker := getInvoker(consumerService, conn)
+ invokerValue := reflect.Value{}
+ if invoker != nil {
+ invokerValue = reflect.ValueOf(invoker)
+ } else {
+ logger.Warnf("grpc client invoker is nil, interface=%s", key)
Review Comment:
感谢建议 已更改<img width="1076" height="453" alt="Image" src="https://github.com/user-attachments/assets/6aefeafe-18ac-4c34-8198-71d4ccfa5919" />
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2917599350
##########
protocol/grpc/server.go:
##########
@@ -139,12 +121,7 @@ func (s *Server) Start(url *common.URL) {
logger.Infof("gRPC Server initialized the TLSConfig configuration")
serverOpts = append(serverOpts, grpc.Creds(credentials.NewTLS(cfg)))
}
- } else {
- serverOpts = append(serverOpts, grpc.Creds(insecure.NewCredentials()))
}
- } else {
- // TODO: remove this else
- serverOpts = append(serverOpts, grpc.Creds(insecure.NewCredentials()))
}
Review Comment:
对
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4053605932
感谢建议 问题 4：RequestTimeout （protocol/dubbo3/dubbo3_invoker.go）已修改
<img width="1835" height="436" alt="image" src="https://github.com/user-attachments/assets/63983420-f0a6-4296-800f-23cf867d6cd6" />
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901473579
##########
protocol/grpc/server.go:
##########
@@ -243,6 +198,14 @@ func registerService(providerServices map[string]*global.ServiceConfig, server *
if invoker == nil {
panic(fmt.Sprintf("no invoker found for servicekey: %v", serviceKey))
}
+ service, ok := invoker.GetURL().GetAttribute(constant.RpcServiceKey)
+ if !ok {
+ panic(fmt.Sprintf("no rpc service found in url attribute %s for servicekey: %v", constant.RpcServiceKey, serviceKey))
+ }
+ ds, ok := service.(DubboGrpcService)
+ if !ok {
+ panic("illegal service type registered")
+ }
ds.SetProxyImpl(invoker)
server.RegisterService(ds.ServiceDesc(), service)
Review Comment:
感谢建议 已更改
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2899091949
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -72,20 +71,16 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker, error) {
rt string
consumerService any
)
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt = config.GetConsumerConfig().RequestTimeout
+ rt = global.DefaultConsumerConfig().RequestTimeout
Review Comment:
原来 `config.GetConsumerConfig().RequestTimeout` 若无配置返回空字符串，由 `url.GetParamDuration` 处理默认值，用户可以明确禁用超时（传零值）。改成 `global.DefaultConsumerConfig().RequestTimeout` 后始终是 `"3s"`，超时无法被禁用，语义变了。
建议：先检查 ConsumerConfig attribute 是否存在，不存在时才使用 `DefaultConsumerConfig()`。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901386419
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
Review Comment:
请问一下是改成
`var consumerService any = nil`这个意思吗
现在的 var consumerService any 零值就是 nil
还是别的意思
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2936177147
##########
protocol/grpc/server.go:
##########
@@ -107,23 +104,9 @@ func (s *Server) Start(url *common.URL) {
grpc.MaxSendMsgSize(maxServerSendMsgSize),
)
- // TODO: remove config TLSConfig
- // delete this branch
- tlsConfig := config.GetRootConfig().TLSConfig
- if tlsConfig != nil {
- var cfg *tls.Config
- cfg, err = config.GetServerTlsConfig(&config.TLSConfig{
- CACertFile: tlsConfig.CACertFile,
- TLSCertFile: tlsConfig.TLSCertFile,
- TLSKeyFile: tlsConfig.TLSKeyFile,
- TLSServerName: tlsConfig.TLSServerName,
- })
- if err != nil {
- return
- }
- logger.Infof("gRPC Server initialized the TLSConfig configuration")
- serverOpts = append(serverOpts, grpc.Creds(credentials.NewTLS(cfg)))
- } else if tlsConfRaw, ok := url.GetAttribute(constant.TLSConfigKey); ok {
+ var transportCreds credentials.TransportCredentials
+ transportCreds = insecure.NewCredentials()
+ if tlsConfRaw, ok := url.GetAttribute(constant.TLSConfigKey); ok {
Review Comment:
感谢建议 直接按 defer 模式修改
<img width="1614" height="753" alt="image" src="https://github.com/user-attachments/assets/9ddfb60f-ecd5-465e-bc11-9349cd7fed5e" />
<img width="1643" height="495" alt="image" src="https://github.com/user-attachments/assets/e9abe26f-ecba-4569-b587-18dcf0028eaf" />
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2936182231
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -72,20 +71,21 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker, error) {
rt string
consumerService any
)
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt = config.GetConsumerConfig().RequestTimeout
- if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey); ok {
- if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok {
- rt = consumerConf.RequestTimeout
+ rt = url.GetParam(constant.TimeoutKey, "")
Review Comment:
感谢建议 已经把多余的 url.GetParam(constant.TimeoutKey, "")删除
按照建议简化
<img width="1888" height="500" alt="image" src="https://github.com/user-attachments/assets/b38e69bf-b40f-4a6f-bb9b-25b5f4fb29aa" />
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2934595127
##########
protocol/grpc/server.go:
##########
@@ -107,23 +104,9 @@ func (s *Server) Start(url *common.URL) {
grpc.MaxSendMsgSize(maxServerSendMsgSize),
)
- // TODO: remove config TLSConfig
- // delete this branch
- tlsConfig := config.GetRootConfig().TLSConfig
- if tlsConfig != nil {
- var cfg *tls.Config
- cfg, err = config.GetServerTlsConfig(&config.TLSConfig{
- CACertFile: tlsConfig.CACertFile,
- TLSCertFile: tlsConfig.TLSCertFile,
- TLSKeyFile: tlsConfig.TLSKeyFile,
- TLSServerName: tlsConfig.TLSServerName,
- })
- if err != nil {
- return
- }
- logger.Infof("gRPC Server initialized the TLSConfig configuration")
- serverOpts = append(serverOpts, grpc.Creds(credentials.NewTLS(cfg)))
- } else if tlsConfRaw, ok := url.GetAttribute(constant.TLSConfigKey); ok {
+ var transportCreds credentials.TransportCredentials
+ transportCreds = insecure.NewCredentials()
+ if tlsConfRaw, ok := url.GetAttribute(constant.TLSConfigKey); ok {
Review Comment:
`lis` 在函数开头 `net.Listen("tcp", addr)` 创建后，TLS 处理段有两个 `return` 路径（L114 类型断言失败、L119 `GetServerTlSConfig` 出错）都不关闭 listener。
导致 listener 既不被 `server.Serve(lis)` 消费，也不被 `Close()`，端口永久占用，后续重启绑定同端口会失败。
建议在所有 early return 前加 `lis.Close()`，或用 defer 模式：
```go
lis, err := net.Listen("tcp", addr)
if err != nil {
panic(err)
}
success := false
defer func() {
if !success {
lis.Close()
}
}()
// ... TLS setup ...
// 在 goroutine 启动前:
success = true
go func() { server.Serve(lis) }()
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4009765182
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `11.94030%` with `59 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 48.02%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`929f78b`](https://app.codecov.io/gh/apache/dubbo-go/commit/929f78b22094b48e6e152c93484eb7c937ccd8a1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 749 commits behind head on develop.
| [Files with missing lines](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [protocol/dubbo3/dubbo3\_protocol.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?src=pr&el=tree&filepath=protocol%2Fdubbo3%2Fdubbo3_protocol.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvZHViYm8zL2R1YmJvM19wcm90b2NvbC5nbw==) | 0.00% | [22 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [protocol/dubbo3/dubbo3\_invoker.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?src=pr&el=tree&filepath=protocol%2Fdubbo3%2Fdubbo3_invoker.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvZHViYm8zL2R1YmJvM19pbnZva2VyLmdv) | 0.00% | [16 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [protocol/grpc/client.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?src=pr&el=tree&filepath=protocol%2Fgrpc%2Fclient.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvZ3JwYy9jbGllbnQuZ28=) | 40.00% | [8 Missing and 4 partials :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [protocol/grpc/server.go](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?src=pr&el=tree&filepath=protocol%2Fgrpc%2Fserver.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cHJvdG9jb2wvZ3JwYy9zZXJ2ZXIuZ28=) | 0.00% | [9 Missing :warning: ](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #3231 +/- ##
===========================================
+ Coverage 46.76% 48.02% +1.25%
===========================================
Files 295 463 +168
Lines 17172 33789 +16617
===========================================
+ Hits 8031 16226 +8195
- Misses 8287 16254 +7967
- Partials 854 1309 +455
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo-go/pull/3231?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#issuecomment-4009746723
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231) **Quality Gate passed**
Issues
 [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3231&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3231&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3231&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3231)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2894040070
##########
protocol/grpc/server.go:
##########
@@ -139,12 +121,7 @@ func (s *Server) Start(url *common.URL) {
logger.Infof("gRPC Server initialized the TLSConfig configuration")
serverOpts = append(serverOpts, grpc.Creds(credentials.NewTLS(cfg)))
}
- } else {
- serverOpts = append(serverOpts, grpc.Creds(insecure.NewCredentials()))
}
- } else {
- // TODO: remove this else
- serverOpts = append(serverOpts, grpc.Creds(insecure.NewCredentials()))
}
Review Comment:
这里不应该是有TLSConfigKey的时候append credentials.NewTLS(cfg)，没有的时候grpc.Creds(insecure.NewCredentials())吗
现在的逻辑是有没有都grpc.Creds(insecure.NewCredentials())
##########
protocol/dubbo3/dubbo3_protocol.go:
##########
@@ -80,17 +77,16 @@ func NewDubboProtocol() *DubboProtocol {
func (dp *DubboProtocol) Export(invoker base.Invoker) base.Exporter {
url := invoker.GetURL()
serviceKey := url.ServiceKey()
- exporter := NewDubboExporter(serviceKey, invoker, dp.ExporterMap(), dp.serviceMap)
- dp.SetExporterMap(serviceKey, exporter)
- logger.Infof("[Triple Protocol] Export service: %s", url.String())
key := url.GetParam(constant.BeanNameKey, "")
var service any
- //TODO: Temporary compatibility with old APIs, can be removed later
- service = config.GetProviderService(key)
if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
service = rpcService
}
+ if service == nil {
+ logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
Review Comment:
```suggestion
logger.Errorf("[Triple Protocol] No rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
```
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
Review Comment:
需不需要置为nil呀
##########
protocol/grpc/client.go:
##########
@@ -127,12 +108,7 @@ func NewClient(url *common.URL) (*Client, error) {
logger.Infof("Grpc Client initialized the TLSConfig configuration")
dialOpts = append(dialOpts, grpc.WithTransportCredentials(credentials.NewTLS(cfg)))
}
- } else {
- dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
- } else {
- // TODO: remove this else
- dialOpts = append(dialOpts, grpc.WithTransportCredentials(insecure.NewCredentials()))
}
Review Comment:
ditto
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901412390
##########
protocol/dubbo3/dubbo3_protocol.go:
##########
@@ -80,17 +77,16 @@ func NewDubboProtocol() *DubboProtocol {
func (dp *DubboProtocol) Export(invoker base.Invoker) base.Exporter {
url := invoker.GetURL()
serviceKey := url.ServiceKey()
- exporter := NewDubboExporter(serviceKey, invoker, dp.ExporterMap(), dp.serviceMap)
- dp.SetExporterMap(serviceKey, exporter)
- logger.Infof("[Triple Protocol] Export service: %s", url.String())
key := url.GetParam(constant.BeanNameKey, "")
var service any
- //TODO: Temporary compatibility with old APIs, can be removed later
- service = config.GetProviderService(key)
if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
service = rpcService
}
+ if service == nil {
+ logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
+ return nil
Review Comment:
感谢建议 已更改
<img width="1595" height="197" alt="image" src="https://github.com/user-attachments/assets/ca3bc952-e490-4645-b0ef-f07246991acd" />
现将 service == nil 时的处理从 logger.Errorf + return nil 改为 panic 直接暴露出原因
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "nanjiek (via GitHub)" <gi...@apache.org>.
nanjiek commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2899840318
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
+ if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
+ consumerService = rpcService
}
invoker := getInvoker(consumerService, conn)
+ invokerValue := reflect.Value{}
+ if invoker != nil {
+ invokerValue = reflect.ValueOf(invoker)
+ } else {
+ logger.Warnf("grpc client invoker is nil, interface=%s", key)
Review Comment:
```suggestion
err := fmt.Errorf("grpc client invoker is nil, interface=%s", key)
logger.Errorf("failed to get grpc client invoker: %v", err)
return nil, err
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "nanjiek (via GitHub)" <gi...@apache.org>.
nanjiek commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2899840318
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
+ if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
+ consumerService = rpcService
}
invoker := getInvoker(consumerService, conn)
+ invokerValue := reflect.Value{}
+ if invoker != nil {
+ invokerValue = reflect.ValueOf(invoker)
+ } else {
+ logger.Warnf("grpc client invoker is nil, interface=%s", key)
Review Comment:
```suggestion
logger.Errorf("[gRPC Client] Initialize failed: invoker is nil for service %s", url.ServiceKey())
return nil, fmt.Errorf("grpc invoker is nil")
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901473579
##########
protocol/grpc/server.go:
##########
@@ -243,6 +198,14 @@ func registerService(providerServices map[string]*global.ServiceConfig, server *
if invoker == nil {
panic(fmt.Sprintf("no invoker found for servicekey: %v", serviceKey))
}
+ service, ok := invoker.GetURL().GetAttribute(constant.RpcServiceKey)
+ if !ok {
+ panic(fmt.Sprintf("no rpc service found in url attribute %s for servicekey: %v", constant.RpcServiceKey, serviceKey))
+ }
+ ds, ok := service.(DubboGrpcService)
+ if !ok {
+ panic("illegal service type registered")
+ }
ds.SetProxyImpl(invoker)
server.RegisterService(ds.ServiceDesc(), service)
Review Comment:
感谢建议 已更改
![Uploading image.png…]()
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2893943234
##########
protocol/dubbo3/dubbo3_protocol.go:
##########
@@ -86,11 +83,13 @@ func (dp *DubboProtocol) Export(invoker base.Invoker) base.Exporter {
key := url.GetParam(constant.BeanNameKey, "")
var service any
- //TODO: Temporary compatibility with old APIs, can be removed later
- service = config.GetProviderService(key)
if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
service = rpcService
}
+ if service == nil {
+ logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
+ return nil
+ }
Review Comment:
已更改：
现在先完成 service 和序列化校验，全部通过后才 SetExporterMap。
效果：失败时不再污染 ExporterMap
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901393416
##########
protocol/grpc/client.go:
##########
@@ -93,24 +88,10 @@ func NewClient(url *common.URL) (*Client, error) {
grpc.MaxCallRecvMsgSize(maxCallRecvMsgSize),
grpc.MaxCallSendMsgSize(maxCallSendMsgSize),
),
+ grpc.WithTransportCredentials(insecure.NewCredentials()),
Review Comment:
好的 感谢建议 请问一下这样更改可以吗
<img width="1416" height="1055" alt="image" src="https://github.com/user-attachments/assets/6bcb347d-5bea-422e-8d3f-11bbef471b2b" />
简单总结一下就是
```
var transportCreds credentials.TransportCredentials
transportCreds = insecure.NewCredentials() // 默认值
if url 有 TLSConfigKey && TLS 有效 && cfg != nil {
transportCreds = credentials.NewTLS(cfg) // 覆盖，不是 append
}
dialOpts = append(dialOpts, grpc.WithTransportCredentials(transportCreds)) // 只 append 一次
```
如果不行的话我就直接改回去了
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2901430939
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -72,20 +71,16 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker, error) {
rt string
consumerService any
)
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt = config.GetConsumerConfig().RequestTimeout
+ rt = global.DefaultConsumerConfig().RequestTimeout
Review Comment:
感谢建议 目前已经是这个逻辑了 可以看看是否有问题
```
rt = global.DefaultConsumerConfig().RequestTimeout // 先用默认值 "3s"
if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey); ok {
if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok {
rt = consumerConf.RequestTimeout // 有用户配置则覆盖
}
}
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "nanjiek (via GitHub)" <gi...@apache.org>.
nanjiek commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2899839070
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
+ if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
+ consumerService = rpcService
}
invoker := getInvoker(consumerService, conn)
+ invokerValue := reflect.Value{}
+ if invoker != nil {
+ invokerValue = reflect.ValueOf(invoker)
+ } else {
+ logger.Warnf("grpc client invoker is nil, interface=%s", key)
+ }
Review Comment:
logger只能记录，程序在初始化 invoker（RPC 调用执行器）时发现对象为 nil应该终止，返回 error，阻止 Client 被创建。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2917602820
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
Review Comment:
是这个意思
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "Snow-kal (via GitHub)" <gi...@apache.org>.
Snow-kal commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2936187073
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -112,19 +112,24 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker, error) {
opts = append(opts, triConfig.WithGRPCMaxCallRecvMessageSize(maxCallRecvMsgSize))
opts = append(opts, triConfig.WithGRPCMaxCallSendMessageSize(maxCallSendMsgSize))
- //TODO: Temporary compatibility with old APIs, can be removed later
- tracingKey := url.GetParam(constant.TracingConfigKey, "")
- if tracingKey != "" {
- tracingConfig := config.GetTracingConfig(tracingKey)
- if tracingConfig != nil {
+ if tracingConfRaw, ok := url.GetAttribute(constant.TracingConfigKey); ok {
+ tracingConfig, ok := tracingConfRaw.(*global.TracingConfig)
+ if !ok {
+ logger.Warnf("invalid tracing config type %T, expected *global.TracingConfig", tracingConfRaw)
+ } else if tracingConfig != nil {
if tracingConfig.Name == "jaeger" {
- if tracingConfig.ServiceName == "" {
- tracingConfig.ServiceName = config.GetApplicationConfig().Name
+ serviceName := tracingConfig.ServiceName
Review Comment:
感谢建议 dubbo3_invoker.go 和 dubbo3_protocol.go 均已修改
（以下截图 第一张是dubbo3_invoker.go 第二张是 dubbo3_protocol.go ）
<img width="1835" height="888" alt="image" src="https://github.com/user-attachments/assets/1b4b28d5-6fbf-4be8-a658-b6e3925a9ccc" />
<img width="2085" height="1064" alt="image" src="https://github.com/user-attachments/assets/2d1a2b3c-a857-4933-93c7-288df7ee34c4" />
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2934602695
##########
protocol/grpc/client.go:
##########
@@ -93,24 +88,10 @@ func NewClient(url *common.URL) (*Client, error) {
grpc.MaxCallRecvMsgSize(maxCallRecvMsgSize),
grpc.MaxCallSendMsgSize(maxCallSendMsgSize),
),
+ grpc.WithTransportCredentials(insecure.NewCredentials()),
Review Comment:
确认已修复。改为 `transportCreds` 变量先赋默认值 `insecure.NewCredentials()`，有合法 TLS config 时覆盖，最后只 append 一次。互斥语义正确，消除了 fail-open 风险。
##########
protocol/grpc/client.go:
##########
@@ -142,25 +118,25 @@ func NewClient(url *common.URL) (*Client, error) {
}
key := url.GetParam(constant.InterfaceKey, "")
- //TODO: Temporary compatibility with old APIs, can be removed later
- consumerService := config.GetConsumerServiceByInterfaceName(key)
- if consumerService == nil {
- if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
- consumerService = rpcService
- }
+ var consumerService any
+ if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
+ consumerService = rpcService
}
invoker := getInvoker(consumerService, conn)
Review Comment:
确认已修复。两道防护都加上了：consumerService 为 nil 时提前 return error + close conn，getInvoker 返回 nil 时同样处理。消除了延迟 panic 的问题。
##########
protocol/dubbo3/dubbo3_protocol.go:
##########
@@ -80,17 +77,16 @@ func NewDubboProtocol() *DubboProtocol {
func (dp *DubboProtocol) Export(invoker base.Invoker) base.Exporter {
url := invoker.GetURL()
serviceKey := url.ServiceKey()
- exporter := NewDubboExporter(serviceKey, invoker, dp.ExporterMap(), dp.serviceMap)
- dp.SetExporterMap(serviceKey, exporter)
- logger.Infof("[Triple Protocol] Export service: %s", url.String())
key := url.GetParam(constant.BeanNameKey, "")
var service any
- //TODO: Temporary compatibility with old APIs, can be removed later
- service = config.GetProviderService(key)
if rpcService, ok := url.GetAttribute(constant.RpcServiceKey); ok {
service = rpcService
}
+ if service == nil {
+ logger.Errorf("no rpc service found in url attribute %s for service key: %s", constant.RpcServiceKey, key)
+ return nil
Review Comment:
确认已修复。改为 panic 并附带明确错误信息，同时 `SetExporterMap` 移到所有校验之后，失败时不再污染 ExporterMap。比原来的 return nil 好得多——排查时能立即看到根因。
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -72,20 +71,16 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker, error) {
rt string
consumerService any
)
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt = config.GetConsumerConfig().RequestTimeout
+ rt = global.DefaultConsumerConfig().RequestTimeout
Review Comment:
优先级链改对了（URL param > ConsumerConfig > 默认值），比之前硬编码 `global.DefaultConsumerConfig().RequestTimeout` 好。但第一行 `url.GetParam(constant.TimeoutKey, "")` 是冗余的——`GetParamDuration` 内部已经做了同样的 `GetParam` 调用，见我新提的 comment。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2934602856
##########
protocol/grpc/server.go:
##########
@@ -243,6 +198,14 @@ func registerService(providerServices map[string]*global.ServiceConfig, server *
if invoker == nil {
panic(fmt.Sprintf("no invoker found for servicekey: %v", serviceKey))
}
+ service, ok := invoker.GetURL().GetAttribute(constant.RpcServiceKey)
+ if !ok {
+ panic(fmt.Sprintf("no rpc service found in url attribute %s for servicekey: %v", constant.RpcServiceKey, serviceKey))
+ }
+ ds, ok := service.(DubboGrpcService)
+ if !ok {
+ panic("illegal service type registered")
+ }
ds.SetProxyImpl(invoker)
server.RegisterService(ds.ServiceDesc(), service)
Review Comment:
确认已修复。`server.RegisterService(ds.ServiceDesc(), ds)` 类型一致，消除了 `any` 类型不匹配的风险。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] Refactor: remove config in grpc and dubbo3 [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3231:
URL: https://github.com/apache/dubbo-go/pull/3231#discussion_r2934595438
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -72,20 +71,21 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker, error) {
rt string
consumerService any
)
- // TODO: Temporary compatibility with old APIs, can be removed later
- rt = config.GetConsumerConfig().RequestTimeout
- if consumerConfRaw, ok := url.GetAttribute(constant.ConsumerConfigKey); ok {
- if consumerConf, ok := consumerConfRaw.(*global.ConsumerConfig); ok {
- rt = consumerConf.RequestTimeout
+ rt = url.GetParam(constant.TimeoutKey, "")
Review Comment:
这一行 `url.GetParam(constant.TimeoutKey, "")` 是多余的。下面 `url.GetParamDuration(constant.TimeoutKey, rt)` 内部已经调用了 `GetParam(TimeoutKey, rt)`，如果 URL 有 TimeoutKey 参数会优先使用。
`rt` 变量只需要作为 `GetParamDuration` 的 fallback default，简化为：
```go
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
}
timeout := url.GetParamDuration(constant.TimeoutKey, rt)
```
##########
protocol/dubbo3/dubbo3_invoker.go:
##########
@@ -112,19 +112,24 @@ func NewDubboInvoker(url *common.URL) (*DubboInvoker, error) {
opts = append(opts, triConfig.WithGRPCMaxCallRecvMessageSize(maxCallRecvMsgSize))
opts = append(opts, triConfig.WithGRPCMaxCallSendMessageSize(maxCallSendMsgSize))
- //TODO: Temporary compatibility with old APIs, can be removed later
- tracingKey := url.GetParam(constant.TracingConfigKey, "")
- if tracingKey != "" {
- tracingConfig := config.GetTracingConfig(tracingKey)
- if tracingConfig != nil {
+ if tracingConfRaw, ok := url.GetAttribute(constant.TracingConfigKey); ok {
+ tracingConfig, ok := tracingConfRaw.(*global.TracingConfig)
+ if !ok {
+ logger.Warnf("invalid tracing config type %T, expected *global.TracingConfig", tracingConfRaw)
+ } else if tracingConfig != nil {
if tracingConfig.Name == "jaeger" {
- if tracingConfig.ServiceName == "" {
- tracingConfig.ServiceName = config.GetApplicationConfig().Name
+ serviceName := tracingConfig.ServiceName
Review Comment:
`dubbo3_invoker.go` 和 `dubbo3_protocol.go` 两处相同逻辑：如果 `tracingConfig.ServiceName` 和 URL 参数 `ApplicationKey` 都为空，`WithJaegerConfig(address, "", useAgent)` 会传入空 service name，Jaeger 收到的 trace 数据无法按服务归类。
建议空时跳过或至少打个 warn：
```go
if serviceName == "" {
logger.Warnf("jaeger tracing skipped: no service name available for %s", url.String())
} else {
opts = append(opts, triConfig.WithJaegerConfig(tracingConfig.Address, serviceName, useAgent))
}
```
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org