Display a warning when using http.get and http.head with more than 2 arguments

[oleiade]

Brief summary

The current prototype of the http.get and http.head methods can prove to be somewhat error-prone, as they take only two arguments, whereas the other HTTP verb-related methods do take 3.

Although not a bug per-se, this UX can lead to confusion in scenarios like the one I ran into recently: trying to set headers on an HTTP request. Following the docs examples, I intuitively wrote http.get('myurl', null, params) with params containing my headers. But the http.get method would ignore my params completely, as it would treat the second parameter as null params instead of a null body.

k6 version

v0.40.0

OS

Docker version and image (if applicable)

No response

Steps to reproduce the problem

import http from "k6/http";
import { check } from "k6";

export default () => {
  const params = {
    headers: {
      "X-My-Header": "something",
    },
  };

  const res = http.get("http://localhost:3434/latency/50ms", null, params);
  check(res, {
    "status is 200": (r) => r.status === 200,
  });
};

Expected behaviour

It's been discussed internally that to let the user know that this is a misuse of the http.get and http.head methods, we should emit a warning. Something indicating to the user that they're probably doing something wrong: "beware: the http.get and http.head methods do not take a payload argument" or something along those lines would probably do.

Actual behaviour

⚔️

[codebien]

we should emit a warning.

@oleiade Why not directly an error?

[oleiade]

I meant warning as in "a step that, conceptually, warns the user". In practice, an error would do 😉

[juliaokmenezes]

Hi does this issue still make sense? I'd like to work on it if possible

[codebien]

Hi @juliaokmenezes, yes, it still makes sense. Feel free to open a new PR for it or ask for details if something is not clear.

[juliaokmenezes]

Hi @codebien, i will try it! Could you please provide, if possible, indications where in the project the HTTP validations are treated, there are any guideline to run the project?

[codebien]

Hey @juliaokmenezes,

You can find the get and head methods defined here:

k6/js/modules/k6/http/http.go

Lines 71 to 82 in 001aff5

	mustExport("get", func(url goja.Value, args ...goja.Value) (*Response, error) {
	// http.get(url, params) doesn't have a body argument, so we add undefined
	// as the third argument to http.request(method, url, body, params)
	args = append([]goja.Value{goja.Undefined()}, args...)
	return mi.defaultClient.Request(http.MethodGet, url, args...)
	})
	mustExport("head", func(url goja.Value, args ...goja.Value) (*Response, error) {
	// http.head(url, params) doesn't have a body argument, so we add undefined
	// as the third argument to http.request(method, url, body, params)
	args = append([]goja.Value{goja.Undefined()}, args...)
	return mi.defaultClient.Request(http.MethodHead, url, args...)
	})

It sounds like a viable way to define a small validation function for the logic required for this issue and then call it from those methods before invoking the defaultClient.Request.