k6 icon indicating copy to clipboard operation
k6 copied to clipboard

JavaScript spread operator throws error with params object

Open ericksoen opened this issue 7 years ago • 16 comments

It would be nice to have the ability to configure a default params object with some common behaviors (authorization, tags, etc.) that request methods can overwrite using the spread operator as necessary.

For example, this is valid syntax in ES6.

let params = {
    headers: {
        "Content-Type": "Application/JSON",
    },
    timeout: 2000
};

let override = { ...params, timeout: 5000};

console.log(override);
// { headers: { 'Content-Type': 'Application/JSON' }, timeout: 5000 }

When I attempt the same behavior in a K6 test script, I get this error message: image

Code sample to reproduce: google-search.js

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

export default function () {
    let params = {
        headers: {
            "Content-Type": "Application/JSON",
        },
        timeout: 5000
    };
    let requests = {
        "googleSearchWithQueryString": {
            method: "GET",
            url: `https://www.google.com/search?q=k6`,
            params: { ...params, timeout: 1000}
        },        
    };

    let responses = http.batch(requests);

    check(responses["googleSearchWithQueryString"], {
        "add concept code result was 200": res => res.status === 200
    });

}

ericksoen avatar Oct 29 '18 17:10 ericksoen

k6 uses goja as JS VM and goja is only 5.1 ES compliant :(. We also use Babel to transform ES6 to ES5.1 JS but that obviously doesn't work always. There are two plugins for this but I will have to test them before I can tell you if it's going to work. Can you try Object.assign in the meantime?

mstoykov avatar Oct 30 '18 09:10 mstoykov

Thanks. That makes sense as a viable workaround. The fix ends up looking like this

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

export default function () {
    let params = {
        headers: {
            "Content-Type": "Application/JSON",
        },
        timeout: 5000
    };
    let requests = {
        "googleSearchWithQueryString": {
            method: "GET",
            url: `https://www.google.com/search?q=k6`,
            params: Object.assign({}, params, {timeout: 1000})
        },        
    };

    let responses = http.batch(requests);

    check(responses["googleSearchWithQueryString"], {
        "add concept code result was 200": res => res.status === 200
    });

}

// { headers: Object { Content-Type: "Application/JSON" }, timeout: 1000 }

ericksoen avatar Oct 30 '18 19:10 ericksoen

Nice! Going to keep this open for when we decide to actually have the spread operator

mstoykov avatar Oct 30 '18 21:10 mstoykov

I just wanted to add my +1 to this issue because I've run into the same problem on my end. The spread operator is very convenient for having default tags or headers and just extending them on specific requests as I tried here: 07_11_2019__11:34:17

sniku avatar Jul 11 '19 09:07 sniku

More fuel to the fire here. Spread operators ftw! Also thank you for sharing the Object.assign() workaround. I was not aware of that myself.

drfiresign avatar Mar 10 '20 18:03 drfiresign

+1, would really like to see ES6 support.

Spiderpig86 avatar Jul 08 '20 07:07 Spiderpig86

To be fair, we have ES6 support. k6 supports the spread operator in function calls and array expansion, it just doesn't work for object literals, which is something new from ECMAScript 2018.

export default function () {
    console.log(...["this", "works"]);

    console.log(...[...["this", "also"], "works"]);

    // This doesn't work though
    //let a = { ...{ foo: "bar" }, test: "mest" }
}

na-- avatar Jul 08 '20 11:07 na--

Hey k6 team, love your product, but this has bitten me a few times. It's hard to keep track of what's in k6 vs what's in browsers. It would be amazing to keep js support close to what's in modern browser (perhaps by just using v8?).

aboodman avatar Jun 07 '21 19:06 aboodman

Hey k6 team, love your product,

Glad to hear that :hugs:

perhaps by just using v8

As I have heard this a couple of times ... let's discuss what under understatement the just using v8 is ;) : tl;dr "just using v8" is more or less "rewrite k6".

Let's start with that k6 is written in go and for the many great things go does, one thing that it isn't really great is ... calling non-go code (generally C or at least ... something that looks like C). In all cases calling into C in go is expensive as(overly simplified):

  1. Go doesn't actually have the same stack as it is expected by C programs, which means that it needs to ... make the stack look somewhat the same and move to it ... which is expensive ... especially as it probably wants to go back to a "go" stack after that.
  2. Go doesn't have a way to stop the C code execution in the middle in order for the thread to run something else, which is possible with a goroutine (albeit only at specific places(many asterisks here) - this leads to creating more OS threads. I personally got bitten by this as this is the same behavior if you read/write to an FS(as you need to call into the kernel) where a webserver was using 1500+ OS threads ... which wasn't great for performance ;)

For the record depending on which source you like consult, calling a go function vs calling cgo function has a difference between 10 and 100+ times. Which arguably isn't that bad ... if you are calling once, but k6 runs hundreds if not thousands of VMs all of which can effectively require full-blown OS threads and I would argue most of the js code executed is ... not worthy of the JIT in v8.

Some fairly (IMO) common looking k6 code:

var resp = http.get("someurl")
if (resp.json().something.or.other == 12) {
   var resp2 = http.get("another URL");;
   check(resp2, {
      "status 200": (r)=> r.status == 200,
      "has correct body": (r)=>r.json().something.or.other.second.time == "cool value",
   });
}

In this code we mostly just move between js and go and transform (or hopefully don't) values from js ones to go ones. Our current VM - goja, written in golang, does a great job of not actually having to transform from golang types to js types and back with us having to do mostly ... nothing, which means that we can ... add features to k6 instead of fighting the JS VM.

I also just tried to install https://github.com/augustoroman/v8 locally on my fedora and .... no luck, the v8 dependency install script requires ubuntu ... so I will likely manage to make it work with an additional docker in the mix but to be honest given that the last commit is from 2 years ago and they support v8 version 6.7. (object spread was actually added in v6.0 so at least that is in there) I don't think I will go through the hoops to get it to work. But the thing is that there isn't a single up-to-date v8 binding project for go as far as I can find and even if there is one ...

We lose cross-compatibility and make the whole release, build and test process a lot more complicated. K6 is currently being officially released for windows, mac os and Linux with later two supporting both amd64 and arm64 .... this means we need to somehow compile v8(or get it precompiled) for all of those on each release ... and run tests against this. Given my previous adventures in using cgo ... it does add quite a bit to the build time ;). Also there is very little OS specific code, most of which has to do with detecting errors, which arguably means that we support all OSs that k6 compiles to and all anyone needs to do is run go install go.k6.io/k6@latest and ... it should work :tada: . You can look at the repo linked for the steps to get v8(and follow them to get the feel of the difference).

Also of note is that while you might think adding to the build time is not that big of a deal this makes the development slower. Currently k6 builds more or less instantly for small changes and running parts of the test locally is also super fast (under second to a few seconds for some of the bigger packages). Adding any amount of c will explode this build times considerably enough and likely will make the built in race detector not ... able to handle races with the C code(note: v8 is actually in C++ which doesn't help)

Also of importance is that while adding v8 gives us access to a lot of stuff ... we still actually need to integrate them in k6, which arguably will be a lot of work, like for example event loops will require quite a considerable rewrite of how we deal with concurrency in k6. But yeah ... purely syntax changes will just work ;)

One final nail is that currently the js package ... mostly the bindings for js in k6 and relative stuff is around 20k lines and the whole k6 (without the vendored dependencies) is 56k lines so quite a lot of the code is also very js dependant. (Arguably a lot of those are actually tests, but they will still likely need to be updated quite considerably :( )

Given all of the above (I probably could continue, but I see no point listing more stuff) "just using v8" is more or less synonymous with "rewrite k6" ... possibly in not go so we don't suffer all the go problems with cgo, without probably being able to use it lightweight concurrency.

Finally... just so I am not so negative - there is a branch adding destructuring to goja, which also works on the spread operator(it's kind of related ;) ), so if you want you can try and help there as it seemed to mostly work, but still needs to handle even more cases ;)

mstoykov avatar Jun 08 '21 09:06 mstoykov

Thanks for writing back and apologies for the lazy use of the word "just". I get it.

I hope that goja keeps relatively up to date with modern browsers since I enjoy using k6 in large part because I get to reuse skills from web development.

aboodman avatar Jun 08 '21 09:06 aboodman

This is now fixed in https://github.com/grafana/k6/pull/2109

mstoykov avatar Sep 08 '21 08:09 mstoykov

Unfortunately, it turns out that while it's fixed in goja, babel, which will be called on code that goja doesn't support such as import/export does not support it and even though it no longer needs to transpile it, it still needs to parse it so it can do w/e other transformations it needs and than spill it back out.

I am reopening this although I don't know of anything, in particular, we can do apart from removing babel or updating it :shrug:

mstoykov avatar Sep 09 '21 09:09 mstoykov

Plus one here as well. I wanted to use the spread operator for request tagging and it is still not working in the latest K6 version. While looking into it, I stumbled accross this already reported issue.

max-pfeiffer avatar Oct 13 '21 12:10 max-pfeiffer

my +1 as well for this issue. Its convenient to use spread operator. Yes.. object.assign is a workaround but then you need to follow 2 different conventions in your dev code and test code which is not too convenient

nishant-shah-social avatar Nov 17 '22 22:11 nishant-shah-social

Still no spread!?!

DaleMckeown avatar Jun 16 '23 09:06 DaleMckeown

up there

xepozz avatar Feb 05 '24 15:02 xepozz