Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(notifier): retry mechanism on slack notif #137

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions internal/server/config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ DB:
NOTIFIER:
PROVIDER: slack
ACCESS_TOKEN:
TIMEOUT_IN_SECONDS: 10
MAX_RETRY_COUNT: 3
WORKSPACES:
- WORKSPACE: goto
ACCESS_TOKEN:
Expand Down
76 changes: 76 additions & 0 deletions pkg/http/retry.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
package http

import (
"bytes"
"fmt"
"io"
"math"
"net/http"
"time"
)

const (
DefaultRetryCount = 3
)

type RetryableTransport struct {
Transport http.RoundTripper
RetryCount int
}

func (t *RetryableTransport) RoundTrip(req *http.Request) (*http.Response, error) {
idilhaq marked this conversation as resolved.
Show resolved Hide resolved
var bodyBytes []byte
if req.Body != nil {
bodyBytes, err := io.ReadAll(req.Body)
if err != nil {
return nil, fmt.Errorf("error reading body: %w", err)
}

req.Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
}

retryCount := t.RetryCount
if t.RetryCount <= 0 {
retryCount = DefaultRetryCount
}

resp, err := t.Transport.RoundTrip(req)
retries := -1
for shouldRetry(err, resp) && retries < retryCount {
if retries > -1 {
time.Sleep(backoff(retries))
// consume any response to reuse the connection.
if resp != nil {
drainBody(resp)
}
}

req.Body = io.NopCloser(bytes.NewBuffer(bodyBytes))
resp, err = t.Transport.RoundTrip(req)

retries++
}

return resp, err
}

func backoff(retries int) time.Duration {
return time.Duration(math.Pow(2, float64(retries))) * time.Second
}

func shouldRetry(err error, resp *http.Response) bool {
if err != nil || resp == nil {
return true
}

return resp.StatusCode == http.StatusBadGateway ||
resp.StatusCode == http.StatusServiceUnavailable ||
resp.StatusCode == http.StatusGatewayTimeout
}

func drainBody(resp *http.Response) {
if resp.Body != nil {
io.Copy(io.Discard, resp.Body)
resp.Body.Close()
}
}
83 changes: 83 additions & 0 deletions pkg/http/retry_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
package http

import (
"fmt"
"math"
"net/http"
"net/http/httptest"
"testing"
"time"
)

func TestRetryableTransport_RoundTrip(t *testing.T) {
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
if r.URL.Path == "/success" {
w.WriteHeader(http.StatusOK)
} else {
w.WriteHeader(http.StatusGatewayTimeout)
}
}))
defer server.Close()

transport := &RetryableTransport{
Transport: http.DefaultTransport,
RetryCount: 3,
}

// Test case 1: Successful request
req, err := http.NewRequest(http.MethodGet, server.URL+"/success", nil)
if err != nil {
t.Fatalf("Failed to create request: %v", err)
}
resp, err := transport.RoundTrip(req)
if err != nil {
t.Fatalf("RoundTrip failed: %v", err)
}
if resp.StatusCode != http.StatusOK {
t.Errorf("Unexpected status code: got %d, want %d", resp.StatusCode, http.StatusOK)
}

// Test case 2: Retry exhausted
req, err = http.NewRequest(http.MethodGet, server.URL+"/failure", nil)
if err != nil {
t.Fatalf("Failed to create request: %v", err)
}
resp, err = transport.RoundTrip(req)
if err != nil {
t.Error("Expected nil but got an error")
}
if resp == nil {
t.Error("Expected an error response but got nil")
}
}

func TestShouldRetry(t *testing.T) {
// Test case 1: Retry on connection reset error
err := fmt.Errorf("connection reset by peer")
resp := &http.Response{StatusCode: http.StatusInternalServerError}
if !shouldRetry(err, resp) {
t.Error("shouldRetry returned false, expected true for connection reset error")
}

// Test case 2: Retry on status code 504
resp = &http.Response{StatusCode: http.StatusGatewayTimeout}
if !shouldRetry(nil, resp) {
t.Error("shouldRetry returned false, expected true for status code 504")
}

// Test case 3: Do not retry on status code 200
resp = &http.Response{StatusCode: http.StatusOK}
if shouldRetry(nil, resp) {
t.Error("shouldRetry returned true, expected false for status code 200")
}
}

func TestBackoff(t *testing.T) {
for i := 0; i < 5; i++ {
backoffDuration := backoff(i)
expectedDuration := time.Duration(math.Pow(2, float64(i))) * time.Second
if backoffDuration != expectedDuration {
t.Errorf("backoff(%d) returned %v, expected %v", i, backoffDuration, expectedDuration)
}
}
}
16 changes: 13 additions & 3 deletions plugins/notifiers/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"net/http"
"time"

retryablehttp "github.com/goto/guardian/pkg/http"
"github.com/goto/guardian/pkg/log"
"github.com/mitchellh/mapstructure"

Expand All @@ -33,8 +34,10 @@ type Config struct {
Provider string `mapstructure:"provider" validate:"omitempty,oneof=slack"`

// slack
AccessToken string `mapstructure:"access_token" validate:"required_without=SlackConfig"`
SlackConfig SlackConfig `mapstructure:"slack_config" validate:"required_without=AccessToken,dive"`
AccessToken string `mapstructure:"access_token" validate:"required_without=SlackConfig"`
SlackConfig SlackConfig `mapstructure:"slack_config" validate:"required_without=AccessToken,dive"`
TimeoutInSeconds int `mapstructure:"timeout_in_seconds"`
MaxRetryCount int `mapstructure:"max_retry_count"`

// custom messages
Messages domain.NotificationMessages
Expand All @@ -47,7 +50,14 @@ func NewClient(config *Config, logger log.Logger) (Client, error) {
return nil, err
}

httpClient := &http.Client{Timeout: 10 * time.Second}
retryableTransport := &retryablehttp.RetryableTransport{
Transport: &http.Transport{},
RetryCount: config.MaxRetryCount,
}
httpClient := &http.Client{
Timeout: time.Duration(config.TimeoutInSeconds) * time.Second,
Transport: retryableTransport,
}

return slack.NewNotifier(slackConfig, httpClient, logger), nil
}
Expand Down
Loading