grafana / k6

A modern load testing tool, using Go and JavaScript - https://k6.io
GNU Affero General Public License v3.0
25.34k stars 1.26k forks source link

Using a function directly for `options.scenarios.*.exec` is confusing #2909

Open mstoykov opened 1 year ago

mstoykov commented 1 year ago

Brief summary

Setting the scenarios exec field to just a function returns a not very useful and unspecific error.

k6 version

v0.27+

OS

all

Docker version and image (if applicable)

No response

Steps to reproduce the problem

Run

export const options = {
    scenarios: {
        scenario1: {
          executor: "constant-arrival-rate",
          rate: 1,
          timeUnit: "1s",
          preallocatedVUs: 1,
          duration: "10s",
          exec: s ,
        }
    }
}

function s () {}

Expected behaviour

At least an error that give some hints on where the problem is.

With a bigger script this just tells you something about json and something called goja . So if you have anything using JSON you are very likely to go look there instead of in your options.

Actual behaviour

ERRO[0000] could not initialize 't.js': could not load JS test 'file:///path/to/script.js': json: unsupported type: func(goja.FunctionCall) goja.Value
mstoykov commented 1 year ago

This is similar to https://github.com/grafana/k6/issues/2579 and https://github.com/grafana/k6/issues/855.

This is the result of calling json.Marshal https://github.com/grafana/k6/blob/a78916c644a4ab741723db5d2050725163a569fe/js/bundle.go#L180-L184 So this is just an error from deep there.

The error that is returned has no additional information. As this is the move it to json the cool UnmarshalTypeError from which you can get the exact field and place in the json this happens. But we here are making the json, so at best we can request a go change to at least have some information on what the field that is encoded was called :shrug: or the path in it.

I don't think the above will go somewhere fast enough or if at all.

Using goja.ExportTo, which is arguably what happens here, gives other error about not being able to convert a map to ScenarioConfigs.

This looks like another dead end as previous attempts to add goja specific Export/Marshal function interface have been rejected.

Proposal 0:

Update the error to at least mention the options so that it is obvious it was in the marshalling of the options not something else. This at least lowers the places a user needs to look for in a big script.

Proposal 1:

Have some specific code that goes through the js object and looks for such problems. This seems fairly okay and likely not the hardest problem, likely a thing that can use some xpath like syntax.

This will let us tell users that for scenario "loginUsers" they are using a function instead of the name of the function and also tell them that that needs to be exported.

A library doing this will be useful in other places as well.

Proposal 2:

Implement specific marshalling code like ExportTo that takes does everything in one go and for each type we can have specific handling for errors. This might leak pretty heavily and will likely need a lot better planning as we don't want the options to depend on goja if possible.

Proposal 2++:

If we have something that gets a function in a place where we want the name of a function, we can't just get the name of the function and call it a day. As the function might not be exported or be exported under a different name or (as is the exact example above).

But we do have the function, so at least in theory we can call it. This does mean that there needs to be special treatment for this case where each VU "parses" this part of the options on its own and gets the function instance.

This will require other changes:

  1. Which function is called is done through it's name from outside the js runner.
  2. We still need to put some value in the JSON :shrug:

Given that this likely will be even more involved and require considerable refactorings in this part of k6.