Skip to content

Commit 0542636

Browse files
committed
lib/netext/httpext: set request context explicitly
Go 1.13 and below has a bug that client timeout will not be propagated to http request context. That bug was fixed already and will be present in go1.14 (golang/go#31657). Currently, we set Timeout in our Client, but do not use it else where. It will not affect the request context for now, but does when go1.14 release. To make it compatible and works correctly with other go versions, we set the request context explicitly with the timeout. Update #1260
1 parent c5a48c4 commit 0542636

File tree

2 files changed

+13
-4
lines changed

2 files changed

+13
-4
lines changed

lib/netext/httpext/request.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -283,7 +283,6 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
283283
resp := &Response{ctx: ctx, URL: preq.URL.URL, Request: *respReq}
284284
client := http.Client{
285285
Transport: transport,
286-
Timeout: preq.Timeout,
287286
CheckRedirect: func(req *http.Request, via []*http.Request) error {
288287
resp.URL = req.URL.String()
289288

@@ -312,7 +311,9 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
312311
},
313312
}
314313

315-
mreq := preq.Req.WithContext(ctx)
314+
reqCtx, cancelFunc := context.WithTimeout(ctx, preq.Timeout)
315+
defer cancelFunc()
316+
mreq := preq.Req.WithContext(reqCtx)
316317
res, resErr := client.Do(mreq)
317318

318319
// TODO(imiric): It would be safer to check for a writeable
@@ -367,7 +368,7 @@ func MakeRequest(ctx context.Context, preq *ParsedHTTPRequest) (*Response, error
367368
}
368369

369370
if resErr != nil {
370-
// Do *not* log errors about the contex being cancelled.
371+
// Do *not* log errors about the context being cancelled.
371372
select {
372373
case <-ctx.Done():
373374
default:

lib/netext/httpext/request_test.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,13 +98,21 @@ func TestMakeRequestError(t *testing.T) {
9898
defer srv.Close()
9999
ctx, cancel := context.WithCancel(context.Background())
100100
defer cancel()
101+
logger := logrus.New()
102+
logger.Level = logrus.DebugLevel
101103
state := &lib.State{
102104
Options: lib.Options{RunTags: &stats.SampleTags{}},
103105
Transport: srv.Client().Transport,
106+
Logger: logger,
104107
}
105108
ctx = lib.WithState(ctx, state)
106109
req, _ := http.NewRequest("GET", srv.URL, nil)
107-
var preq = &ParsedHTTPRequest{Req: req, URL: &URL{u: req.URL}, Body: new(bytes.Buffer)}
110+
var preq = &ParsedHTTPRequest{
111+
Req: req,
112+
URL: &URL{u: req.URL},
113+
Body: new(bytes.Buffer),
114+
Timeout: 10 * time.Second,
115+
}
108116

109117
res, err := MakeRequest(ctx, preq)
110118

0 commit comments

Comments
 (0)