From b51be1dac45d3afa221c0fc8b4798bc801777b80 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 14 Jun 2021 10:38:35 +0200 Subject: [PATCH 1/3] Implement Delivery.Timeout in HTTPMessageSender Signed-off-by: Francesco Guardiani --- pkg/kncloudevents/message_sender.go | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/kncloudevents/message_sender.go b/pkg/kncloudevents/message_sender.go index 8ff4c6c7067..6d223f7da40 100644 --- a/pkg/kncloudevents/message_sender.go +++ b/pkg/kncloudevents/message_sender.go @@ -57,8 +57,18 @@ func (s *HTTPMessageSender) SendWithRetries(req *nethttp.Request, config *RetryC return s.Send(req) } + client := s.Client + if config.RequestTimeout != 0 { + client = &nethttp.Client{ + Transport: client.Transport, + CheckRedirect: client.CheckRedirect, + Jar: client.Jar, + Timeout: config.RequestTimeout, + } + } + retryableClient := retryablehttp.Client{ - HTTPClient: s.Client, + HTTPClient: client, RetryWaitMin: defaultRetryWaitMin, RetryWaitMax: defaultRetryWaitMax, RetryMax: config.RetryMax, From 3126ebe9dd668a959648789b060b2f723e61c376 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 14 Jun 2021 12:19:01 +0200 Subject: [PATCH 2/3] Test Signed-off-by: Francesco Guardiani --- pkg/kncloudevents/message_sender_test.go | 41 ++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/pkg/kncloudevents/message_sender_test.go b/pkg/kncloudevents/message_sender_test.go index 601eb81199e..1527a29288b 100644 --- a/pkg/kncloudevents/message_sender_test.go +++ b/pkg/kncloudevents/message_sender_test.go @@ -154,6 +154,47 @@ func TestHTTPMessageSenderSendWithRetriesWithBufferedMessage(t *testing.T) { } } +func TestHTTPMessageSenderSendWithRetriesWithSingleRequestTimeout(t *testing.T) { + t.Parallel() + + timeout := time.Second * 3 + + var n int32 + server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { + newVal := atomic.AddInt32(&n, 1) + if newVal == 5 { + writer.WriteHeader(http.StatusOK) + } else { + // Let's add one more second + time.Sleep(timeout + time.Second) + writer.WriteHeader(http.StatusAccepted) + } + })) + + sender := &HTTPMessageSender{ + Client: getClient(), + } + config := &RetryConfig{ + RetryMax: 5, + CheckRetry: func(ctx context.Context, resp *http.Response, err error) (bool, error) { + return true, nil + }, + Backoff: func(attemptNum int, resp *http.Response) time.Duration { + return time.Millisecond + }, + RequestTimeout: timeout, + } + + request, err := http.NewRequest("POST", server.URL, nil) + require.NoError(t, err) + + got, err := sender.SendWithRetries(request, config) + require.NoError(t, err) + + require.Equal(t, http.StatusOK, got.StatusCode) + require.Equal(t, 5, int(atomic.LoadInt32(&n))) +} + func TestRetriesOnNetworkErrors(t *testing.T) { n := int32(10) From 407cd6163bebd9071e019b42f65d4966bd0c3d58 Mon Sep 17 00:00:00 2001 From: slinkydeveloper Date: Mon, 14 Jun 2021 12:39:13 +0200 Subject: [PATCH 3/3] Test is passing Signed-off-by: Francesco Guardiani --- pkg/kncloudevents/message_sender_test.go | 15 +++++++-------- pkg/kncloudevents/retries.go | 3 +++ 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/pkg/kncloudevents/message_sender_test.go b/pkg/kncloudevents/message_sender_test.go index 1527a29288b..d7275933ed7 100644 --- a/pkg/kncloudevents/message_sender_test.go +++ b/pkg/kncloudevents/message_sender_test.go @@ -162,23 +162,22 @@ func TestHTTPMessageSenderSendWithRetriesWithSingleRequestTimeout(t *testing.T) var n int32 server := httptest.NewServer(http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) { newVal := atomic.AddInt32(&n, 1) - if newVal == 5 { + if newVal >= 5 { writer.WriteHeader(http.StatusOK) } else { // Let's add one more second - time.Sleep(timeout + time.Second) + time.Sleep(timeout) writer.WriteHeader(http.StatusAccepted) } })) + defer server.Close() sender := &HTTPMessageSender{ Client: getClient(), } config := &RetryConfig{ - RetryMax: 5, - CheckRetry: func(ctx context.Context, resp *http.Response, err error) (bool, error) { - return true, nil - }, + RetryMax: 5, + CheckRetry: RetryIfGreaterThan300, Backoff: func(attemptNum int, resp *http.Response) time.Duration { return time.Millisecond }, @@ -189,10 +188,10 @@ func TestHTTPMessageSenderSendWithRetriesWithSingleRequestTimeout(t *testing.T) require.NoError(t, err) got, err := sender.SendWithRetries(request, config) - require.NoError(t, err) - require.Equal(t, http.StatusOK, got.StatusCode) require.Equal(t, 5, int(atomic.LoadInt32(&n))) + require.NoError(t, err) + require.Equal(t, http.StatusOK, got.StatusCode) } func TestRetriesOnNetworkErrors(t *testing.T) { diff --git a/pkg/kncloudevents/retries.go b/pkg/kncloudevents/retries.go index b43e93e0885..416049b46d7 100644 --- a/pkg/kncloudevents/retries.go +++ b/pkg/kncloudevents/retries.go @@ -63,6 +63,9 @@ type RetryConfig struct { CheckRetry CheckRetry Backoff Backoff + + // RequestTimeout represents the timeout of the single request + RequestTimeout time.Duration } func NoRetries() RetryConfig {