From c89fb0052590c90070de610894f2f75e46528b1f Mon Sep 17 00:00:00 2001 From: xiegang Date: Wed, 27 Dec 2023 13:34:51 +0800 Subject: [PATCH 1/4] Failfast for explicit error http request instead of retrying (#296) - error http status code: 400, 401, 404, 405 - Error Code Description: https://www.apolloconfig.com/#/en/usage/other-language-client-user-guide?id=_16-error-code-description --- protocol/http/request.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/protocol/http/request.go b/protocol/http/request.go index 1c4a00e..51bb4b4 100644 --- a/protocol/http/request.go +++ b/protocol/http/request.go @@ -79,7 +79,7 @@ func getDefaultTransport(insecureSkipVerify bool) *http.Transport { return defaultTransport } -//CallBack 请求回调函数 +// CallBack 请求回调函数 type CallBack struct { SuccessCallBack func([]byte, CallBack) (interface{}, error) NotModifyCallBack func() error @@ -87,7 +87,7 @@ type CallBack struct { Namespace string } -//Request 建立网络请求 +// Request 建立网络请求 func Request(requestURL string, connectionConfig *env.ConnectConfig, callBack *CallBack) (interface{}, error) { client := &http.Client{} //如有设置自定义超时时间即使用 @@ -175,6 +175,9 @@ func Request(requestURL string, connectionConfig *env.ConnectConfig, callBack *C return nil, callBack.NotModifyCallBack() } return nil, nil + case http.StatusBadRequest, http.StatusUnauthorized, http.StatusNotFound, http.StatusMethodNotAllowed: + log.Errorf("Connect Apollo Server Fail, url:%s, StatusCode:%d", requestURL, res.StatusCode) + return nil, errors.New(fmt.Sprintf("Connect Apollo Server Fail, StatusCode:%d", res.StatusCode)) default: log.Errorf("Connect Apollo Server Fail, url:%s, StatusCode:%d", requestURL, res.StatusCode) // if error then sleep @@ -190,7 +193,7 @@ func Request(requestURL string, connectionConfig *env.ConnectConfig, callBack *C return nil, err } -//RequestRecovery 可以恢复的请求 +// RequestRecovery 可以恢复的请求 func RequestRecovery(appConfig config.AppConfig, connectConfig *env.ConnectConfig, callBack *CallBack) (interface{}, error) { From c717e75ba81f3b5e9f5eaa705972ac2fd155f3be Mon Sep 17 00:00:00 2001 From: larry4xie Date: Thu, 28 Dec 2023 13:41:16 +0800 Subject: [PATCH 2/4] Add unit tests for: Failfast for explicit error http request instead of retrying (#296) --- protocol/http/request_server_test.go | 18 +++++++++---- protocol/http/request_test.go | 40 +++++++++++++++++++++++++++- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/protocol/http/request_server_test.go b/protocol/http/request_server_test.go index 7499857..71e3ca0 100644 --- a/protocol/http/request_server_test.go +++ b/protocol/http/request_server_test.go @@ -50,10 +50,10 @@ var servicesResponseStr = `[{ var normalBackupConfigCount = 0 -//Normal response -//First request will hold 5s and response http.StatusNotModified -//Second request will hold 5s and response http.StatusNotModified -//Second request will response [{"namespaceName":"application","notificationId":3}] +// Normal response +// First request will hold 5s and response http.StatusNotModified +// Second request will hold 5s and response http.StatusNotModified +// Second request will response [{"namespaceName":"application","notificationId":3}] func runNormalBackupConfigResponse() *httptest.Server { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { normalBackupConfigCount++ @@ -84,7 +84,7 @@ func runNormalBackupConfigResponseWithHTTPS() *httptest.Server { return ts } -//wait long time then response +// wait long time then response func runLongTimeResponse() *httptest.Server { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { time.Sleep(10 * time.Second) @@ -94,3 +94,11 @@ func runLongTimeResponse() *httptest.Server { return ts } + +func runStatusCodeResponse(status int) *httptest.Server { + ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(status) + })) + + return ts +} diff --git a/protocol/http/request_test.go b/protocol/http/request_test.go index 66e2b30..4377449 100644 --- a/protocol/http/request_test.go +++ b/protocol/http/request_test.go @@ -19,6 +19,7 @@ package http import ( "fmt" + "net/http" "net/url" "testing" "time" @@ -133,6 +134,43 @@ func TestCustomTimeout(t *testing.T) { Assert(t, o, NilVal()) } +func TestFailFastStatusCode(t *testing.T) { + time.Sleep(1 * time.Second) + + tests := []struct { + name string + status int + }{ + {name: "400", status: http.StatusBadRequest}, + {name: "401", status: http.StatusUnauthorized}, + {name: "404", status: http.StatusNotFound}, + {name: "405", status: http.StatusMethodNotAllowed}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testFailFastStatusCode(t, tt.status) + }) + } +} + +func testFailFastStatusCode(t *testing.T, status int) { + server := runStatusCodeResponse(status) + appConfig := getTestAppConfig() + appConfig.IP = server.URL + + startTime := time.Now().Unix() + _, err := RequestRecovery(*appConfig, &env.ConnectConfig{ + URI: getConfigURLSuffix(appConfig, appConfig.NamespaceName), + IsRetry: true, + }, &CallBack{ + SuccessCallBack: nil, + }) + duration := time.Now().Unix() - startTime + + Assert(t, err, NotNilVal()) + Assert(t, int64(0), Equal(duration)) +} + func mockIPList(t *testing.T, appConfigFunc func() config.AppConfig) { time.Sleep(1 * time.Second) @@ -159,7 +197,7 @@ func getConfigURLSuffix(config *config.AppConfig, namespaceName string) string { utils.GetInternal()) } -//SyncServerIPListSuccessCallBack 同步服务器列表成功后的回调 +// SyncServerIPListSuccessCallBack 同步服务器列表成功后的回调 func SyncServerIPListSuccessCallBack(responseBody []byte, callback CallBack) (o interface{}, err error) { log.Debugf("get all server info: %s", string(responseBody)) From 126c7016dc42580d9761fefa6400f058c7a80a46 Mon Sep 17 00:00:00 2001 From: larry4xie Date: Fri, 29 Dec 2023 09:14:32 +0800 Subject: [PATCH 3/4] Fix other unit tests for: Failfast for explicit error http request instead of retrying (#296) --- component/remote/async_test.go | 31 ++++++++++++++++++++++++------- 1 file changed, 24 insertions(+), 7 deletions(-) diff --git a/component/remote/async_test.go b/component/remote/async_test.go index dcb428e..439aa1c 100644 --- a/component/remote/async_test.go +++ b/component/remote/async_test.go @@ -146,7 +146,7 @@ func initMockNotifyAndConfigServerWithTwoErrResponse() *httptest.Server { return runMockConfigServer(handlerMap, onlynormaltworesponse) } -//run mock config server +// run mock config server func runMockConfigServer(handlerMap map[string]func(http.ResponseWriter, *http.Request), notifyHandler func(http.ResponseWriter, *http.Request)) *httptest.Server { appConfig := env.InitFileConfig() @@ -177,11 +177,12 @@ func initNotifications() *config.AppConfig { return appConfig } -//Error response -//will hold 5s and keep response 404 -func runErrorResponse() *httptest.Server { +// Error response +// will hold 5s and keep response 404 +func runErrorResponse(status int, body []byte) *httptest.Server { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - w.WriteHeader(http.StatusNotFound) + w.WriteHeader(status) + _, _ = w.Write(body) })) return ts @@ -281,10 +282,26 @@ func TestGetRemoteConfig(t *testing.T) { } func TestErrorGetRemoteConfig(t *testing.T) { + tests := []struct { + name string + status int + errContained string + }{ + {name: "500", status: http.StatusInternalServerError, errContained: "over Max Retry Still Error"}, + {name: "404", status: http.StatusNotFound, errContained: "Connect Apollo Server Fail"}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + testErrorGetRemoteConfig(t, tt.status, tt.errContained) + }) + } +} + +func testErrorGetRemoteConfig(t *testing.T, status int, errContained string) { //clear initNotifications() appConfig := initNotifications() - server1 := runErrorResponse() + server1 := runErrorResponse(status, nil) appConfig.IP = server1.URL server.SetNextTryConnTime(appConfig.GetHost(), 0) @@ -303,7 +320,7 @@ func TestErrorGetRemoteConfig(t *testing.T) { t.Log("remoteConfigs:", remoteConfigs) t.Log("remoteConfigs size:", len(remoteConfigs)) - Assert(t, "over Max Retry Still Error", Equal(err.Error())) + Assert(t, err.Error(), StartWith(errContained)) } func TestCreateApolloConfigWithJson(t *testing.T) { From 8b7b49c668c92e31dc8999de5deebf34257d6cc0 Mon Sep 17 00:00:00 2001 From: larry4xie Date: Fri, 29 Dec 2023 09:19:02 +0800 Subject: [PATCH 4/4] update test func comment --- component/remote/async_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/component/remote/async_test.go b/component/remote/async_test.go index 439aa1c..c7d645a 100644 --- a/component/remote/async_test.go +++ b/component/remote/async_test.go @@ -177,8 +177,7 @@ func initNotifications() *config.AppConfig { return appConfig } -// Error response -// will hold 5s and keep response 404 +// Error response with status and body func runErrorResponse(status int, body []byte) *httptest.Server { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { w.WriteHeader(status)