From 47bba48c19690f5123bdc233958d2852c8e16f1a Mon Sep 17 00:00:00 2001
From: watermelo <80680489@qq.com>
Date: Fri, 22 May 2020 17:06:03 +0800
Subject: [PATCH] Opt: optimize error handling

---
 config/metadata_report_config.go     |  2 +-
 registry/directory/directory.go      | 17 ++++++++++++-----
 registry/directory/directory_test.go | 16 ++++++++++++++++
 3 files changed, 29 insertions(+), 6 deletions(-)

diff --git a/config/metadata_report_config.go b/config/metadata_report_config.go
index 41fb6b476..2b926c0b9 100644
--- a/config/metadata_report_config.go
+++ b/config/metadata_report_config.go
@@ -103,7 +103,7 @@ func startMetadataReport(metadataType string, metadataReportConfig *MetadataRepo
 	if url, err := metadataReportConfig.ToUrl(); err == nil {
 		instance.GetMetadataReportInstance(url)
 	} else {
-		return perrors.New("MetadataConfig is invalid!")
+		return perrors.Wrap(err, "Start MetadataReport failed.")
 	}
 
 	return nil
diff --git a/registry/directory/directory.go b/registry/directory/directory.go
index 552aa5706..322ae9afd 100644
--- a/registry/directory/directory.go
+++ b/registry/directory/directory.go
@@ -152,9 +152,11 @@ func (dir *RegistryDirectory) refreshInvokers(res *registry.ServiceEvent) {
 }
 
 func (dir *RegistryDirectory) toGroupInvokers() []protocol.Invoker {
-	newInvokersList := []protocol.Invoker{}
+	var (
+		err             error
+		newInvokersList []protocol.Invoker
+	)
 	groupInvokersMap := make(map[string][]protocol.Invoker)
-	groupInvokersList := []protocol.Invoker{}
 
 	dir.cacheInvokersMap.Range(func(key, value interface{}) bool {
 		newInvokersList = append(newInvokersList, value.(protocol.Invoker))
@@ -170,6 +172,7 @@ func (dir *RegistryDirectory) toGroupInvokers() []protocol.Invoker {
 			groupInvokersMap[group] = []protocol.Invoker{invoker}
 		}
 	}
+	groupInvokersList := make([]protocol.Invoker, 0, len(groupInvokersMap))
 	if len(groupInvokersMap) == 1 {
 		//len is 1 it means no group setting ,so do not need cluster again
 		for _, invokers := range groupInvokersMap {
@@ -178,9 +181,13 @@ func (dir *RegistryDirectory) toGroupInvokers() []protocol.Invoker {
 	} else {
 		for _, invokers := range groupInvokersMap {
 			staticDir := directory.NewStaticDirectory(invokers)
-			cluster := extension.GetCluster(dir.GetUrl().SubURL.GetParam(constant.CLUSTER_KEY, constant.DEFAULT_CLUSTER))
-			staticDir.BuildRouterChain(invokers)
-			groupInvokersList = append(groupInvokersList, cluster.Join(staticDir))
+			cst := extension.GetCluster(dir.GetUrl().SubURL.GetParam(constant.CLUSTER_KEY, constant.DEFAULT_CLUSTER))
+			err = staticDir.BuildRouterChain(invokers)
+			if err != nil {
+				logger.Error(err)
+				continue
+			}
+			groupInvokersList = append(groupInvokersList, cst.Join(staticDir))
 		}
 	}
 
diff --git a/registry/directory/directory_test.go b/registry/directory/directory_test.go
index f1d5ce434..f70fce175 100644
--- a/registry/directory/directory_test.go
+++ b/registry/directory/directory_test.go
@@ -18,6 +18,8 @@
 package directory
 
 import (
+	"github.com/apache/dubbo-go/protocol/mock"
+	"github.com/golang/mock/gomock"
 	"net/url"
 	"strconv"
 	"testing"
@@ -169,7 +171,21 @@ Loop1:
 			break Loop1
 		}
 	}
+}
 
+func Test_toGroupInvokers(t *testing.T) {
+	registryDirectory, _ := normalRegistryDir()
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+	invoker := mock.NewMockInvoker(ctrl)
+	newUrl, _ := common.NewURL("dubbo://192.168.1.1:20000/com.ikurento.user.UserProvider")
+	invoker.EXPECT().GetUrl().Return(newUrl).AnyTimes()
+
+	registryDirectory.cacheInvokersMap.Store("group1", invoker)
+	registryDirectory.cacheInvokersMap.Store("group2", invoker)
+	registryDirectory.cacheInvokersMap.Store("group1", invoker)
+	groupInvokers := registryDirectory.toGroupInvokers()
+	assert.Len(t, groupInvokers, 2)
 }
 
 func normalRegistryDir(noMockEvent ...bool) (*RegistryDirectory, *registry.MockRegistry) {
-- 
GitLab