You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "cvictory (via GitHub)" <gi...@apache.org> on 2026/04/29 11:06:58 UTC

[PR] fix(polaris): prevent nil pointer panic when route rules don't match [dubbo-go]

cvictory opened a new pull request, #3313:
URL: https://github.com/apache/dubbo-go/pull/3313

   ## Bug
   - **Symptom**: When Polaris only has uid-based route rules without discovery fallback, consumer gets `RouteRuleNotMatch` followed by nil pointer panic.
   - **Root cause**: `Route()` in `cluster/router/polaris/router.go` returns an empty invoker slice when `ProcessRouters` returns instances that don't map to any known invoker. The empty slice propagates to cluster invokers (failfast, failover, etc.) which access `invokers[0]`, causing nil pointer panic.
   
   ## Fix
   After building the result from `ProcessRouters` response, if the result is empty (no matching invokers), log a warning and return the original invokers as a safe fallback. This follows the same defensive pattern used in 5 other error paths in the same method.
   
   **Changed files:**
   - `cluster/router/polaris/router.go` — 4-line empty-result guard
   - `cluster/router/polaris/router_test.go` — 7 new regression tests with mock infrastructure
   
   ## Verification
   - Tests: 13/13 pass (6 existing + 7 new)
   - `go vet`: clean
   - `go build`: clean
   - Key regression tests:
     - `TestRouteRuleNotMatchFallbackToOriginalInvokers` — non-matching instances → fallback
     - `TestRouteEmptyProcessRoutersResultFallbackToOriginalInvokers` — empty result → fallback
     - `TestRouteHappyPathReturnsMatchedInvokers` — matching instances → filtered (behavior preserved)
   
   Closes #3303


-- 
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] fix(polaris): prevent nil pointer panic when route rules don't match [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3313:
URL: https://github.com/apache/dubbo-go/pull/3313#discussion_r3563491807


##########
cluster/router/polaris/router_test.go:
##########
@@ -18,19 +18,169 @@
 package polaris

Review Comment:
   [P1] 新增回归测试未通过仓库格式检查
   
   当前 head 的必需 CI 在 `make check-fmt` 阶段失败，并明确指出 `cluster/router/polaris/router_test.go` 会被 formatter 修改。请运行仓库规定的 `make fmt` 并提交格式化结果；CI 恢复通过前不能合并。



-- 
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] fix(polaris): prevent nil pointer panic when route rules don't match [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3313:
URL: https://github.com/apache/dubbo-go/pull/3313#discussion_r3161590801


##########
cluster/router/polaris/router.go:
##########
@@ -188,6 +188,11 @@ func (p *polarisRouter) Route(invokers []base.Invoker, url *common.URL,
 		}
 	}
 
+	if len(ret) == 0 {

Review Comment:
   [P0] `len(ret)==0` 时直接回退到 `invokers` 会绕过真正生效的 Polaris 路由规则。`ret` 为空并不只代表“实例 ID 映射不上”，也可能是路由规则故意把当前请求全部过滤掉；这时正确行为应该是返回空结果并让上层按“无可用 provider”处理，而不是把原始 invokers 全部放回来继续发流量。现在的实现会把本应被路由规则拦掉的请求重新打到 provider 上，属于治理语义错误。



-- 
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] fix(polaris): prevent nil pointer panic when route rules don't match [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
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: 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] fix(polaris): prevent nil pointer panic when route rules don't match [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks closed pull request #3313: fix(polaris): prevent nil pointer panic when route rules don't match
URL: https://github.com/apache/dubbo-go/pull/3313


-- 
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] fix(polaris): prevent nil pointer panic when route rules don't match [dubbo-go]

Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3313:
URL: https://github.com/apache/dubbo-go/pull/3313#discussion_r3161601936


##########
cluster/router/polaris/router.go:
##########
@@ -188,6 +188,11 @@ func (p *polarisRouter) Route(invokers []base.Invoker, url *common.URL,
 		}
 	}
 
+	if len(ret) == 0 {

Review Comment:
   [P0] `len(ret)==0` 时直接回退到 `invokers` 会绕过真正生效的 Polaris 路由规则。`ret` 为空并不只代表“实例 ID 映射不上”，也可能是路由规则故意把当前请求全部过滤掉；这时正确行为应该是返回空结果并让上层按“无可用 provider”处理，而不是把原始 invokers 全部放回来继续发流量。现在的实现会把本应被路由规则拦掉的请求重新打到 provider 上，属于治理语义错误。



-- 
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] fix(polaris): prevent nil pointer panic when route rules don't match [dubbo-go]

Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3313:
URL: https://github.com/apache/dubbo-go/pull/3313#issuecomment-4343125797

   ## [![Quality Gate Passed](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/checks/QualityGateBadge/qg-passed-20px.png 'Quality Gate Passed')](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3313) **Quality Gate passed**  
   Issues  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [1 New issue](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3313&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/accepted-16px.png '') [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3313&issueStatuses=ACCEPTED)
   
   Measures  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3313&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3313&metric=new_coverage&view=list)  
   ![](https://sonarsource.github.io/sonarcloud-github-static-resources/v2/common/passed-16px.png '') [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3313&metric=new_duplicated_lines_density&view=list)  
     
   [See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3313)
   
   


-- 
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