AlexStocks commented on code in PR #3284:
URL: https://github.com/apache/dubbo-go/pull/3284#discussion_r3039389591
##########
proxy/proxy_factory/utils.go:
##########
@@ -26,12 +26,21 @@ import (
perrors "github.com/pkg/errors"
)
-// CallLocalMethod is used to handle invoke exception in user func.
+// callLocalMethod invokes method with the given arguments, recovering from
panics.
+// For variadic methods where the last argument is already a typed slice, it
uses
+// CallSlice to correctly expand the slice into variadic parameters.
func callLocalMethod(method reflect.Method, in []reflect.Value)
([]reflect.Value, error) {
var (
returnValues []reflect.Value
retErr error
)
+ useCallSlice := false
Review Comment:
[P1] `callLocalMethod` 现在只要目标方法是 variadic，且最后一个实参恰好是 slice/array，就会直接走
`CallSlice`。这个判断范围太宽，不只 generic variadic
修复链路会命中，普通本地调用里“最后一个参数本来就是切片”的场景也会被误判成“需要展开 variadic
尾参”，调用语义会被改掉。这个分支至少需要带上更严格的前置条件，只在明确收到 generic filter 预打包后的 variadic 尾参时才切到
`CallSlice`，并补一个非 generic 的回归测试证明不会误伤普通调用。
##########
filter/generic/service_filter.go:
##########
@@ -78,44 +80,126 @@ func (f *genericServiceFilter) Invoke(ctx context.Context,
invoker base.Invoker,
`, mtdName, types, args)
// get the type of the argument
- ivkUrl := invoker.GetURL()
- svc := common.ServiceMap.GetServiceByServiceKey(ivkUrl.Protocol,
ivkUrl.ServiceKey())
+ ivkURL := invoker.GetURL()
+ svc := common.ServiceMap.GetServiceByServiceKey(ivkURL.Protocol,
ivkURL.ServiceKey())
method := svc.Method()[mtdName]
if method == nil {
return &result.RPCResult{
- Err: perrors.Errorf("\"%s\" method is not found,
service key: %s", mtdName, ivkUrl.ServiceKey()),
+ Err: perrors.Errorf("\"%s\" method is not found,
service key: %s", mtdName, ivkURL.ServiceKey()),
}
}
+
argsType := method.ArgsType()
+ if err := validateGenericArgs(method.IsVariadic(), len(argsType),
len(args), mtdName); err != nil {
Review Comment:
[P1] 这次修的是 generic variadic 的核心执行路径，但 PR 里没有看到对应回归测试。现在新增了 variadic
参数数量校验、离散参数/数组参数归一化，以及 `CallSlice` 调度分支；如果没有覆盖 Java 侧两种传参形式（离散 args、单个数组 args）和
`...interface{}` 场景，后续很容易再回归成 panic 或错误打包。建议至少补 3 组测试：1）generic variadic +
离散参数；2）generic variadic + 单个数组参数；3）`...interface{}` 不会被多包一层。
--
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]
