Skip to content

Commit

Permalink
impr: BKTCLT-35 go client: custom HTTP client
Browse files Browse the repository at this point in the history
Add the possibility to attach a custom HTTP client to a new instance
of BucketClient.

The first intended use is to add a timeout to HTTP requests sent by
BucketClient.
  • Loading branch information
jonathan-gramain committed Jan 15, 2025
1 parent c0be0a4 commit 535c9ce
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 5 deletions.
23 changes: 20 additions & 3 deletions go/bucketclient.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,29 @@
package bucketclient

import (
"net/http"
)

type BucketClient struct {
Endpoint string
Metrics *BucketClientMetrics
Endpoint string
HTTPClient *http.Client
Metrics *BucketClientMetrics
}

// New creates a new BucketClient instance, with the provided endpoint (e.g. "localhost:9000")
// and the default HTTP client.
func New(bucketdEndpoint string) *BucketClient {
return &BucketClient{
Endpoint: bucketdEndpoint,
Endpoint: bucketdEndpoint,
HTTPClient: http.DefaultClient,
}
}

// NewWithHTTPClient creates a new BucketClient instance, with the provided endpoint
// (e.g. "localhost:9000") using the provided http.Client instance.
func NewWithHTTPClient(bucketdEndpoint string, httpClient *http.Client) *BucketClient {
return &BucketClient{
Endpoint: bucketdEndpoint,
HTTPClient: httpClient,
}
}
2 changes: 1 addition & 1 deletion go/bucketclientrequest.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ func (client *BucketClient) Request(ctx context.Context,
if options.idempotent {
request.Header["Idempotency-Key"] = []string{}
}
response, err = http.DefaultClient.Do(request)
response, err = client.HTTPClient.Do(request)
}
}
if err != nil {
Expand Down
52 changes: 51 additions & 1 deletion go/bucketclientrequest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ var _ = Describe("BucketClient.Request()", func() {
Expect(err).To(MatchError(ContainSubstring("no responder found")))
})

Context("with valid URL", Ordered, func() {
Context("with valid URL and default HTTP client", Ordered, func() {
It("succeeds with a 200 response on GET request", func(ctx SpecContext) {
httpmock.RegisterResponder(
"GET", "http://localhost:9000/default/bucket/somebucket/someobject",
Expand Down Expand Up @@ -148,4 +148,54 @@ var _ = Describe("BucketClient.Request()", func() {
Expect(err).To(MatchError(context.DeadlineExceeded))
})
})

Context("with valid URL and custom HTTP client with timeout", Ordered, func() {
var clientWithTimeout *bucketclient.BucketClient

BeforeAll(func() {
clientWithTimeout = bucketclient.NewWithHTTPClient("http://localhost:9000",
&http.Client{
Timeout: 1 * time.Second,
})
})

It("succeeds with a 200 response on GET request", func(ctx SpecContext) {
httpmock.RegisterResponder(
"GET", "http://localhost:9000/default/bucket/somebucket/someobject",
httpmock.NewStringResponder(200, `{"some":"metadata","version":"1234"}`),
)
Expect(clientWithTimeout.Request(ctx, "GetObject", "GET",
"/default/bucket/somebucket/someobject")).To(Equal(
[]byte(`{"some":"metadata","version":"1234"}`)))
})

It("times out after the configured delay without a response", func(ctx SpecContext) {
httpmock.RegisterResponder(
"GET", "http://localhost:9000/default/bucket/somebucket/someobject",
func(req *http.Request) (*http.Response, error) {
// respond after 2 seconds > timeout of one second
time.Sleep(2 * time.Second)
return httpmock.NewStringResponse(200,
`{"some":"metadata","version":"1234"}`), nil
},
)
startTime := time.Now()
_, err := clientWithTimeout.Request(ctx, "GetObject", "GET",
"/default/bucket/somebucket/someobject")
duration := time.Since(startTime)

Expect(err).ToNot(BeNil())
bcErr, isBCErr := err.(*bucketclient.BucketClientError)
Expect(isBCErr).To(BeTrue())
Expect(bcErr.StatusCode).To(Equal(0))
// the error returned by the HTTP layer seems to be inconsistent between
// "Client.Timeout exceeded" and "context deadline exceeded" for some reason,
// I did not dig into it further but it makes it hard to properly test the
// properties about the returned error, so just checking that there is an
// error and that we didn't wait much more than the timeout.
Expect(ctx.Err()).To(BeNil())
Expect(duration).To(BeNumerically(">=", 900*time.Millisecond))
Expect(duration).To(BeNumerically("<=", 1100*time.Millisecond))
})
})
})

0 comments on commit 535c9ce

Please sign in to comment.