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

Implement retry on HTTP error for broken connection #18

Merged
merged 1 commit into from
Aug 11, 2024
Merged
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
3 changes: 2 additions & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,8 @@ stop:

mock-gen:
go install go.uber.org/mock/mockgen@latest
mockgen -source=http/client.go -package=mocks -destination=mocks/http_client_mock.go -mock_names=Client=HttpClientMock
mockgen -source=http/client.go -package=httpmock -destination=mocks/http/client_mock.go -mock_names=Client=ClientMock
mockgen -source=http/retryhttp/client.go -package=mocks -destination=mocks/http_retry_client_mock.go -mock_names=Client=HttpRetryClientMock
mockgen -source=captcha/provider.go -package=mocks -destination=mocks/captcha_provider_mock.go -mock_names=Provider=CaptchaProviderMock
mockgen -source=service/api/service.go -package=mocks -destination=mocks/api_service_mock.go -mock_names=Service=ApiServiceMock
mockgen -source=router/api/handler.go -package=mocks -destination=mocks/api_handler_mock.go -mock_names=Handler=ApiHandlerMock
Expand Down
8 changes: 5 additions & 3 deletions captcha/google_recaptcha.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,18 @@ import (

"github.com/cash-track/gateway/config"
"github.com/cash-track/gateway/headers"
"github.com/cash-track/gateway/http"
"github.com/cash-track/gateway/http/retryhttp"
)

const (
googleApiReCaptchaVerifyUrl = "https://www.google.com/recaptcha/api/siteverify"
googleApiReadTimeout = 500 * time.Millisecond
googleApiWriteTimeout = time.Second
googleApiRetryAttempts = uint(2)
)

type GoogleReCaptchaProvider struct {
client http.Client
client retryhttp.Client
secret string
}

Expand All @@ -34,9 +35,10 @@ type googleReCaptchaVerifyResponse struct {
ErrorCodes []string `json:"error-codes,omitempty"`
}

func NewGoogleReCaptchaProvider(httpClient http.Client, options config.Config) *GoogleReCaptchaProvider {
func NewGoogleReCaptchaProvider(httpClient retryhttp.Client, options config.Config) *GoogleReCaptchaProvider {
httpClient.WithReadTimeout(googleApiReadTimeout)
httpClient.WithWriteTimeout(googleApiWriteTimeout)
httpClient.WithRetryAttempts(googleApiRetryAttempts)

return &GoogleReCaptchaProvider{
client: httpClient,
Expand Down
21 changes: 14 additions & 7 deletions captcha/google_recaptcha_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,15 @@ import (

func TestVerify(t *testing.T) {
ctrl := gomock.NewController(t)
c := mocks.NewHttpClientMock(ctrl)
c := mocks.NewHttpRetryClientMock(ctrl)

ctx := fasthttp.RequestCtx{}
ctx.SetRemoteAddr(&net.TCPAddr{IP: []byte{0xA, 0x0, 0x0, 0x1}})
ctx.Request.Header.Set(headers.XCtCaptchaChallenge, "captcha_challenge_2")

c.EXPECT().WithReadTimeout(gomock.Eq(googleApiReadTimeout))
c.EXPECT().WithWriteTimeout(gomock.Eq(googleApiWriteTimeout))
c.EXPECT().WithRetryAttempts(gomock.Eq(googleApiRetryAttempts))
c.EXPECT().Do(gomock.Any(), gomock.Any()).DoAndReturn(func(req *fasthttp.Request, resp *fasthttp.Response) error {
resp.SetStatusCode(fasthttp.StatusOK)
resp.SetBodyString(`{"success":true,"score":0.99,"error-codes":["no-error"]}`)
Expand All @@ -50,14 +51,15 @@ func TestVerify(t *testing.T) {

func TestVerifyUnsuccessful(t *testing.T) {
ctrl := gomock.NewController(t)
c := mocks.NewHttpClientMock(ctrl)
c := mocks.NewHttpRetryClientMock(ctrl)

ctx := fasthttp.RequestCtx{}
ctx.SetRemoteAddr(&net.TCPAddr{IP: []byte{0xA, 0x0, 0x0, 0x1}})
ctx.Request.Header.Set(headers.XCtCaptchaChallenge, "captcha_challenge_2")

c.EXPECT().WithReadTimeout(gomock.Eq(googleApiReadTimeout))
c.EXPECT().WithWriteTimeout(gomock.Eq(googleApiWriteTimeout))
c.EXPECT().WithRetryAttempts(gomock.Eq(googleApiRetryAttempts))
c.EXPECT().Do(gomock.Any(), gomock.Any()).DoAndReturn(func(req *fasthttp.Request, resp *fasthttp.Response) error {
resp.SetStatusCode(fasthttp.StatusOK)
resp.SetBodyString(`{"success":false,"score":0.99,"error-codes":["bad-input"]}`)
Expand All @@ -84,14 +86,15 @@ func TestVerifyUnsuccessful(t *testing.T) {

func TestVerifyEmptySecret(t *testing.T) {
ctrl := gomock.NewController(t)
c := mocks.NewHttpClientMock(ctrl)
c := mocks.NewHttpRetryClientMock(ctrl)

ctx := fasthttp.RequestCtx{}
ctx.SetRemoteAddr(&net.TCPAddr{IP: []byte{0xA, 0x0, 0x0, 0x1}})
ctx.Request.Header.Set(headers.XCtCaptchaChallenge, "captcha_challenge_2")

c.EXPECT().WithReadTimeout(gomock.Eq(googleApiReadTimeout))
c.EXPECT().WithWriteTimeout(gomock.Eq(googleApiWriteTimeout))
c.EXPECT().WithRetryAttempts(gomock.Eq(googleApiRetryAttempts))

p := NewGoogleReCaptchaProvider(c, config.Config{
CaptchaSecret: "",
Expand All @@ -104,7 +107,7 @@ func TestVerifyEmptySecret(t *testing.T) {

func TestVerifyOptions(t *testing.T) {
ctrl := gomock.NewController(t)
c := mocks.NewHttpClientMock(ctrl)
c := mocks.NewHttpRetryClientMock(ctrl)

ctx := fasthttp.RequestCtx{}
ctx.SetRemoteAddr(&net.TCPAddr{IP: []byte{0xA, 0x0, 0x0, 0x1}})
Expand All @@ -113,6 +116,7 @@ func TestVerifyOptions(t *testing.T) {

c.EXPECT().WithReadTimeout(gomock.Eq(googleApiReadTimeout))
c.EXPECT().WithWriteTimeout(gomock.Eq(googleApiWriteTimeout))
c.EXPECT().WithRetryAttempts(gomock.Eq(googleApiRetryAttempts))

p := NewGoogleReCaptchaProvider(c, config.Config{
CaptchaSecret: "captcha_secret_1",
Expand All @@ -125,14 +129,15 @@ func TestVerifyOptions(t *testing.T) {

func TestVerifyEmptyChallenge(t *testing.T) {
ctrl := gomock.NewController(t)
c := mocks.NewHttpClientMock(ctrl)
c := mocks.NewHttpRetryClientMock(ctrl)

ctx := fasthttp.RequestCtx{}
ctx.SetRemoteAddr(&net.TCPAddr{IP: []byte{0xA, 0x0, 0x0, 0x1}})
ctx.Request.Header.Set(headers.XCtCaptchaChallenge, "")

c.EXPECT().WithReadTimeout(gomock.Eq(googleApiReadTimeout))
c.EXPECT().WithWriteTimeout(gomock.Eq(googleApiWriteTimeout))
c.EXPECT().WithRetryAttempts(gomock.Eq(googleApiRetryAttempts))

p := NewGoogleReCaptchaProvider(c, config.Config{
CaptchaSecret: "captcha_secret_1",
Expand All @@ -145,14 +150,15 @@ func TestVerifyEmptyChallenge(t *testing.T) {

func TestVerifyRequestFail(t *testing.T) {
ctrl := gomock.NewController(t)
c := mocks.NewHttpClientMock(ctrl)
c := mocks.NewHttpRetryClientMock(ctrl)

ctx := fasthttp.RequestCtx{}
ctx.SetRemoteAddr(&net.TCPAddr{IP: []byte{0xA, 0x0, 0x0, 0x1}})
ctx.Request.Header.Set(headers.XCtCaptchaChallenge, "captcha_challenge_2")

c.EXPECT().WithReadTimeout(gomock.Eq(googleApiReadTimeout))
c.EXPECT().WithWriteTimeout(gomock.Eq(googleApiWriteTimeout))
c.EXPECT().WithRetryAttempts(gomock.Eq(googleApiRetryAttempts))
c.EXPECT().Do(gomock.Any(), gomock.Any()).Return(fmt.Errorf("broken pipe"))

p := NewGoogleReCaptchaProvider(c, config.Config{
Expand All @@ -166,14 +172,15 @@ func TestVerifyRequestFail(t *testing.T) {

func TestVerifyBadResponse(t *testing.T) {
ctrl := gomock.NewController(t)
c := mocks.NewHttpClientMock(ctrl)
c := mocks.NewHttpRetryClientMock(ctrl)

ctx := fasthttp.RequestCtx{}
ctx.SetRemoteAddr(&net.TCPAddr{IP: []byte{0xA, 0x0, 0x0, 0x1}})
ctx.Request.Header.Set(headers.XCtCaptchaChallenge, "captcha_challenge_2")

c.EXPECT().WithReadTimeout(gomock.Eq(googleApiReadTimeout))
c.EXPECT().WithWriteTimeout(gomock.Eq(googleApiWriteTimeout))
c.EXPECT().WithRetryAttempts(gomock.Eq(googleApiRetryAttempts))
c.EXPECT().Do(gomock.Any(), gomock.Any()).DoAndReturn(func(req *fasthttp.Request, resp *fasthttp.Response) error {
resp.SetStatusCode(fasthttp.StatusOK)
resp.SetBodyString(`{"success":true`)
Expand Down
50 changes: 50 additions & 0 deletions http/retryhttp/client.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package retryhttp

import (
"log"
"strings"

"github.com/cash-track/gateway/http"
"github.com/valyala/fasthttp"
)

const defaultRetryAttempts = 1

type Client interface {
http.Client
WithRetryAttempts(attempts uint) Client
DoWithRetry(req *fasthttp.Request, resp *fasthttp.Response, attempts uint) error
}

type FastHttpRetryClient struct {
http.Client
attempts uint
}

func NewFastHttpRetryClient() Client {
return &FastHttpRetryClient{
Client: http.NewFastHttpClient(),
attempts: defaultRetryAttempts,
}
}

func (c *FastHttpRetryClient) Do(req *fasthttp.Request, resp *fasthttp.Response) error {
return c.DoWithRetry(req, resp, c.attempts)
}

func (c *FastHttpRetryClient) DoWithRetry(req *fasthttp.Request, resp *fasthttp.Response, attempts uint) error {
err := c.Client.Do(req, resp)

if attempts == 1 || err == nil || !strings.Contains(err.Error(), "broken pipe") {
return err
}

log.Printf("retrying request due to an error [attempt %d] : %s", attempts, err.Error())

return c.DoWithRetry(req, resp, attempts-1)
}

func (c *FastHttpRetryClient) WithRetryAttempts(attempts uint) Client {
c.attempts = attempts
return c
}
42 changes: 42 additions & 0 deletions http/retryhttp/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
package retryhttp

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
"github.com/valyala/fasthttp"
"go.uber.org/mock/gomock"

"github.com/cash-track/gateway/http"
httpmock "github.com/cash-track/gateway/mocks/http"
)

func TestNewFastHttpRetryClient(t *testing.T) {
client := NewFastHttpRetryClient()
assert.NotNil(t, client)
}

func TestDoWithRetry(t *testing.T) {
ctrl := gomock.NewController(t)
c := httpmock.NewClientMock(ctrl)
c.EXPECT().Do(gomock.Any(), gomock.Any()).Times(2).Return(fmt.Errorf("unknown error: broken pipe or closed connection"))

client := FastHttpRetryClient{
Client: c,
}

client.WithRetryAttempts(2)
err := client.Do(&fasthttp.Request{}, &fasthttp.Response{})

assert.Error(t, err)
}

func TestWithRetryAttempts(t *testing.T) {
client := FastHttpRetryClient{
Client: &http.FastHttpClient{},
}
client.WithRetryAttempts(3)

assert.Equal(t, uint(3), client.attempts)
}
6 changes: 3 additions & 3 deletions main.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import (
"github.com/cash-track/gateway/captcha"
"github.com/cash-track/gateway/config"
"github.com/cash-track/gateway/headers"
"github.com/cash-track/gateway/http"
"github.com/cash-track/gateway/http/retryhttp"
"github.com/cash-track/gateway/logger"
"github.com/cash-track/gateway/router"
apiHandler "github.com/cash-track/gateway/router/api"
Expand All @@ -27,8 +27,8 @@ func main() {
r := router.New(
apiHandler.NewHttp(
config.Global,
apiService.NewHttp(http.NewFastHttpClient(), config.Global),
captcha.NewGoogleReCaptchaProvider(http.NewFastHttpClient(), config.Global),
apiService.NewHttp(retryhttp.NewFastHttpRetryClient(), config.Global),
captcha.NewGoogleReCaptchaProvider(retryhttp.NewFastHttpRetryClient(), config.Global),
),
)
h := prom.NewPrometheus("http").WrapHandler(r.Router)
Expand Down
84 changes: 84 additions & 0 deletions mocks/http/client_mock.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading