From a1668b318c0ffb9ddada7f022a7128435b1199f6 Mon Sep 17 00:00:00 2001 From: Jonathan Gramain Date: Thu, 19 Dec 2024 14:18:58 -0800 Subject: [PATCH 1/2] impr: BKTCLT-35 go client: custom HTTP client 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. --- go/bucketclient.go | 18 ++++++++++++ go/bucketclientrequest.go | 2 +- go/bucketclientrequest_test.go | 52 +++++++++++++++++++++++++++++++++- 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/go/bucketclient.go b/go/bucketclient.go index 75f1691..ff76f87 100644 --- a/go/bucketclient.go +++ b/go/bucketclient.go @@ -1,12 +1,30 @@ package bucketclient +import ( + "net/http" +) + type BucketClient struct { + *http.Client Endpoint string 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 { + bc := &BucketClient{ + Client: http.DefaultClient, + Endpoint: bucketdEndpoint, + } + return bc +} + +// 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{ + Client: httpClient, Endpoint: bucketdEndpoint, } } diff --git a/go/bucketclientrequest.go b/go/bucketclientrequest.go index 8d38755..7918c3f 100644 --- a/go/bucketclientrequest.go +++ b/go/bucketclientrequest.go @@ -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.Do(request) } } if err != nil { diff --git a/go/bucketclientrequest_test.go b/go/bucketclientrequest_test.go index 29c4750..c9fe6f0 100644 --- a/go/bucketclientrequest_test.go +++ b/go/bucketclientrequest_test.go @@ -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", @@ -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)) + }) + }) }) From 5486a4c811d805230522abb282a3080fd9b5f023 Mon Sep 17 00:00:00 2001 From: Jonathan Gramain Date: Wed, 15 Jan 2025 11:55:24 -0800 Subject: [PATCH 2/2] BKTCLT-35 bump version to 7.10.16 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 34eb085..df2ac79 100644 --- a/package.json +++ b/package.json @@ -3,7 +3,7 @@ "engines": { "node": ">=16" }, - "version": "7.10.15", + "version": "7.10.16", "description": "Bucket Client", "main": "index.js", "files": [