From 6714d82c858618d60dabd99aee5e6b399a5823af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erhan=20Akp=C4=B1nar?= Date: Sat, 28 Jan 2023 20:22:42 +0300 Subject: [PATCH 1/6] fix --- middleware/context_timeout_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/middleware/context_timeout_test.go b/middleware/context_timeout_test.go index 238565dfd..53553a12e 100644 --- a/middleware/context_timeout_test.go +++ b/middleware/context_timeout_test.go @@ -173,7 +173,7 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { return nil } - timeout := 100 * time.Millisecond + timeout := 1 * time.Second m := ContextTimeoutWithConfig(ContextTimeoutConfig{ Timeout: timeout, ErrorHandler: timeoutErrorHandler, @@ -189,7 +189,7 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { // NOTE: when difference between timeout duration and handler execution time is almost the same (in range of 100microseconds) // the result of timeout does not seem to be reliable - could respond timeout, could respond handler output // difference over 500microseconds (0.5millisecond) response seems to be reliable - if err := sleepWithContext(c.Request().Context(), time.Duration(500*time.Millisecond)); err != nil { + if err := sleepWithContext(c.Request().Context(), time.Duration(3*time.Second)); err != nil { return err } From 980b400b07473201c61a44aabc615d7bf08babf0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erhan=20Akp=C4=B1nar?= Date: Sat, 28 Jan 2023 20:26:10 +0300 Subject: [PATCH 2/6] remove unnecessary control --- middleware/context_timeout.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/middleware/context_timeout.go b/middleware/context_timeout.go index eafc8a831..be260e188 100644 --- a/middleware/context_timeout.go +++ b/middleware/context_timeout.go @@ -64,9 +64,7 @@ func (config ContextTimeoutConfig) ToMiddleware() (echo.MiddlewareFunc, error) { c.SetRequest(c.Request().WithContext(timeoutContext)) if err := next(c); err != nil { - if config.ErrorHandler != nil { - return config.ErrorHandler(err, c) - } + return config.ErrorHandler(err, c) } return nil } From 10d3e1c51298d0bb9833fb8b393c74c20d79f492 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erhan=20Akp=C4=B1nar?= Date: Sat, 28 Jan 2023 20:34:37 +0300 Subject: [PATCH 3/6] change sleep --- middleware/context_timeout_test.go | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/middleware/context_timeout_test.go b/middleware/context_timeout_test.go index 53553a12e..039b0bef2 100644 --- a/middleware/context_timeout_test.go +++ b/middleware/context_timeout_test.go @@ -189,6 +189,7 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { // NOTE: when difference between timeout duration and handler execution time is almost the same (in range of 100microseconds) // the result of timeout does not seem to be reliable - could respond timeout, could respond handler output // difference over 500microseconds (0.5millisecond) response seems to be reliable + if err := sleepWithContext(c.Request().Context(), time.Duration(3*time.Second)); err != nil { return err } @@ -207,15 +208,20 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { } func sleepWithContext(ctx context.Context, d time.Duration) error { - timer := time.NewTimer(d) + ch := make(chan string) + go func() { + time.Sleep(d) + select { + case ch <- "abc": + default: + return + } + }() select { case <-ctx.Done(): - if !timer.Stop() { - <-timer.C - } - return context.DeadlineExceeded - case <-timer.C: + return ctx.Err() + case <-ch: + return nil } - return nil } From bb156d9b915cf0f40f792c2a1f69bc3dd09b6c7f Mon Sep 17 00:00:00 2001 From: hakankutluay Date: Sat, 28 Jan 2023 20:39:21 +0300 Subject: [PATCH 4/6] change stop execution --- middleware/context_timeout_test.go | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/middleware/context_timeout_test.go b/middleware/context_timeout_test.go index 039b0bef2..a115ac116 100644 --- a/middleware/context_timeout_test.go +++ b/middleware/context_timeout_test.go @@ -208,20 +208,16 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { } func sleepWithContext(ctx context.Context, d time.Duration) error { - ch := make(chan string) - go func() { - time.Sleep(d) - select { - case ch <- "abc": - default: - return - } + timer := time.NewTimer(d) + + defer func() { + _ = timer.Stop() }() select { case <-ctx.Done(): - return ctx.Err() - case <-ch: + return context.DeadlineExceeded + case <-timer.C: return nil } } From 9a57bef9e4aa03478f89d57e102422781d4acadb Mon Sep 17 00:00:00 2001 From: hakankutluay Date: Sat, 28 Jan 2023 20:47:19 +0300 Subject: [PATCH 5/6] increase timeouts --- middleware/context_timeout_test.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/middleware/context_timeout_test.go b/middleware/context_timeout_test.go index a115ac116..75653beff 100644 --- a/middleware/context_timeout_test.go +++ b/middleware/context_timeout_test.go @@ -20,7 +20,7 @@ func TestContextTimeoutSkipper(t *testing.T) { Skipper: func(context echo.Context) bool { return true }, - Timeout: 1 * time.Nanosecond, + Timeout: 10 * time.Millisecond, }) req := httptest.NewRequest(http.MethodGet, "/", nil) @@ -30,7 +30,7 @@ func TestContextTimeoutSkipper(t *testing.T) { c := e.NewContext(req, rec) err := m(func(c echo.Context) error { - time.Sleep(25 * time.Microsecond) + time.Sleep(25 * time.Millisecond) return errors.New("response from handler") })(c) @@ -49,7 +49,7 @@ func TestContextTimeoutErrorOutInHandler(t *testing.T) { t.Parallel() m := ContextTimeoutWithConfig(ContextTimeoutConfig{ // Timeout has to be defined or the whole flow for timeout middleware will be skipped - Timeout: 50 * time.Millisecond, + Timeout: 10 * time.Millisecond, }) req := httptest.NewRequest(http.MethodGet, "/", nil) @@ -76,7 +76,7 @@ func TestContextTimeoutSuccessfulRequest(t *testing.T) { t.Parallel() m := ContextTimeoutWithConfig(ContextTimeoutConfig{ // Timeout has to be defined or the whole flow for timeout middleware will be skipped - Timeout: 50 * time.Millisecond, + Timeout: 10 * time.Millisecond, }) req := httptest.NewRequest(http.MethodGet, "/", nil) @@ -133,7 +133,7 @@ func TestContextTimeoutTestRequestClone(t *testing.T) { func TestContextTimeoutWithDefaultErrorMessage(t *testing.T) { t.Parallel() - timeout := 1 * time.Millisecond + timeout := 10 * time.Millisecond m := ContextTimeoutWithConfig(ContextTimeoutConfig{ Timeout: timeout, }) @@ -145,7 +145,7 @@ func TestContextTimeoutWithDefaultErrorMessage(t *testing.T) { c := e.NewContext(req, rec) err := m(func(c echo.Context) error { - if err := sleepWithContext(c.Request().Context(), time.Duration(2*time.Millisecond)); err != nil { + if err := sleepWithContext(c.Request().Context(), time.Duration(20*time.Millisecond)); err != nil { return err } return c.String(http.StatusOK, "Hello, World!") @@ -173,7 +173,7 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { return nil } - timeout := 1 * time.Second + timeout := 10 * time.Millisecond m := ContextTimeoutWithConfig(ContextTimeoutConfig{ Timeout: timeout, ErrorHandler: timeoutErrorHandler, @@ -190,7 +190,7 @@ func TestContextTimeoutCanHandleContextDeadlineOnNextHandler(t *testing.T) { // the result of timeout does not seem to be reliable - could respond timeout, could respond handler output // difference over 500microseconds (0.5millisecond) response seems to be reliable - if err := sleepWithContext(c.Request().Context(), time.Duration(3*time.Second)); err != nil { + if err := sleepWithContext(c.Request().Context(), time.Duration(20*time.Millisecond)); err != nil { return err } From b2399c94fa45be6995f115d44adc93e4c8134116 Mon Sep 17 00:00:00 2001 From: hakankutluay Date: Sat, 28 Jan 2023 20:59:33 +0300 Subject: [PATCH 6/6] fix test --- middleware/context_timeout_test.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/middleware/context_timeout_test.go b/middleware/context_timeout_test.go index 75653beff..605ca8e65 100644 --- a/middleware/context_timeout_test.go +++ b/middleware/context_timeout_test.go @@ -30,7 +30,10 @@ func TestContextTimeoutSkipper(t *testing.T) { c := e.NewContext(req, rec) err := m(func(c echo.Context) error { - time.Sleep(25 * time.Millisecond) + if err := sleepWithContext(c.Request().Context(), time.Duration(20*time.Millisecond)); err != nil { + return err + } + return errors.New("response from handler") })(c)