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: [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]

Reply via email to