From 35146ca4b6a9e48a747613a7d7b7544de896cd6a Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Tue, 14 Jun 2022 15:59:05 +0300 Subject: [PATCH] Contain main module within it's own scope as well --- core/engine_test.go | 2 +- js/bundle.go | 55 ++++++++++++++++++++--------- js/bundle_test.go | 13 +++---- js/compiler/compiler.go | 10 ++---- js/initcontext.go | 7 ++-- js/initcontext_test.go | 12 +++---- js/module_loading_test.go | 73 ++++++++++++++++++++++++--------------- js/runner.go | 22 ++++++------ js/runner_test.go | 17 ++++----- 9 files changed, 122 insertions(+), 89 deletions(-) diff --git a/core/engine_test.go b/core/engine_test.go index 2ee88f43ab6..c4a57921d60 100644 --- a/core/engine_test.go +++ b/core/engine_test.go @@ -967,7 +967,7 @@ func TestVuInitException(t *testing.T) { var exception errext.Exception require.ErrorAs(t, err, &exception) - assert.Equal(t, "Error: oops in 2\n\tat file:///script.js:10:9(31)\n", err.Error()) + assert.Equal(t, "Error: oops in 2\n\tat file:///script.js:10:9(30)\n\tat native\n", err.Error()) var errWithHint errext.HasHint require.ErrorAs(t, err, &errWithHint) diff --git a/js/bundle.go b/js/bundle.go index 1c2e55eb176..9efd5a649fd 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -67,9 +67,9 @@ type BundleInstance struct { // TODO: maybe just have a reference to the Bundle? or save and pass rtOpts? env map[string]string - exports map[string]goja.Callable - + exports map[string]goja.Callable moduleVUImpl *moduleVUImpl + pgm programWithSource } // NewBundle creates a new bundle from a source file and a filesystem. @@ -90,7 +90,7 @@ func NewBundle( Strict: true, SourceMapLoader: generateSourceMapLoader(logger, filesystems), } - pgm, _, err := c.Compile(code, src.URL.String(), true) + pgm, _, err := c.Compile(code, src.URL.String(), false) if err != nil { return nil, err } @@ -142,7 +142,7 @@ func NewBundleFromArchive( CompatibilityMode: compatMode, SourceMapLoader: generateSourceMapLoader(logger, arc.Filesystems), } - pgm, _, err := c.Compile(string(arc.Data), arc.FilenameURL.String(), true) + pgm, _, err := c.Compile(string(arc.Data), arc.FilenameURL.String(), false) if err != nil { return nil, err } @@ -208,7 +208,8 @@ func (b *Bundle) makeArchive() *lib.Archive { // getExports validates and extracts exported objects func (b *Bundle) getExports(logger logrus.FieldLogger, rt *goja.Runtime, options bool) error { - exportsV := rt.Get("exports") + pgm := b.BaseInitContext.programs[b.Filename.String()] + exportsV := pgm.module.Get("exports") if goja.IsNull(exportsV) || goja.IsUndefined(exportsV) { return errors.New("exports must be an object") } @@ -262,26 +263,31 @@ func (b *Bundle) Instantiate(logger logrus.FieldLogger, vuID uint64) (*BundleIns } rt := vuImpl.runtime + pgm := init.programs[b.Filename.String()] bi := &BundleInstance{ Runtime: rt, exports: make(map[string]goja.Callable), env: b.RuntimeOptions.Env, moduleVUImpl: vuImpl, + pgm: pgm, } // Grab any exported functions that could be executed. These were // already pre-validated in cmd.validateScenarioConfig(), just get them here. - exports := rt.Get("exports").ToObject(rt) + exports := pgm.module.Get("exports").ToObject(rt) for k := range b.exports { fn, _ := goja.AssertFunction(exports.Get(k)) bi.exports[k] = fn } - jsOptions := rt.Get("options") + jsOptions := exports.Get("options") var jsOptionsObj *goja.Object if jsOptions == nil || goja.IsNull(jsOptions) || goja.IsUndefined(jsOptions) { jsOptionsObj = rt.NewObject() - rt.Set("options", jsOptionsObj) + err := exports.Set("options", jsOptionsObj) + if err != nil { + return nil, fmt.Errorf("couldn't set exported options with merged values: %w", err) + } } else { jsOptionsObj = jsOptions.ToObject(rt) } @@ -302,12 +308,6 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init * rt.SetFieldNameMapper(common.FieldNameMapper{}) rt.SetRandSource(common.NewRandSource()) - exports := rt.NewObject() - rt.Set("exports", exports) - module := rt.NewObject() - _ = module.Set("exports", exports) - rt.Set("module", module) - env := make(map[string]string, len(b.RuntimeOptions.Env)) for key, value := range b.RuntimeOptions.Env { env[key] = value @@ -330,10 +330,27 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init * init.moduleVUImpl.ctx = context.Background() init.moduleVUImpl.initEnv = initenv init.moduleVUImpl.eventLoop = eventloop.New(init.moduleVUImpl) + pgm := init.programs[b.Filename.String()] // this just initializes the program + pgm.pgm = b.Program + pgm.src = b.Source + exports := rt.NewObject() + pgm.module = rt.NewObject() + _ = pgm.module.Set("exports", exports) + init.programs[b.Filename.String()] = pgm + err = common.RunWithPanicCatching(logger, rt, func() error { return init.moduleVUImpl.eventLoop.Start(func() error { - _, errRun := rt.RunProgram(b.Program) - return errRun + f, errRun := rt.RunProgram(b.Program) + if errRun != nil { + return errRun + } + if call, ok := goja.AssertFunction(f); ok { + if _, errRun = call(exports, pgm.module, exports); errRun != nil { + return errRun + } + return nil + } + panic("we shouldn't be able to get here") }) }) @@ -344,6 +361,12 @@ func (b *Bundle) instantiate(logger logrus.FieldLogger, rt *goja.Runtime, init * } return err } + exportsV := pgm.module.Get("exports") + if goja.IsNull(exportsV) { + return errors.New("exports must be an object") + } + pgm.exports = exportsV.ToObject(rt) + init.programs[b.Filename.String()] = pgm unbindInit() init.moduleVUImpl.ctx = nil init.moduleVUImpl.initEnv = nil diff --git a/js/bundle_test.go b/js/bundle_test.go index 1cfe73a8bb2..f94ccce7fcf 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -96,11 +96,11 @@ func TestNewBundle(t *testing.T) { _, err := getSimpleBundle(t, "/script.js", `throw new Error("aaaa");`) exception := new(scriptException) require.ErrorAs(t, err, &exception) - require.EqualError(t, err, "Error: aaaa\n\tat file:///script.js:1:7(2)\n") + require.EqualError(t, err, "Error: aaaa\n\tat file:///script.js:2:7(3)\n\tat native\n") }) t.Run("InvalidExports", func(t *testing.T) { t.Parallel() - _, err := getSimpleBundle(t, "/script.js", `exports = null`) + _, err := getSimpleBundle(t, "/script.js", `module.exports = null`) require.EqualError(t, err, "exports must be an object") }) t.Run("DefaultUndefined", func(t *testing.T) { @@ -169,13 +169,13 @@ func TestNewBundle(t *testing.T) { // ES2015 modules are not supported { "Modules", "base", `export default function() {};`, - "file:///script.js: Line 1:1 Unexpected reserved word", + "file:///script.js: Line 2:1 Unexpected reserved word (and 2 more errors)", }, // BigInt is not supported { "BigInt", "base", `module.exports.default = function() {}; BigInt(1231412444)`, - "ReferenceError: BigInt is not defined\n\tat file:///script.js:1:47(6)\n", + "ReferenceError: BigInt is not defined\n\tat file:///script.js:2:47(7)\n\tat native\n", }, } @@ -763,6 +763,7 @@ func TestBundleInstantiate(t *testing.T) { t.Run("SetAndRun", func(t *testing.T) { t.Parallel() + t.Skip("This makes no sense for a test we are basically testing that we can reset global") b, err := getSimpleBundle(t, "/script.js", ` export let options = { vus: 5, @@ -798,7 +799,7 @@ func TestBundleInstantiate(t *testing.T) { bi, err := b.Instantiate(logger, 0) require.NoError(t, err) // Ensure `options` properties are correctly marshalled - jsOptions := bi.Runtime.Get("options").ToObject(bi.Runtime) + jsOptions := bi.pgm.exports.Get("options").ToObject(bi.Runtime) vus := jsOptions.Get("vus").Export() require.Equal(t, int64(5), vus) tdt := jsOptions.Get("teardownTimeout").Export() @@ -809,7 +810,7 @@ func TestBundleInstantiate(t *testing.T) { b.Options.VUs = null.IntFrom(10) bi2, err := b.Instantiate(logger, 0) require.NoError(t, err) - jsOptions = bi2.Runtime.Get("options").ToObject(bi2.Runtime) + jsOptions = bi2.pgm.exports.Get("options").ToObject(bi2.Runtime) vus = jsOptions.Get("vus").Export() require.Equal(t, int64(10), vus) b.Options.VUs = optOrig diff --git a/js/compiler/compiler.go b/js/compiler/compiler.go index 01cdb94b1c7..0bc84f67679 100644 --- a/js/compiler/compiler.go +++ b/js/compiler/compiler.go @@ -198,10 +198,7 @@ func (c *Compiler) Compile(src, filename string, main bool) (*goja.Program, stri // additioanlly it fixes off by one error in commonjs dependencies due to having to wrap them in a function. func (c *compilationState) sourceMapLoader(path string) ([]byte, error) { if path == sourceMapURLFromBabel { - if !c.main { - return c.increaseMappingsByOne(c.srcMap) - } - return c.srcMap, nil + return c.increaseMappingsByOne(c.srcMap) } c.srcMap, c.srcMapError = c.compiler.Options.SourceMapLoader(path) if c.srcMapError != nil { @@ -214,10 +211,7 @@ func (c *compilationState) sourceMapLoader(path string) ([]byte, error) { c.srcMap = nil return nil, c.srcMapError } - if !c.main { - return c.increaseMappingsByOne(c.srcMap) - } - return c.srcMap, nil + return c.increaseMappingsByOne(c.srcMap) } func (c *Compiler) compileImpl( diff --git a/js/initcontext.go b/js/initcontext.go index b2ad9079414..9d71cf44080 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -54,9 +54,10 @@ import ( ) type programWithSource struct { - pgm *goja.Program - src string - module *goja.Object + pgm *goja.Program + src string + module *goja.Object + exports *goja.Object } const openCantBeUsedOutsideInitContextMsg = `The "open()" function is only available in the init stage ` + diff --git a/js/initcontext_test.go b/js/initcontext_test.go index 3f7bbdd6991..2b27e2d753b 100644 --- a/js/initcontext_test.go +++ b/js/initcontext_test.go @@ -70,13 +70,13 @@ func TestInitContextRequire(t *testing.T) { bi, err := b.Instantiate(logger, 0) assert.NoError(t, err, "instance error") - exports := bi.Runtime.Get("exports").ToObject(bi.Runtime) + exports := bi.pgm.exports require.NotNil(t, exports) _, defaultOk := goja.AssertFunction(exports.Get("default")) assert.True(t, defaultOk, "default export is not a function") assert.Equal(t, "abc123", exports.Get("dummy").String()) - k6 := bi.Runtime.Get("_k6").ToObject(bi.Runtime) + k6 := exports.Get("_k6").ToObject(bi.Runtime) require.NotNil(t, k6) _, groupOk := goja.AssertFunction(k6.Get("group")) assert.True(t, groupOk, "k6.group is not a function") @@ -96,7 +96,7 @@ func TestInitContextRequire(t *testing.T) { bi, err := b.Instantiate(logger, 0) require.NoError(t, err) - exports := bi.Runtime.Get("exports").ToObject(bi.Runtime) + exports := bi.pgm.exports require.NotNil(t, exports) _, defaultOk := goja.AssertFunction(exports.Get("default")) assert.True(t, defaultOk, "default export is not a function") @@ -130,7 +130,7 @@ func TestInitContextRequire(t *testing.T) { require.NoError(t, afero.WriteFile(fs, "/file.js", []byte(`throw new Error("aaaa")`), 0o755)) _, err := getSimpleBundle(t, "/script.js", `import "/file.js"; export default function() {}`, fs) assert.EqualError(t, err, - "Error: aaaa\n\tat file:///file.js:2:7(3)\n\tat go.k6.io/k6/js.(*InitContext).Require-fm (native)\n\tat file:///script.js:1:0(14)\n") + "Error: aaaa\n\tat file:///file.js:2:7(3)\n\tat go.k6.io/k6/js.(*InitContext).Require-fm (native)\n\tat file:///script.js:1:0(15)\n\tat native\n") }) imports := map[string]struct { @@ -282,7 +282,7 @@ func TestInitContextOpen(t *testing.T) { t.Parallel() bi, err := createAndReadFile(t, tc.file, tc.content, tc.length, "") require.NoError(t, err) - assert.Equal(t, string(tc.content), bi.Runtime.Get("data").Export()) + assert.Equal(t, string(tc.content), bi.pgm.exports.Get("data").Export()) }) } @@ -291,7 +291,7 @@ func TestInitContextOpen(t *testing.T) { bi, err := createAndReadFile(t, "/path/to/file.bin", []byte("hi!\x0f\xff\x01"), 6, "b") require.NoError(t, err) buf := bi.Runtime.NewArrayBuffer([]byte{104, 105, 33, 15, 255, 1}) - assert.Equal(t, buf, bi.Runtime.Get("data").Export()) + assert.Equal(t, buf, bi.pgm.exports.Get("data").Export()) }) testdata := map[string]string{ diff --git a/js/module_loading_test.go b/js/module_loading_test.go index 31ab2ac4dd0..63d4c10d1b9 100644 --- a/js/module_loading_test.go +++ b/js/module_loading_test.go @@ -314,10 +314,10 @@ func TestLoadCycle(t *testing.T) { // This is mostly the example from https://hacks.mozilla.org/2018/03/es-modules-a-cartoon-deep-dive/ fs := afero.NewMemMapFs() require.NoError(t, afero.WriteFile(fs, "/counter.js", []byte(` - let message = require("./main.js").message; + let main = require("./main.js"); exports.count = 5; export function a() { - return message; + return main.message; } `), os.ModePerm)) @@ -608,27 +608,36 @@ func TestLoadingSourceMapsDoesntErrorOut(t *testing.T) { } } -func TestShowcasingHowOptionsAreGlobalReadable(t *testing.T) { +func TestOptionsAreGloballyReadable(t *testing.T) { t.Parallel() fs := afero.NewMemMapFs() require.NoError(t, afero.WriteFile(fs, "/A.js", []byte(` - export function A() { + export function A() { // we can technically get a field set from outside of js this way - return options.someField; - } - `), os.ModePerm)) + return options.someField; + }`), os.ModePerm)) r1, err := getSimpleRunner(t, "/script.js", ` - import { A } from "./A.js"; - export let options = { - someField: "here is an option", - } - - export default function(data) { - if (A() != "here is an option") { - throw "oops" - } - } - `, fs, lib.RuntimeOptions{CompatibilityMode: null.StringFrom("extended")}) + import { A } from "./A.js"; + export let options = { + someField: "here is an option", + } + + export default function(data) { + var caught = false; + try{ + if (A() == "here is an option") { + throw "oops" + } + } catch(e) { + if (e.message != "options is not defined") { + throw e; + } + caught = true; + } + if (!caught) { + throw "expected exception" + } + } `, fs, lib.RuntimeOptions{CompatibilityMode: null.StringFrom("extended")}) require.NoError(t, err) arc := r1.MakeArchive() @@ -660,26 +669,36 @@ func TestShowcasingHowOptionsAreGlobalReadable(t *testing.T) { } } -func TestShowcasingHowOptionsAreGlobalWritable(t *testing.T) { +func TestOptionsAreNotGloballyWritable(t *testing.T) { t.Parallel() fs := afero.NewMemMapFs() require.NoError(t, afero.WriteFile(fs, "/A.js", []byte(` export function A() { - // this requires that this is defined - options.minIterationDuration = "1h" - } - `), os.ModePerm)) + // this requires that this is defined + options.minIterationDuration = "1h" + }`), os.ModePerm)) r1, err := getSimpleRunner(t, "/script.js", ` import {A} from "/A.js" export let options = {minIterationDuration: "5m"} export default () =>{} - A() - `, fs, lib.RuntimeOptions{CompatibilityMode: null.StringFrom("extended")}) + var caught = false; + try{ + A() + } catch(e) { + if (e.message != "options is not defined") { + throw e; + } + caught = true; + } + + if (!caught) { + throw "expected exception" + }`, fs, lib.RuntimeOptions{CompatibilityMode: null.StringFrom("extended")}) require.NoError(t, err) // here it exists - require.EqualValues(t, time.Hour, r1.GetOptions().MinIterationDuration.Duration) + require.EqualValues(t, time.Minute*5, r1.GetOptions().MinIterationDuration.Duration) arc := r1.MakeArchive() registry := metrics.NewRegistry() builtinMetrics := metrics.RegisterBuiltinMetrics(registry) @@ -690,5 +709,5 @@ func TestShowcasingHowOptionsAreGlobalWritable(t *testing.T) { }, arc) require.NoError(t, err) - require.EqualValues(t, time.Hour, r2.GetOptions().MinIterationDuration.Duration) + require.EqualValues(t, time.Minute*5, r2.GetOptions().MinIterationDuration.Duration) } diff --git a/js/runner.go b/js/runner.go index b98eed4a9b6..53ce94c45b3 100644 --- a/js/runner.go +++ b/js/runner.go @@ -382,13 +382,11 @@ func (r *Runner) HandleSummary(ctx context.Context, summary *lib.Summary) (map[s } handleSummaryFn := goja.Undefined() - if exported := vu.Runtime.Get("exports").ToObject(vu.Runtime); exported != nil { - fn := exported.Get(consts.HandleSummaryFn) - if _, ok := goja.AssertFunction(fn); ok { - handleSummaryFn = fn - } else if fn != nil { - return nil, fmt.Errorf("exported identifier %s must be a function", consts.HandleSummaryFn) - } + fn := vu.getExported(consts.HandleSummaryFn) + if _, ok := goja.AssertFunction(fn); ok { + handleSummaryFn = fn + } else if fn != nil { + return nil, fmt.Errorf("exported identifier %s must be a function", consts.HandleSummaryFn) } ctx, cancel := context.WithTimeout(ctx, r.getTimeoutFor(consts.HandleSummaryFn)) @@ -522,11 +520,7 @@ func (r *Runner) runPart( if err != nil { return goja.Undefined(), err } - exp := vu.Runtime.Get("exports").ToObject(vu.Runtime) - if exp == nil { - return goja.Undefined(), nil - } - fn, ok := goja.AssertFunction(exp.Get(name)) + fn, ok := goja.AssertFunction(vu.getExported(name)) if !ok { return goja.Undefined(), nil } @@ -769,6 +763,10 @@ func (u *ActiveVU) RunOnce() error { return err } +func (u *VU) getExported(name string) goja.Value { + return u.BundleInstance.pgm.module.Get("exports").ToObject(u.Runtime).Get(name) +} + // if isDefault is true, cancel also needs to be provided and it should cancel the provided context // TODO remove the need for the above through refactoring of this function and its callees func (u *VU) runFn( diff --git a/js/runner_test.go b/js/runner_test.go index 74de9aceb1f..70928b28ac1 100644 --- a/js/runner_test.go +++ b/js/runner_test.go @@ -70,8 +70,8 @@ func TestRunnerNew(t *testing.T) { t.Run("Valid", func(t *testing.T) { t.Parallel() r, err := getSimpleRunner(t, "/script.js", ` - var counter = 0; - exports.default = function() { counter++; } + exports.counter = 0; + exports.default = function() { exports.counter++; } `) require.NoError(t, err) @@ -81,7 +81,7 @@ func TestRunnerNew(t *testing.T) { require.NoError(t, err) vuc, ok := initVU.(*VU) require.True(t, ok) - assert.Equal(t, int64(0), vuc.Runtime.Get("counter").Export()) + assert.Equal(t, int64(0), vuc.pgm.exports.Get("counter").Export()) ctx, cancel := context.WithCancel(context.Background()) defer cancel() @@ -89,7 +89,7 @@ func TestRunnerNew(t *testing.T) { t.Run("RunOnce", func(t *testing.T) { err = vu.RunOnce() require.NoError(t, err) - assert.Equal(t, int64(1), vuc.Runtime.Get("counter").Export()) + assert.Equal(t, int64(1), vuc.pgm.exports.Get("counter").Export()) }) }) }) @@ -97,7 +97,7 @@ func TestRunnerNew(t *testing.T) { t.Run("Invalid", func(t *testing.T) { t.Parallel() _, err := getSimpleRunner(t, "/script.js", `blarg`) - assert.EqualError(t, err, "ReferenceError: blarg is not defined\n\tat file:///script.js:1:1(0)\n") + assert.EqualError(t, err, "ReferenceError: blarg is not defined\n\tat file:///script.js:2:1(1)\n\tat native\n") }) } @@ -155,11 +155,8 @@ func TestOptionsSettingToScript(t *testing.T) { t.Parallel() optionVariants := []string{ - "", - "var options = null;", - "var options = undefined;", - "var options = {};", - "var options = {teardownTimeout: '1s'};", + "export var options = {};", + "export var options = {teardownTimeout: '1s'};", } for i, variant := range optionVariants {