Skip to content

Commit

Permalink
Call responseCallback with the emitted status code
Browse files Browse the repository at this point in the history
There is still situation where it is possible k6 will get status code
and then while it is reading the rest of the response an error will
occur - timeout reading the body, bad compression, etc. All of those
cases emit status code `0` and return status code `0` to the js script.

But responseCallback was getting the status code directly from the
response which meant that you can have an emitted metric with tag
`status` and `expected_response` where the two don't make sense as the
`status` is `0` but the `expected_response` was calculated based on a
different value.

While it might be more logical to return the status and the error in
this case intact this has been the case for nearly 3 years and given the
way the current js HTTP API is used this will likely just lead to very
confused users which got status `200`, checked on it and then their
response.body was cut halfway through.
  • Loading branch information
mstoykov committed Apr 13, 2021
1 parent b84e0af commit 311b905
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 12 deletions.
30 changes: 19 additions & 11 deletions lib/netext/httpext/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,11 @@ func TestURL(t *testing.T) {

func TestMakeRequestTimeout(t *testing.T) {
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.Header().Add("Content-Length", "100000")
w.WriteHeader(200)
if f, ok := w.(http.Flusher); ok {
f.Flush()
}
time.Sleep(100 * time.Millisecond)
}))
defer srv.Close()
Expand All @@ -246,14 +251,16 @@ func TestMakeRequestTimeout(t *testing.T) {
Transport: srv.Client().Transport,
Samples: samples,
Logger: logger,
BPool: bpool.NewBufferPool(100),
}
ctx = lib.WithState(ctx, state)
req, _ := http.NewRequest("GET", srv.URL, nil)
preq := &ParsedHTTPRequest{
Req: req,
URL: &URL{u: req.URL, URL: srv.URL},
Body: new(bytes.Buffer),
Timeout: 10 * time.Millisecond,
Req: req,
URL: &URL{u: req.URL, URL: srv.URL},
Body: new(bytes.Buffer),
Timeout: 50 * time.Millisecond,
ResponseCallback: func(i int) bool { return i == 0 },
}

res, err := MakeRequest(ctx, preq)
Expand All @@ -262,14 +269,15 @@ func TestMakeRequestTimeout(t *testing.T) {
assert.Len(t, samples, 1)
sampleCont := <-samples
allSamples := sampleCont.GetSamples()
require.Len(t, allSamples, 8)
require.Len(t, allSamples, 9)
expTags := map[string]string{
"error": "context deadline exceeded",
"error_code": "1000",
"status": "0",
"method": "GET",
"url": srv.URL,
"name": srv.URL,
"error": "context deadline exceeded",
"error_code": "1000",
"status": "0",
"expected_response": "true", // we wait for status code 0
"method": "GET",
"url": srv.URL,
"name": srv.URL,
}
for _, s := range allSamples {
assert.Equal(t, expTags, s.Tags.CloneTags())
Expand Down
2 changes: 1 addition & 1 deletion lib/netext/httpext/transport.go
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ func (t *transport) measureAndEmitMetrics(unfReq *unfinishedRequest) *finishedRe
var failed float64
if t.responseCallback != nil {
var statusCode int
if unfReq.response != nil {
if unfReq.err == nil {
statusCode = unfReq.response.StatusCode
}
expected := t.responseCallback(statusCode)
Expand Down

0 comments on commit 311b905

Please sign in to comment.