AlexStocks commented on code in PR #3313:
URL: https://github.com/apache/dubbo-go/pull/3313#discussion_r3610132824
##########
cluster/router/polaris/router_test.go:
##########
@@ -110,3 +260,173 @@ func
TestNewPolarisRouterInvalidRegistriesTypeShouldFallback(t *testing.T) {
require.NotNil(t, r.Registries)
require.Empty(t, r.Registries)
}
+
+// --- Route() regression tests for issue #3303 ---
+
+func TestRouteOpenRouteDisabledReturnsOriginalInvokers(t *testing.T) {
+ r := &polarisRouter{
+ openRoute: false,
+ Registries: map[string]*global.RegistryConfig{},
+ }
+ url := mustNewURL(t, baseServiceURL+"?interface=com.xxx.Service")
+ invokers := []base.Invoker{newMockInvokerWithInstanceID(t, "inst-1")}
+
+ got := r.Route(invokers, url, &mockInvocation{})
+ assert.Equal(t, invokers, got, "should return original invokers when
openRoute is false")
+}
+
+func TestRouteEmptyInvokersReturnsEmpty(t *testing.T) {
+ r := &polarisRouter{
+ openRoute: true,
+ Registries: map[string]*global.RegistryConfig{},
+ }
+ url := mustNewURL(t, baseServiceURL+"?interface=com.xxx.Service")
+
+ got := r.Route(nil, url, &mockInvocation{})
+ assert.Empty(t, got, "should return empty when input invokers is nil")
+
+ got = r.Route([]base.Invoker{}, url, &mockInvocation{})
+ assert.Empty(t, got, "should return empty when input invokers is empty")
+}
+
+func TestRouteGetAllInstancesErrorReturnsOriginalInvokers(t *testing.T) {
+ consumerAPI := &mockConsumerAPI{
+ allInstancesResp: nil,
+ allInstancesErr: errors.New("polaris unavailable"),
+ }
+ routerAPI := &mockRouterAPI{}
+ r := newTestRouter(t, routerAPI, consumerAPI)
+ url := mustNewURL(t, baseServiceURL+"?interface=com.xxx.Service")
+ invokers := []base.Invoker{newMockInvokerWithInstanceID(t, "inst-1")}
+
+ got := r.Route(invokers, url, &mockInvocation{})
+ assert.Equal(t, invokers, got, "should return original invokers when
GetAllInstances fails")
+}
+
+func TestRouteProcessRoutersErrorReturnsOriginalInvokers(t *testing.T) {
+ inst := &mockInstance{id: "inst-1"}
+ consumerAPI := &mockConsumerAPI{
+ allInstancesResp: &model.InstancesResponse{
+ Instances: []model.Instance{inst},
+ },
+ }
+ engine := &mockEngine{
+ routeRule: &model.ServiceRuleResponse{
+ Value: &v1.Routing{},
+ },
+ }
+ sdkCtx := &mockSDKContext{engine: engine}
+ routerAPI := &mockRouterAPI{
+ sdkCtx: sdkCtx,
+ processErr: errors.New("route processing failed"),
+ }
+ r := newTestRouter(t, routerAPI, consumerAPI)
+ url := mustNewURL(t, baseServiceURL+"?interface=com.xxx.Service")
+ invokers := []base.Invoker{newMockInvokerWithInstanceID(t, "inst-1")}
+
+ got := r.Route(invokers, url, &mockInvocation{})
+ assert.Equal(t, invokers, got, "should return original invokers when
ProcessRouters fails")
+}
+
+// TestRouteRuleNotMatchFallbackToOriginalInvokers is the key regression test
for issue #3303.
+// When ProcessRouters returns instances that don't match any known invoker
(route rule not match),
+// Route() should return the original invokers instead of an empty slice to
prevent nil pointer panic.
+func TestRouteRuleNotMatchFallbackToOriginalInvokers(t *testing.T) {
+ inst := &mockInstance{id: "inst-1"}
+ consumerAPI := &mockConsumerAPI{
+ allInstancesResp: &model.InstancesResponse{
+ Instances: []model.Instance{inst},
+ },
+ }
+ engine := &mockEngine{
+ routeRule: &model.ServiceRuleResponse{
+ Value: &v1.Routing{},
+ },
+ }
+ sdkCtx := &mockSDKContext{engine: engine}
+ // ProcessRouters returns instances with IDs that don't exist in
invokersMap
+ nonMatchingInst := &mockInstance{id: "unknown-inst-999"}
+ routerAPI := &mockRouterAPI{
+ sdkCtx: sdkCtx,
+ processResp: &model.InstancesResponse{
+ Instances: []model.Instance{nonMatchingInst},
+ },
+ }
+ r := newTestRouter(t, routerAPI, consumerAPI)
+ url := mustNewURL(t, baseServiceURL+"?interface=com.xxx.Service")
+ invokers := []base.Invoker{newMockInvokerWithInstanceID(t, "inst-1")}
+
+ var got []base.Invoker
+ require.NotPanics(t, func() {
Review Comment:
[P1] 这个测试没有复现 Issue 描述的 panic 调用链
`polarisRouter.Route` 本身只构造并返回切片，当前测试直接调用它再包一层 `NotPanics`，即使删除本 PR 的 5
行生产修改也不会在这里触发 Issue 所称的 cluster/load-balance 空切片 panic；它只能证明作者设定的 fallback
返回值。仓库的 `BaseClusterInvoker.CheckInvokers` 已对空列表返回明确错误，因此必须先用真实 RouterChain +
具体 cluster invoker 复现 #3303 的栈，再在正确层修复。建议补一个从 Directory.List/RouterChain 进入
failfast/failover 等调用路径的回归测试，断言不 panic 且返回 `no provider`/route-not-match
错误，而不是把原始 providers 放回去。否则当前测试既未证明根因，也把绕过路由规则的行为固化成期望。
--
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]
