From 880491449845e0dc14134d4faef995683ad36aa2 Mon Sep 17 00:00:00 2001 From: Joseph Date: Fri, 28 Feb 2025 12:42:26 +0000 Subject: [PATCH 1/5] Removed executor thing, it had no value because we don't use the mock --- httpclient/client.go | 31 ++---------- httpclient/cookies.go | 4 +- httpclient/http.go | 106 ------------------------------------------ 3 files changed, 7 insertions(+), 134 deletions(-) delete mode 100644 httpclient/http.go diff --git a/httpclient/client.go b/httpclient/client.go index 0806083..d4ff513 100644 --- a/httpclient/client.go +++ b/httpclient/client.go @@ -9,40 +9,19 @@ package httpclient import ( "fmt" - "io" "net/http" "net/http/cookiejar" - "net/url" "time" "github.com/deploymenttheory/go-api-http-client/concurrency" "go.uber.org/zap" ) -// HTTPExecutor is an interface which wraps http.Client to allow mocking. -type HTTPExecutor interface { - - // Inherited - CloseIdleConnections() - Do(req *http.Request) (*http.Response, error) - Get(url string) (resp *http.Response, err error) - Head(url string) (resp *http.Response, err error) - Post(url string, contentType string, body io.Reader) (resp *http.Response, err error) - PostForm(url string, data url.Values) (resp *http.Response, err error) - - // Additional - SetCookieJar(jar http.CookieJar) - SetCookies(url *url.URL, cookies []*http.Cookie) - SetCustomTimeout(time.Duration) - Cookies(*url.URL) []*http.Cookie - SetRedirectPolicy(*func(req *http.Request, via []*http.Request) error) -} - // Master struct/object type Client struct { config *ClientConfig Integration *APIIntegration - http HTTPExecutor + http *http.Client Sugar *zap.SugaredLogger Concurrency *concurrency.ConcurrencyHandler } @@ -100,7 +79,7 @@ type ClientConfig struct { // RetryEligiableRequests when false bypasses any retry logic for a simpler request flow. RetryEligiableRequests bool `json:"retry_eligiable_requests"` - HTTPExecutor HTTPExecutor + HTTPExecutor http.Client } // BuildClient creates a new HTTP client with the provided configuration. @@ -131,10 +110,10 @@ func (c *ClientConfig) Build() (*Client, error) { return nil, err } - httpClient.SetCookieJar(cookieJar) + httpClient.Jar = cookieJar if c.CustomRedirectPolicy != nil { - httpClient.SetRedirectPolicy(c.CustomRedirectPolicy) + httpClient.CheckRedirect = *c.CustomRedirectPolicy } // TODO refactor concurrency @@ -150,7 +129,7 @@ func (c *ClientConfig) Build() (*Client, error) { client := &Client{ Integration: &c.Integration, - http: httpClient, + http: &httpClient, config: c, Sugar: c.Sugar, Concurrency: concurrencyHandler, diff --git a/httpclient/cookies.go b/httpclient/cookies.go index bed2cf9..5137488 100644 --- a/httpclient/cookies.go +++ b/httpclient/cookies.go @@ -12,12 +12,12 @@ func (c *Client) loadCustomCookies() error { return err } - c.http.SetCookies(cookieUrl, c.config.CustomCookies) + c.http.Jar.SetCookies(cookieUrl, c.config.CustomCookies) if c.config.HideSensitiveData { c.Sugar.Debug("[REDACTED] cookies set successfully") } else { - c.Sugar.Debug("custom cookies set: %v", c.http.Cookies(cookieUrl)) + c.Sugar.Debug("custom cookies set: %v", c.http.Jar.Cookies(cookieUrl)) } return nil diff --git a/httpclient/http.go b/httpclient/http.go deleted file mode 100644 index d8b7208..0000000 --- a/httpclient/http.go +++ /dev/null @@ -1,106 +0,0 @@ -package httpclient - -import ( - "bytes" - "fmt" - "io" - "net/http" - "net/url" - "time" -) - -// ProdExecutor wraps http.Client and implements functions to adjust some of it's attrs -type ProdExecutor struct { - *http.Client -} - -// SetCookieJar func to wrap c.Jar = jar -func (c *ProdExecutor) SetCookieJar(jar http.CookieJar) { - c.Jar = jar -} - -// SetCookies wraps http.Client.Jar.SetCookies -func (c *ProdExecutor) SetCookies(url *url.URL, cookies []*http.Cookie) { - c.Jar.SetCookies(url, cookies) -} - -// SetCustomTimeout wraps http.Client.Timeout = timeout -func (c *ProdExecutor) SetCustomTimeout(timeout time.Duration) { - c.Timeout = timeout -} - -// Cookies wraps http.Client.Jar.Cookies() -func (c *ProdExecutor) Cookies(url *url.URL) []*http.Cookie { - return c.Jar.Cookies(url) -} - -// SetRedirectPolicy wraps http.Client.Jar.CheckRedirect = -func (c *ProdExecutor) SetRedirectPolicy(policy *func(req *http.Request, via []*http.Request) error) { - c.CheckRedirect = *policy -} - -// Mocking - -// MockExecutor implements the same function pattern above but allows controllable responses for mocking/testing -type MockExecutor struct { - LockedResponseCode int - ResponseBody string -} - -// CloseIdleConnections does nothing. -func (m *MockExecutor) CloseIdleConnections() { -} - -// Do returns a http.Response with controllable body and status code. -func (m *MockExecutor) Do(req *http.Request) (*http.Response, error) { - statusString := http.StatusText(m.LockedResponseCode) - - if statusString == "" { - return nil, fmt.Errorf("unknown response code requested: %d", m.LockedResponseCode) - } - - response := &http.Response{ - StatusCode: m.LockedResponseCode, - Body: io.NopCloser(bytes.NewBufferString(m.ResponseBody)), - Header: make(http.Header), - } - - return response, nil -} - -// Get returns Do -func (m *MockExecutor) Get(_ string) (*http.Response, error) { - return m.Do(nil) -} - -// Head returns Do -func (m *MockExecutor) Head(_ string) (*http.Response, error) { - return m.Do(nil) -} - -// Post returns Do -func (m *MockExecutor) Post(_ string, _ string, _ io.Reader) (*http.Response, error) { - return m.Do(nil) -} - -// PostForm returns Do -func (m *MockExecutor) PostForm(_ string, _ url.Values) (*http.Response, error) { - return m.Do(nil) -} - -// SetCookieJar does nothing yet -func (m *MockExecutor) SetCookieJar(jar http.CookieJar) {} - -// SetCookies does nothing yet -func (m *MockExecutor) SetCookies(url *url.URL, cookies []*http.Cookie) {} - -// SetCustomTimeout does nothing yet -func (m *MockExecutor) SetCustomTimeout(time.Duration) {} - -// Cookies does nothing yet -func (m *MockExecutor) Cookies(*url.URL) []*http.Cookie { - return nil -} - -// SetRedirectPolicy does nothing -func (m *MockExecutor) SetRedirectPolicy(*func(req *http.Request, via []*http.Request) error) {} From a99a2c87b5de36f73fe6f314a591ea403052ccd0 Mon Sep 17 00:00:00 2001 From: Joseph Date: Fri, 28 Feb 2025 12:55:04 +0000 Subject: [PATCH 2/5] renamed --- httpclient/client.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/httpclient/client.go b/httpclient/client.go index d4ff513..abef88d 100644 --- a/httpclient/client.go +++ b/httpclient/client.go @@ -79,7 +79,7 @@ type ClientConfig struct { // RetryEligiableRequests when false bypasses any retry logic for a simpler request flow. RetryEligiableRequests bool `json:"retry_eligiable_requests"` - HTTPExecutor http.Client + HTTP http.Client } // BuildClient creates a new HTTP client with the provided configuration. @@ -103,7 +103,7 @@ func (c *ClientConfig) Build() (*Client, error) { c.Sugar.Debug("configuration valid") - httpClient := c.HTTPExecutor + httpClient := c.HTTP cookieJar, err := cookiejar.New(nil) if err != nil { From df4a05d30a0cf55243188c448edf066e87a8a34e Mon Sep 17 00:00:00 2001 From: Joseph Date: Fri, 28 Feb 2025 13:18:22 +0000 Subject: [PATCH 3/5] added temporary hardcoded timeout --- httpclient/request.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/httpclient/request.go b/httpclient/request.go index 4b67311..4311bea 100644 --- a/httpclient/request.go +++ b/httpclient/request.go @@ -271,6 +271,11 @@ func (c *Client) request(ctx context.Context, method, endpoint string, body inte startTime := time.Now() req = req.WithContext(ctx) + + // TEMPORARY HARD CODED TIMEOUT + c.http.Timeout = 3 * time.Second + //////////////////////////////// + resp, err := c.http.Do(req) if err != nil { c.Sugar.Error("Failed to send request", zap.String("method", method), zap.String("endpoint", endpoint), zap.Error(err)) From c08b82501526c3c74139407a62dd3bcd97ad82ce Mon Sep 17 00:00:00 2001 From: bobby williams Date: Fri, 28 Feb 2025 19:46:46 +0000 Subject: [PATCH 4/5] change timeout to 5 for testing --- httpclient/request.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/httpclient/request.go b/httpclient/request.go index 4311bea..b710716 100644 --- a/httpclient/request.go +++ b/httpclient/request.go @@ -273,7 +273,7 @@ func (c *Client) request(ctx context.Context, method, endpoint string, body inte req = req.WithContext(ctx) // TEMPORARY HARD CODED TIMEOUT - c.http.Timeout = 3 * time.Second + c.http.Timeout = 5 * time.Second //////////////////////////////// resp, err := c.http.Do(req) From f9d0df9c00e2a73f629a7b59858c177beec95c48 Mon Sep 17 00:00:00 2001 From: Bobby Date: Tue, 4 Mar 2025 13:27:32 +0000 Subject: [PATCH 5/5] Tidied up timeout --- httpclient/client.go | 10 ++++++++++ httpclient/request.go | 4 ---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/httpclient/client.go b/httpclient/client.go index abef88d..281052c 100644 --- a/httpclient/client.go +++ b/httpclient/client.go @@ -17,6 +17,8 @@ import ( "go.uber.org/zap" ) +const DefaultTimeout time.Duration = 5 * time.Second + // Master struct/object type Client struct { config *ClientConfig @@ -105,6 +107,12 @@ func (c *ClientConfig) Build() (*Client, error) { httpClient := c.HTTP + if c.CustomTimeout == 0 { + c.CustomTimeout = DefaultTimeout + } + + httpClient.Timeout = c.CustomTimeout + cookieJar, err := cookiejar.New(nil) if err != nil { return nil, err @@ -127,6 +135,8 @@ func (c *ClientConfig) Build() (*Client, error) { ) } + + client := &Client{ Integration: &c.Integration, http: &httpClient, diff --git a/httpclient/request.go b/httpclient/request.go index b710716..3ac3ced 100644 --- a/httpclient/request.go +++ b/httpclient/request.go @@ -272,10 +272,6 @@ func (c *Client) request(ctx context.Context, method, endpoint string, body inte req = req.WithContext(ctx) - // TEMPORARY HARD CODED TIMEOUT - c.http.Timeout = 5 * time.Second - //////////////////////////////// - resp, err := c.http.Do(req) if err != nil { c.Sugar.Error("Failed to send request", zap.String("method", method), zap.String("endpoint", endpoint), zap.Error(err))