Let open and require be used outside of the init context, somewhat

[mstoykov]

Description

Both open and require are only allowed in the init context.
This is in big parts so that k6 archive script.js (and consequently k6 cloud and any future k6 distributed-run ...) to be able to gather all the files it needs by runnign the init context a single time.

This still let users open files randomly in the init context and we made this more consistent with #2314. Where only files opened by the first VU to initialize are openable in the rest of the VUs as well.

Proposal

Let open be used wherever, but only files opened in the original init context can be opened.

Do the same for require. Probably a good idea to also limit which k6/* are imported as well ... at least in the beginning.

Use cases

Open

• Only open one of many binary files when the given iteration needs it
• Only open files in the scenarios that need them.

Require

• Only require a big library in the scenarios that need it.
• Only require a big library if the need arises.

Caveats

This isn't meant to fix any of the Use case entirely.

But the droppage of the open restriction will in practice be simplifying the implementation.

require likely should get restrictions similar to #2314 and tests to confirm that those work for:

1. local files
2. remote files
3. internal modules

Dynamic import in practice will have similar problems like require.

We can always relax all of those APIs in the future if we decided to.

[pablochacin]

Let open be used wherever, but only files opened in the original init context can be opened.

The phrasing here sounds strange: how can a file opened in the init be opened elsewhere?

[mstoykov]

The phrasing here sounds strange: how can a file opened in the init be opened elsewhere?

I am not certain I get your question.

It currently is not possible and this is basically issue to lift this limitation.

Specifically currently

if (__VU == 0) { // we likely should have better way to detect this that only runs on the really first init context
  open("./test0.bin");
  open("./test1.bin");
  // more opens
  open("./test99.bin");
}

export default function() {
  open(`./test${__ITER%100}.bin`); // this will never work and will tell you `open` can't be used outside the init context.
}

Does that answer your question @pablochacin ?

[na--]

I think we should probably not mess with open() and require() anymore 🤔

The current super-global open() already has enough problems with memory usage and its poor UX, so we want to deprecate it and replace it with a better API (#2977). The focus should be there. However, I think it's worth considering if that new file API can have some sort of a 2-phase action... 🤔

For example, we can provide some mechanism for users to mark files for inclusion in the archive bundles for distributed tests. Or, maybe we can make it so they must always first open (without fully reading) a file in the init context, so k6 can automatically mark it for inclusion. And then, if a file was marked (or pre-opened) in the init context, the user should be able to work with it however they want in the VU context, including re-reading it in a streaming fashion from the start multiple times.

Regarding require(), assuming we have some similar mechanism for marking/pre-opening files in the init context, I am not sure what the consequences of allowing it outside of the init context would be 🤔 I don't understand the implications for native support for ES modules or dynamic import, but I trust you to know what you are doing there. However, it seems to me that the problems that you describe ("Only require a big library in the scenarios that need it.", "Only require a big library if the need arises.") would be better solved by test suites (#1342).

I am not necessarily against relaxing require() restrictions down the line, but it seems to me that it can wait and we should start with the file API improvements first. After all, if we have that, users that really want require() in VU context can eval('function() {' + readall(someJsFileHandle) + '}();'), which is not far off from what require() actually does 😅

[mstoykov]

I feel like the pushback here is that this is more work ... but arguably for open this is less work as it is mostly deleting a bunch a few lines we currently add:

k6/js/bundle.go

Lines 372 to 374 in d946fdb

	if modSys.vu.State() != nil { // fix
	return nil, fmt.Errorf(cantBeUsedOutsideInitContextMsg, "open")
	}

k6/js/bundle.go

Line 384 in d946fdb

mustSet("open", goja.Undefined())

k6/js/runner.go

Lines 256 to 259 in d946fdb

	_ = vu.Runtime.GlobalObject().Set("open",
	func() {
	common.Throw(vu.Runtime, fmt.Errorf(cantBeUsedOutsideInitContextMsg, "open"))
	})

And open will work the way we intend to.

Potentially we might need to change the error message a bit if a file isn't found outside of the init context for better UX.

require is a arguably going to be a bit more involved, but with the latest "moduleSystem" refactor arguably we can have it not return new module instances after some call, the same way we do it for the file system.

k6/js/bundle.go

Lines 317 to 321 in d946fdb

	// If we've already initialized the original VU init context, forbid
	// any subsequent VUs to open new files
	if vuID == 0 {
	allowOnlyOpenedFiles(b.filesystems["file"])
	}

better solved by test suites

Test suites are k6 specific solution to a k6 specific problem. Dynamic import and require are things that people will be using with a general js codebase and might be found through out js projects even now.

So even if test suites were here, it will still not be a great solution in general IMO.

[oleiade]

I somewhat align with what I understand @pablochacin comment to be, and also with the points raised by @na-- 🤝

In the sense that it's all about finding a compelling UI/UX to address the underlying issue: how can we make sure that every file/module referenced by the users are collected by k6 in the archiving process.

I find the initial proposal to double open files a bit confusing and somewhat inelegant from the user perspective, but I get the idea.

Ideation

Some alternatives which would achieve the same that come to my mind (assuming #3101):

Dedicated inclusion functions

We could instead have a dedicated include call or something along those lines?
The user would explicitly declare the files they wish to use in the init context, and k6 would keep track of them internally to make sure those files are included when archiving. I would expect such calls to fail if the resource doesn't exist.

include('my/source/file.txt')
includeDir('testdata')

export default function () {
    // succeeds
    await open('my/source/file.txt');
    
    // fails with 'resource csvs/data.csv not included by k6, have you called `include` on the file, or `includeDir` on its parent?
    await open('csvs/data.csv')
}

Dedicated option(s)

I know we're generally rather conservative when it comes to adding options to k6. However, I could see an option work well in that case, so for the sake of completness, here goes:

export const options = {
    // ...
    
    resources: [
        'my/source/file.txt',
        'testdata',
    ]
}

export default function () {
        // succeeds
    await open('my/source/file.txt');
    
    // fails with 'resource csvs/data.csv not included by k6, have you called `include` on the file, or `includeDir` on its parent?
    await open('csvs/data.csv')
}

[codebien]

I'm of the opinion we should split this issue into two dedicated issues. I see that they have most of the same foundations but my understanding is that open and require have different details and will end in specific implementations. They probably will have also different delivery timelines. For this reason, I'll focus my comment on open.

@mstoykov I can definitely see the case where we allow opening a file directly in the iteration function or in any other places different from the init context without having the requirement of doing it first in the init context. And, it is mostly my interpretation of your points in the Use case list.

Only open one of many binary files when the given iteration needs it
Only open files in the scenarios that need them.

But I'm struggling to see what kind of demand we really have for the original issue regarding making open as the proposal in #3020 (comment) and if we really should address it.

The original rationale behind this seems to be:

But the droppage of the open restriction will in practice be simplifying the implementation.

and the case you reported before #3020 (comment) (that seems to me more of an edge case but here I could be really wrong).

At this point, with us moving forward with the File API, should we still consider the simplification a requirement? I think we should open an issue for the new API to address directly the Use cases you reported and that @na-- detailed here #3020 (comment).

[mstoykov]

I'm of the opinion we should split this issue into two dedicated issues

I am okay with that - and likely what should've been done to begin with. The main reason for them to be together is that ... implementation wise they are very similar.

I can definitely see the case where we allow opening a file directly in the iteration function or in any other places different from the init context without having the requirement of doing it first in the init context.

no specifically on

without having the requirement of doing it first in the init context.

This requirement still stands or more accurately the requirement to open all files in the first init context.

the original message:

Let open be used wherever, but only files opened in the original init context can be opened.

Same goes for all js modules.

The whole point is that after the init context we do a bunch of work specifically to not allow this and I am for dropping this and allowing people to be able to just call open normally.

This again simplifies the actual implementation and arguably aligns this old, but very unlikely to ever be removed APIs, with the currently in development new APIs.

Opening files not opened in the first init context will completely break k6 ability to run distributed. And adding an API to "include" file is exactly the same as opening it and not using, so at least for me that is not an argument.