From dd294e8cd669c7b44aefcfe83bbc347dfc6093dc Mon Sep 17 00:00:00 2001 From: Kevin Wan Date: Mon, 2 Jan 2023 13:51:15 +0800 Subject: [PATCH] fix: #2700, timeout not enough for writing responses (#2738) * fix: #2700, timeout not enough for writing responses * fix: test fail * chore: add comments --- rest/engine.go | 7 +++---- rest/engine_test.go | 2 +- rest/server.go | 24 +++++++++++++++++++----- tools/goctl/util/stringx/string_test.go | 8 ++++---- 4 files changed, 27 insertions(+), 14 deletions(-) diff --git a/rest/engine.go b/rest/engine.go index 5209693c..18a2f618 100644 --- a/rest/engine.go +++ b/rest/engine.go @@ -287,10 +287,9 @@ func (ng *engine) withTimeout() internal.StartOption { // without this timeout setting, the server will time out and respond 503 Service Unavailable, // which triggers the circuit breaker. svr.ReadTimeout = 4 * time.Duration(timeout) * time.Millisecond / 5 - // factor 0.9, to avoid clients not reading the response - // without this timeout setting, the server will time out and respond 503 Service Unavailable, - // which triggers the circuit breaker. - svr.WriteTimeout = 9 * time.Duration(timeout) * time.Millisecond / 10 + // factor 1.1, to avoid servers don't have enough time to write responses. + // setting the factor less than 1.0 may lead clients not receiving the responses. + svr.WriteTimeout = 11 * time.Duration(timeout) * time.Millisecond / 10 } } } diff --git a/rest/engine_test.go b/rest/engine_test.go index ef7ab474..f80c434d 100644 --- a/rest/engine_test.go +++ b/rest/engine_test.go @@ -323,7 +323,7 @@ func TestEngine_withTimeout(t *testing.T) { assert.Equal(t, time.Duration(test.timeout)*time.Millisecond*4/5, svr.ReadTimeout) assert.Equal(t, time.Duration(0), svr.ReadHeaderTimeout) - assert.Equal(t, time.Duration(test.timeout)*time.Millisecond*9/10, svr.WriteTimeout) + assert.Equal(t, time.Duration(test.timeout)*time.Millisecond*11/10, svr.WriteTimeout) assert.Equal(t, time.Duration(0), svr.IdleTimeout) }) } diff --git a/rest/server.go b/rest/server.go index f3104344..f54435fd 100644 --- a/rest/server.go +++ b/rest/server.go @@ -90,6 +90,25 @@ func (s *Server) Routes() []Route { return routes } +// ServeHTTP is for test purpose, allow developer to do a unit test with +// all defined router without starting an HTTP Server. +// +// For example: +// +// server := MustNewServer(...) +// server.addRoute(...) // router a +// server.addRoute(...) // router b +// server.addRoute(...) // router c +// +// r, _ := http.NewRequest(...) +// w := httptest.NewRecorder(...) +// server.ServeHTTP(w, r) +// // verify the response +func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { + s.ngin.bindRoutes(s.router) + s.router.ServeHTTP(w, r) +} + // Start starts the Server. // Graceful shutdown is enabled by default. // Use proc.SetTimeToForceQuit to customize the graceful shutdown period. @@ -307,8 +326,3 @@ func newCorsRouter(router httpx.Router, headerFn func(http.Header), origins ...s func (c *corsRouter) ServeHTTP(w http.ResponseWriter, r *http.Request) { c.middleware(c.Router.ServeHTTP)(w, r) } - -func (s *Server) ServeHTTP(w http.ResponseWriter, r *http.Request) { - s.ngin.bindRoutes(s.router) - s.router.ServeHTTP(w, r) -} diff --git a/tools/goctl/util/stringx/string_test.go b/tools/goctl/util/stringx/string_test.go index be51442a..1ed1ee7c 100644 --- a/tools/goctl/util/stringx/string_test.go +++ b/tools/goctl/util/stringx/string_test.go @@ -92,19 +92,19 @@ func TestString_Camel2Snake(t *testing.T) { }{ { input: "goZero", - want: "go_zero", + want: "go_zero", }, { input: "Gozero", - want: "gozero", + want: "gozero", }, { input: "GoZero", - want: "go_zero", + want: "go_zero", }, { input: "Go_Zero", - want: "go__zero", + want: "go__zero", }, } for _, c := range cases {