Skip to content

Commit 68f0e34

Browse files
committed
fix(ofrep): close http response body after using it
This commit fixes the memory leak when using http client. It also cleanups few tests and typos. Signed-off-by: Roman Dmytrenko <[email protected]>
1 parent c4a1c78 commit 68f0e34

File tree

4 files changed

+23
-63
lines changed

4 files changed

+23
-63
lines changed

providers/ofrep/internal/evaluate/flags_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@ func TestBooleanEvaluation(t *testing.T) {
126126
expect: false,
127127
},
128128
{
129-
name: "disbaled flag",
129+
name: "disabled flag",
130130
resolver: mockResolver{
131131
success: &successDisabled,
132132
},
@@ -192,7 +192,7 @@ func TestIntegerEvaluation(t *testing.T) {
192192
expect: 1,
193193
},
194194
{
195-
name: "disbaled flag",
195+
name: "disabled flag",
196196
resolver: mockResolver{
197197
success: &successDisabled,
198198
},
@@ -250,7 +250,7 @@ func TestFloatEvaluation(t *testing.T) {
250250
expect: 1.05,
251251
},
252252
{
253-
name: "disbaled flag",
253+
name: "disabled flag",
254254
resolver: mockResolver{
255255
success: &successDisabled,
256256
},
@@ -341,7 +341,7 @@ func TestObjectEvaluation(t *testing.T) {
341341
expect: map[string]interface{}{},
342342
},
343343
{
344-
name: "disbaled flag",
344+
name: "disabled flag",
345345
resolver: mockResolver{
346346
success: &successDisabled,
347347
},

providers/ofrep/internal/outbound/http.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ func (h *Outbound) Single(ctx context.Context, key string, payload []byte) (*Res
7070
if err != nil {
7171
return nil, err
7272
}
73+
defer rsp.Body.Close()
7374

7475
b, err := io.ReadAll(rsp.Body)
7576
if err != nil {

providers/ofrep/internal/outbound/http_test.go

Lines changed: 4 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -2,43 +2,25 @@ package outbound
22

33
import (
44
"context"
5-
"errors"
65
"fmt"
76
"net/http"
7+
"net/http/httptest"
88
"testing"
9-
"time"
109
)
1110

1211
func TestHttpOutbound(t *testing.T) {
1312
// given
14-
host := "localhost:18181"
1513
key := "flag"
16-
17-
server := http.Server{
18-
Addr: host,
19-
Handler: mockHandler{
20-
t: t,
21-
key: key,
22-
},
23-
}
24-
25-
go func() {
26-
err := server.ListenAndServe()
27-
if err != nil && !errors.Is(err, http.ErrServerClosed) {
28-
t.Logf("error starting mock server: %v", err)
29-
return
30-
}
31-
}()
32-
33-
<-time.After(3 * time.Second)
14+
server := httptest.NewServer(mockHandler{t: t, key: key})
15+
t.Cleanup(server.Close)
3416

3517
outbound := NewHttp(Configuration{
3618
Callbacks: []HeaderCallback{
3719
func() (string, string) {
3820
return "Authorization", "Token"
3921
},
4022
},
41-
BaseURI: fmt.Sprintf("http://%s", host),
23+
BaseURI: server.URL,
4224
})
4325

4426
// when
@@ -52,12 +34,6 @@ func TestHttpOutbound(t *testing.T) {
5234
if response.Status != http.StatusOK {
5335
t.Errorf("expected 200, but got %d", response.Status)
5436
}
55-
56-
// cleanup
57-
err = server.Shutdown(context.Background())
58-
if err != nil {
59-
t.Errorf("error shuttting down mock server: %v", err)
60-
}
6137
}
6238

6339
type mockHandler struct {

providers/ofrep/provider_test.go

Lines changed: 14 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,13 @@ package ofrep
22

33
import (
44
"context"
5-
"fmt"
6-
"github.com/open-feature/go-sdk/openfeature"
75
"net/http"
6+
"net/http/httptest"
87
"testing"
98
"time"
109

10+
"github.com/open-feature/go-sdk/openfeature"
11+
1112
"github.com/open-feature/go-sdk-contrib/providers/ofrep/internal/outbound"
1213
)
1314

@@ -22,11 +23,11 @@ func TestConfigurations(t *testing.T) {
2223
h, v := c.Callbacks[0]()
2324

2425
if h != "HEADER" {
25-
t.Errorf(fmt.Sprintf("expected header %s, but got %s", "HEADER", h))
26+
t.Errorf("expected header %s, but got %s", "HEADER", h)
2627
}
2728

2829
if v != "VALUE" {
29-
t.Errorf(fmt.Sprintf("expected value %s, but got %s", "VALUE", v))
30+
t.Errorf("expected value %s, but got %s", "VALUE", v)
3031
}
3132
})
3233

@@ -38,11 +39,11 @@ func TestConfigurations(t *testing.T) {
3839
h, v := c.Callbacks[0]()
3940

4041
if h != "Authorization" {
41-
t.Errorf(fmt.Sprintf("expected header %s, but got %s", "Authorization", h))
42+
t.Errorf("expected header %s, but got %s", "Authorization", h)
4243
}
4344

4445
if v != "Bearer TOKEN" {
45-
t.Errorf(fmt.Sprintf("expected value %s, but got %s", "Bearer TOKEN", v))
46+
t.Errorf("expected value %s, but got %s", "Bearer TOKEN", v)
4647
}
4748
})
4849

@@ -54,44 +55,31 @@ func TestConfigurations(t *testing.T) {
5455
h, v := c.Callbacks[0]()
5556

5657
if h != "X-API-Key" {
57-
t.Errorf(fmt.Sprintf("expected header %s, but got %s", "X-API-Key", h))
58+
t.Errorf("expected header %s, but got %s", "X-API-Key", h)
5859
}
5960

6061
if v != "TOKEN" {
61-
t.Errorf(fmt.Sprintf("expected value %s, but got %s", "TOKEN", v))
62+
t.Errorf("expected value %s, but got %s", "TOKEN", v)
6263
}
6364
})
6465
}
6566

6667
func TestWiringE2E(t *testing.T) {
6768
// mock server with mocked response
68-
host := "localhost:18182"
69-
70-
server := http.Server{
71-
Addr: host,
72-
Handler: mockHandler{
69+
server := httptest.NewServer(
70+
mockHandler{
7371
response: "{\"value\":true,\"key\":\"my-flag\",\"reason\":\"STATIC\",\"variant\":\"true\",\"metadata\":{}}",
7472
t: t,
7573
},
76-
}
77-
78-
go func() {
79-
err := server.ListenAndServe()
80-
if err != nil {
81-
t.Logf("error starting mock server: %v", err)
82-
return
83-
}
84-
}()
85-
86-
// time for server to be ready
87-
<-time.After(3 * time.Second)
74+
)
75+
t.Cleanup(server.Close)
8876

8977
// custom client with reduced timeout
9078
customClient := &http.Client{
9179
Timeout: 1 * time.Second,
9280
}
9381

94-
provider := NewProvider(fmt.Sprintf("http://%s", host), WithClient(customClient))
82+
provider := NewProvider(server.URL, WithClient(customClient))
9583
booleanEvaluation := provider.BooleanEvaluation(context.Background(), "flag", false, nil)
9684

9785
if booleanEvaluation.Value != true {
@@ -109,11 +97,6 @@ func TestWiringE2E(t *testing.T) {
10997
if booleanEvaluation.Error() != nil {
11098
t.Errorf("expected no errors, but got %v", booleanEvaluation.Error())
11199
}
112-
113-
err := server.Shutdown(context.Background())
114-
if err != nil {
115-
t.Errorf("error shuttting down mock server: %v", err)
116-
}
117100
}
118101

119102
type mockHandler struct {

0 commit comments

Comments
 (0)