Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix respect throw option with invalid URLs #2045

Merged
merged 8 commits into from
Jun 8, 2021
Merged

Fix respect throw option with invalid URLs #2045

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
1dc120b
Fix throw not being respected on Malformed URL
Feb 11, 2021
2692aa9
Define custom K6Error for invalid URL, respect throw with http.batch()
May 21, 2021
17f6df6
Return Response objects even when URL is invalid
Jun 4, 2021
b894ab7
Remove rendering of original error in K6Error
Jun 4, 2021
39a0ed9
Pass request URL as interface{} instead of goja.Value
Jun 7, 2021
6b8cd30
Remove redundant conversion to goja.Value
Jun 7, 2021
316d04f
Fix panic when calling json() on an empty Response
Jun 8, 2021
af755c4
Fix panic for batch requests, return error code
Jun 8, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions js/modules/k6/http/http.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"go.k6.io/k6/js/common"
"go.k6.io/k6/lib"
"go.k6.io/k6/lib/netext"
"go.k6.io/k6/lib/netext/httpext"
)

const (
Expand Down Expand Up @@ -99,14 +100,30 @@ type HTTP struct {
responseCallback func(int) bool
}

// XCookieJar creates a new cookie jar object.
func (*HTTP) XCookieJar(ctx *context.Context) *HTTPCookieJar {
return newCookieJar(ctx)
}

// CookieJar returns the active cookie jar for the current VU.
func (*HTTP) CookieJar(ctx context.Context) (*HTTPCookieJar, error) {
state := lib.GetState(ctx)
if state == nil {
return nil, ErrJarForbiddenInInitContext
}
return &HTTPCookieJar{state.CookieJar, &ctx}, nil
}

// URL creates a new URL from the provided parts
func (*HTTP) URL(parts []string, pieces ...string) (httpext.URL, error) {
var name, urlstr string
for i, part := range parts {
name += part
urlstr += part
if i < len(pieces) {
name += "${}"
urlstr += pieces[i]
}
}
return httpext.NewURL(urlstr, name)
}
60 changes: 0 additions & 60 deletions js/modules/k6/http/http_url.go
View file
Open in desktop

This file was deleted.

104 changes: 64 additions & 40 deletions js/modules/k6/http/request.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ package http
import (
"bytes"
"context"
"errors"
"fmt"
"mime/multipart"
"net/http"
Expand Down Expand Up @@ -89,9 +90,9 @@ func (h *HTTP) Options(ctx context.Context, url goja.Value, args ...goja.Value)
// Request makes an http request of the provided `method` and returns a corresponding response by
// taking goja.Values as arguments
func (h *HTTP) Request(ctx context.Context, method string, url goja.Value, args ...goja.Value) (*Response, error) {
u, err := ToURL(url)
if err != nil {
return nil, err
state := lib.GetState(ctx)
if state == nil {
return nil, ErrHTTPForbiddenInInitContext
}

var body interface{}
Expand All @@ -104,9 +105,19 @@ func (h *HTTP) Request(ctx context.Context, method string, url goja.Value, args
params = args[1]
}

req, err := h.parseRequest(ctx, method, u, body, params)
req, err := h.parseRequest(ctx, method, url, body, params)
if err != nil {
return nil, err
if state.Options.Throw.Bool {
return nil, err
}
state.Logger.WithField("error", err).Warn("Request Failed")
r := httpext.NewResponse(ctx)
r.Error = err.Error()
var k6e httpext.K6Error
if errors.As(err, &k6e) {
r.ErrorCode = int(k6e.Code)
}
return &Response{Response: r}, nil
}

resp, err := httpext.MakeRequest(ctx, req)
Expand All @@ -120,19 +131,27 @@ func (h *HTTP) Request(ctx context.Context, method string, url goja.Value, args
//TODO break this function up
//nolint: gocyclo
func (h *HTTP) parseRequest(
ctx context.Context, method string, reqURL httpext.URL, body interface{}, params goja.Value,
ctx context.Context, method string, reqURL, body interface{}, params goja.Value,
) (*httpext.ParsedHTTPRequest, error) {
rt := common.GetRuntime(ctx)
state := lib.GetState(ctx)
if state == nil {
return nil, ErrHTTPForbiddenInInitContext
}

if urlJSValue, ok := reqURL.(goja.Value); ok {
reqURL = urlJSValue.Export()
}
u, err := httpext.ToURL(reqURL)
if err != nil {
return nil, err
}

result := &httpext.ParsedHTTPRequest{
URL: &reqURL,
URL: &u,
Req: &http.Request{
Method: method,
URL: reqURL.GetURL(),
URL: u.GetURL(),
Header: make(http.Header),
},
Timeout: 60 * time.Second,
Expand Down Expand Up @@ -380,16 +399,22 @@ func (h *HTTP) prepareBatchArray(
results := make([]*Response, reqCount)

for i, req := range requests {
resp := httpext.NewResponse(ctx)
parsedReq, err := h.parseBatchRequest(ctx, i, req)
if err != nil {
return nil, nil, err
resp.Error = err.Error()
var k6e httpext.K6Error
if errors.As(err, &k6e) {
resp.ErrorCode = int(k6e.Code)
}
results[i] = h.responseFromHttpext(resp)
return batchReqs, results, err
}
response := new(httpext.Response)
batchReqs[i] = httpext.BatchParsedHTTPRequest{
ParsedHTTPRequest: parsedReq,
Response: response,
Response: resp,
}
results[i] = h.responseFromHttpext(response)
results[i] = h.responseFromHttpext(resp)
}

return batchReqs, results, nil
Expand All @@ -404,16 +429,22 @@ func (h *HTTP) prepareBatchObject(

i := 0
for key, req := range requests {
resp := httpext.NewResponse(ctx)
parsedReq, err := h.parseBatchRequest(ctx, key, req)
if err != nil {
return nil, nil, err
resp.Error = err.Error()
var k6e httpext.K6Error
if errors.As(err, &k6e) {
resp.ErrorCode = int(k6e.Code)
}
results[key] = h.responseFromHttpext(resp)
return batchReqs, results, err
}
response := new(httpext.Response)
batchReqs[i] = httpext.BatchParsedHTTPRequest{
ParsedHTTPRequest: parsedReq,
Response: response,
Response: resp,
}
results[key] = h.responseFromHttpext(response)
results[key] = h.responseFromHttpext(resp)
i++
}

Expand All @@ -422,7 +453,7 @@ func (h *HTTP) prepareBatchObject(

// Batch makes multiple simultaneous HTTP requests. The provideds reqsV should be an array of request
// objects. Batch returns an array of responses and/or error
func (h *HTTP) Batch(ctx context.Context, reqsV goja.Value) (goja.Value, error) {
func (h *HTTP) Batch(ctx context.Context, reqsV goja.Value) (interface{}, error) {
state := lib.GetState(ctx)
if state == nil {
return nil, ErrBatchForbiddenInInitContext
Expand All @@ -444,7 +475,11 @@ func (h *HTTP) Batch(ctx context.Context, reqsV goja.Value) (goja.Value, error)
}

if err != nil {
return nil, err
if state.Options.Throw.Bool {
return nil, err
}
state.Logger.WithField("error", err).Warn("A batch request failed")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we fine with this returning a different message than for a single request failure (Request Failed)? I thought it would be useful for troubleshooting to know it was a batch request, but maybe this should have the same message for consistency?

return results, nil
}

reqCount := len(batchReqs)
Expand All @@ -459,20 +494,18 @@ func (h *HTTP) Batch(ctx context.Context, reqsV goja.Value) (goja.Value, error)
err = e
}
}
return common.GetRuntime(ctx).ToValue(results), err
return results, err
}

func (h *HTTP) parseBatchRequest(
ctx context.Context, key interface{}, val interface{},
) (*httpext.ParsedHTTPRequest, error) {
var (
method = HTTP_METHOD_GET
ok bool
err error
reqURL httpext.URL
body interface{}
params goja.Value
rt = common.GetRuntime(ctx)
method = HTTP_METHOD_GET
ok bool
body, reqURL interface{}
params goja.Value
rt = common.GetRuntime(ctx)
)

switch data := val.(type) {
Expand All @@ -486,10 +519,7 @@ func (h *HTTP) parseBatchRequest(
if !ok {
return nil, fmt.Errorf("invalid method type '%#v'", data[0])
}
reqURL, err = ToURL(data[1])
if err != nil {
return nil, err
}
reqURL = data[1]
if dataLen > 2 {
body = data[2]
}
Expand All @@ -499,12 +529,11 @@ func (h *HTTP) parseBatchRequest(

case map[string]interface{}:
// Handling of {method: "GET", url: "https://test.k6.io"}
if murl, ok := data["url"]; !ok {
return nil, fmt.Errorf("batch request %q doesn't have an url key", key)
} else if reqURL, err = ToURL(murl); err != nil {
return nil, err
if _, ok := data["url"]; !ok {
return nil, fmt.Errorf("batch request %v doesn't have a url key", key)
}

reqURL = data["url"]
body = data["body"] // It's fine if it's missing, the map lookup will return

if newMethod, ok := data["method"]; ok {
Expand All @@ -520,13 +549,8 @@ func (h *HTTP) parseBatchRequest(
if p, ok := data["params"]; ok {
params = rt.ToValue(p)
}

default:
// Handling of "http://example.com/" or http.url`http://example.com/{$id}`
reqURL, err = ToURL(data)
if err != nil {
return nil, err
}
reqURL = val
}

return h.parseRequest(ctx, method, reqURL, body, params)
Expand Down
Loading