Repository: open-telemetry/opamp-go
Pull request #135 (2022-09-23 18:35:33)

Add Go Docs and Build Status badges to README.md

codecov[bot]: # [Codecov](https://codecov.io/gh/open-telemetry/opamp-go/pull/135?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) Report
Base: **74.80**% // Head: **74.74**% // Decreases project coverage by **`-0.05%`** :warning:
> Coverage data is based on head [(`a780ae3`)](https://codecov.io/gh/open-telemetry/opamp-go/pull/135?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) compared to base [(`83a6659`)](https://codecov.io/gh/open-telemetry/opamp-go/commit/83a66593d008cf212944b9198aaac117d76b36f8?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry).
> Patch has no changes to coverable lines.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff             @@
##             main     #135      +/-   ##
==========================================
- Coverage   74.80%   74.74%   -0.06%     
==========================================
  Files          23       23              
  Lines        1770     1770              
==========================================
- Hits         1324     1323       -1     
  Misses        333      333              
- Partials      113      114       +1     
```


| [Impacted Files](https://codecov.io/gh/open-telemetry/opamp-go/pull/135?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry) | Coverage Δ | |
|---|---|---|
| [client/wsclient.go](https://codecov.io/gh/open-telemetry/opamp-go/pull/135/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-Y2xpZW50L3dzY2xpZW50Lmdv) | `84.75% <0.00%> (-2.44%)` | :arrow_down: |
| [client/internal/tcpproxy.go](https://codecov.io/gh/open-telemetry/opamp-go/pull/135/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry#diff-Y2xpZW50L2ludGVybmFsL3RjcHByb3h5Lmdv) | `80.95% <0.00%> (+4.76%)` | :arrow_up: |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/open-telemetry/opamp-go/pull/135?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=open-telemetry).


Patch:
diff --git a/README.md b/README.md
index 3d75c91..57d6dd0 100644
--- a/README.md
+++ b/README.md
@@ -9,6 +9,10 @@
 </p>
 
 <p align="center">
+  <a href="https://pkg.go.dev/github.com/open-telemetry/opamp-go">
+    <img alt="Go Docs" height="28" src="https://godoc.org/github.com/open-telemetry/opamp-go?status.svg"></a>
+  <a href="https://github.com/open-telemetry/opamp-go/actions/workflows/build-and-test.yml?query=branch%3Amain">
+    <img alt="Build Status" src="https://img.shields.io/github/workflow/status/open-telemetry/opamp-go/build-and-test/main?style=for-the-badge"></a>
   <a href="https://goreportcard.com/report/github.com/open-telemetry/opamp-go">
     <img alt="Go Report Card" src="https://goreportcard.com/badge/github.com/open-telemetry/opamp-go?style=for-the-badge"></a>
   <a href="https://codecov.io/gh/open-telemetry/opamp-go/branch/main/">
diff --git a/internal/retryafter.go b/internal/retryafter.go
index 70eb831..784473b 100644
--- a/internal/retryafter.go
+++ b/internal/retryafter.go
@@ -1,31 +1,61 @@
 package internal
 
 import (
+	"errors"
 	"net/http"
 	"strconv"
-	"strings"
 	"time"
 )
 
 const retryAfterHTTPHeader = "Retry-After"
 
+var errCouldNotParseRetryAfterHeader = errors.New("could not parse" + retryAfterHTTPHeader + "header")
+
 type OptionalDuration struct {
 	Duration time.Duration
 	// true if duration field is defined.
 	Defined bool
 }
 
+func parseDelaySeconds(s string) (time.Duration, error) {
+	n, err := strconv.Atoi(s)
+
+	// Verify duration parsed properly and bigger than 0
+	if err == nil && n > 0 {
+		duration := time.Duration(n) * time.Second
+		return duration, nil
+	}
+	return 0, errCouldNotParseRetryAfterHeader
+}
+
+func parseHTTPDate(s string) (time.Duration, error) {
+	t, err := http.ParseTime(s)
+
+	// Verify duration parsed properly and bigger than 0
+	if err == nil {
+		if duration := time.Until(t); duration > 0 {
+			return duration, nil
+		}
+	}
+	return 0, errCouldNotParseRetryAfterHeader
+}
+
 // ExtractRetryAfterHeader extracts Retry-After response header if the status
 // is 503 or 429. Returns 0 duration if the header is not found or the status
 // is different.
 func ExtractRetryAfterHeader(resp *http.Response) OptionalDuration {
 	if resp.StatusCode == http.StatusServiceUnavailable ||
 		resp.StatusCode == http.StatusTooManyRequests {
-		retryAfter := strings.TrimSpace(resp.Header.Get(retryAfterHTTPHeader))
+		retryAfter := resp.Header.Get(retryAfterHTTPHeader)
 		if retryAfter != "" {
-			retryIntervalSec, err := strconv.Atoi(retryAfter)
+			// Parse delay-seconds https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3
+			retryInterval, err := parseDelaySeconds(retryAfter)
+			if err == nil {
+				return OptionalDuration{Defined: true, Duration: retryInterval}
+			}
+			// Parse HTTP-date https://datatracker.ietf.org/doc/html/rfc7231#section-7.1.3
+			retryInterval, err = parseHTTPDate(retryAfter)
 			if err == nil {
-				retryInterval := time.Duration(retryIntervalSec) * time.Second
 				return OptionalDuration{Defined: true, Duration: retryInterval}
 			}
 		}


Test patch:
diff --git a/internal/retryafter_test.go b/internal/retryafter_test.go
new file mode 100644
index 0000000..20bc199
--- /dev/null
+++ b/internal/retryafter_test.go
@@ -0,0 +1,88 @@
+package internal
+
+import (
+	"github.com/stretchr/testify/assert"
+	"math/rand"
+	"net/http"
+	"strconv"
+	"testing"
+	"time"
+)
+
+func response503() *http.Response {
+	return &http.Response{
+		StatusCode: http.StatusServiceUnavailable,
+		Header:     map[string][]string{},
+	}
+}
+
+func assertUndefinedDuration(t *testing.T, d OptionalDuration) {
+	assert.NotNil(t, d)
+	assert.Equal(t, false, d.Defined)
+	assert.Equal(t, time.Duration(0), d.Duration)
+}
+
+func assertDuration(t *testing.T, duration OptionalDuration, expected time.Duration) {
+	assert.NotNil(t, duration)
+	assert.Equal(t, true, duration.Defined)
+
+	// LessOrEqual to consider the time passes during the tests (actual duration would decrease in HTTP-date tests)
+	assert.LessOrEqual(t, duration.Duration, expected)
+}
+
+func TestExtractRetryAfterHeaderDelaySeconds(t *testing.T) {
+	// Generate random n > 0 int
+	rand.Seed(time.Now().UnixNano())
+	retryIntervalSec := rand.Intn(9999)
+
+	// Generate a 503 status code response with Retry-After = n header
+	resp := response503()
+	resp.Header.Add(retryAfterHTTPHeader, strconv.Itoa(retryIntervalSec))
+
+	expectedDuration := time.Second * time.Duration(retryIntervalSec)
+	assertDuration(t, ExtractRetryAfterHeader(resp), expectedDuration)
+
+	// Verify status code 429
+	resp.StatusCode = http.StatusTooManyRequests
+	assertDuration(t, ExtractRetryAfterHeader(resp), expectedDuration)
+
+	// Verify different status code than {429, 503}
+	resp.StatusCode = http.StatusBadGateway
+	assertUndefinedDuration(t, ExtractRetryAfterHeader(resp))
+
+	// Verify no duration is created for n < 0
+	resp.Header.Set(retryAfterHTTPHeader, strconv.Itoa(-1))
+	assertUndefinedDuration(t, ExtractRetryAfterHeader(resp))
+}
+
+func TestExtractRetryAfterHeaderHttpDate(t *testing.T) {
+	// Generate a random n > 0 second duration
+	now := time.Now()
+	rand.Seed(now.UnixNano())
+	retryIntervalSec := rand.Intn(9999)
+	expectedDuration := time.Second * time.Duration(retryIntervalSec)
+
+	// Set a response with Retry-After header = random n > 0 int
+	resp := response503()
+	retryAfter := now.Add(time.Second * time.Duration(retryIntervalSec)).UTC()
+
+	// Verify HTTP-date TimeFormat format is being parsed correctly
+	resp.Header.Set(retryAfterHTTPHeader, retryAfter.Format(http.TimeFormat))
+	assertDuration(t, ExtractRetryAfterHeader(resp), expectedDuration)
+
+	// Verify ANSI time format
+	resp.Header.Set(retryAfterHTTPHeader, retryAfter.Format(time.ANSIC))
+	assertDuration(t, ExtractRetryAfterHeader(resp), expectedDuration)
+
+	// Verify RFC850 time format
+	resp.Header.Set(retryAfterHTTPHeader, retryAfter.Format(time.RFC850))
+	assertDuration(t, ExtractRetryAfterHeader(resp), expectedDuration)
+
+	// Verify non HTTP-date RFC1123 format isn't being parsed
+	resp.Header.Set(retryAfterHTTPHeader, retryAfter.Format(time.RFC1123))
+	assertUndefinedDuration(t, ExtractRetryAfterHeader(resp))
+
+	// Verify no duration is created for n < 0
+	resp.Header.Set(retryAfterHTTPHeader, now.Add(-1*time.Second).UTC().Format(http.TimeFormat))
+	assertUndefinedDuration(t, ExtractRetryAfterHeader(resp))
+}