From cf9a84b7fb5a5254441cf958c68056a9ffeb6911 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Tue, 22 Oct 2024 11:20:07 +0300 Subject: [PATCH 1/3] bare minimum top-level-await support --- js/modules/resolution.go | 3 +-- js/tc39/breaking_test_errors-experimental_enhanced.json | 2 ++ js/tc39/breaking_test_errors-extended.json | 2 ++ js/tc39/tc39_test.go | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) diff --git a/js/modules/resolution.go b/js/modules/resolution.go index a651beb5c73..1601905fbd0 100644 --- a/js/modules/resolution.go +++ b/js/modules/resolution.go @@ -262,8 +262,7 @@ func (ms *ModuleSystem) RunSourceData(source *loader.SourceData) (sobek.ModuleRe case sobek.PromiseStateFulfilled: return mod, nil default: - // TODO(@mstoykov): this will require having some callbacks through the code, but should be doable, just not pretty - panic("TLA not supported in k6 at the moment") + return mod, nil } } diff --git a/js/tc39/breaking_test_errors-experimental_enhanced.json b/js/tc39/breaking_test_errors-experimental_enhanced.json index 7c4ed0b7868..da3e68f87fa 100644 --- a/js/tc39/breaking_test_errors-experimental_enhanced.json +++ b/js/tc39/breaking_test_errors-experimental_enhanced.json @@ -82,6 +82,8 @@ "test/language/module-code/instn-star-err-not-found.js-strict:true": "test/language/module-code/instn-star-err-not-found.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/module-code/parse-err-hoist-lex-fun.js-strict:true": "test/language/module-code/parse-err-hoist-lex-fun.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/module-code/parse-err-return.js-strict:true": "test/language/module-code/parse-err-return.js: error is not an object (Test262: This statement should not be evaluated.)", + "test/language/module-code/top-level-await/new-await.js-strict:true": "test/language/module-code/top-level-await/new-await.js: error is not an object (Test262: This statement should not be evaluated.)", + "test/language/module-code/top-level-await/no-operand.js-strict:true": "test/language/module-code/top-level-await/no-operand.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/reserved-words/await-module.js-strict:true": "test/language/reserved-words/await-module.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/statements/class/class-name-ident-await-escaped-module.js-strict:true": "test/language/statements/class/class-name-ident-await-escaped-module.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/statements/class/class-name-ident-await-module.js-strict:true": "test/language/statements/class/class-name-ident-await-module.js: error is not an object (Test262: This statement should not be evaluated.)", diff --git a/js/tc39/breaking_test_errors-extended.json b/js/tc39/breaking_test_errors-extended.json index 7c4ed0b7868..da3e68f87fa 100644 --- a/js/tc39/breaking_test_errors-extended.json +++ b/js/tc39/breaking_test_errors-extended.json @@ -82,6 +82,8 @@ "test/language/module-code/instn-star-err-not-found.js-strict:true": "test/language/module-code/instn-star-err-not-found.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/module-code/parse-err-hoist-lex-fun.js-strict:true": "test/language/module-code/parse-err-hoist-lex-fun.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/module-code/parse-err-return.js-strict:true": "test/language/module-code/parse-err-return.js: error is not an object (Test262: This statement should not be evaluated.)", + "test/language/module-code/top-level-await/new-await.js-strict:true": "test/language/module-code/top-level-await/new-await.js: error is not an object (Test262: This statement should not be evaluated.)", + "test/language/module-code/top-level-await/no-operand.js-strict:true": "test/language/module-code/top-level-await/no-operand.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/reserved-words/await-module.js-strict:true": "test/language/reserved-words/await-module.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/statements/class/class-name-ident-await-escaped-module.js-strict:true": "test/language/statements/class/class-name-ident-await-escaped-module.js: error is not an object (Test262: This statement should not be evaluated.)", "test/language/statements/class/class-name-ident-await-module.js-strict:true": "test/language/statements/class/class-name-ident-await-module.js: error is not an object (Test262: This statement should not be evaluated.)", diff --git a/js/tc39/tc39_test.go b/js/tc39/tc39_test.go index 409989bd8f0..0b7d8537302 100644 --- a/js/tc39/tc39_test.go +++ b/js/tc39/tc39_test.go @@ -56,8 +56,8 @@ var ( featuresBlockList = []string{ "IsHTMLDDA", // not supported at all "async-iteration", // not supported at all - "top-level-await", // not supported at all "String.prototype.replaceAll", // not supported at all, Stage 4 since 2020 + "dynamic-import", // not support in k6 // from Sobek "Symbol.asyncIterator", From 6e84ccad7778324eb22b5acdca5f97b19d56a4e3 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Tue, 22 Oct 2024 19:59:34 +0300 Subject: [PATCH 2/3] top-level-await with error handling --- js/bundle.go | 11 ++++++++++- js/bundle_test.go | 17 ++++++++++++++++- js/modules/require_impl.go | 10 ++++++++++ js/modules/resolution.go | 32 ++++++++++++++++++++++++-------- js/tc39/tc39_test.go | 9 ++++++++- 5 files changed, 68 insertions(+), 11 deletions(-) diff --git a/js/bundle.go b/js/bundle.go index c84f165c241..0001dd8f8f5 100644 --- a/js/bundle.go +++ b/js/bundle.go @@ -331,9 +331,10 @@ func (b *Bundle) instantiate(vuImpl *moduleVUImpl, vuID uint64) (*BundleInstance env: b.preInitState.RuntimeOptions.Env, moduleVUImpl: vuImpl, } + var result *modules.RunSourceDataResult callback := func() error { // this exists so that Sobek catches uncatchable panics such as Interrupt var err error - bi.mainModule, err = modSys.RunSourceData(b.sourceData) + result, err = modSys.RunSourceData(b.sourceData) return err } @@ -346,6 +347,14 @@ func (b *Bundle) instantiate(vuImpl *moduleVUImpl, vuID uint64) (*BundleInstance <-initDone + if err == nil { + var finished bool + bi.mainModule, finished, err = result.Result() + if !finished { + return nil, errors.New("initializing the main module hasn't finished, this is a bug in k6 please report it") + } + } + if err != nil { var exception *sobek.Exception if errors.As(err, &exception) { diff --git a/js/bundle_test.go b/js/bundle_test.go index a0f48a8af7a..0cbc4405101 100644 --- a/js/bundle_test.go +++ b/js/bundle_test.go @@ -126,7 +126,7 @@ func TestNewBundle(t *testing.T) { t.Run("InvalidExports", func(t *testing.T) { t.Parallel() _, err := getSimpleBundle(t, "/script.js", `module.exports = null`) - require.EqualError(t, err, "GoError: CommonJS's exports must not be null\n") // TODO: try to remove the GoError from herer + require.EqualError(t, err, "CommonJS's exports must not be null") }) t.Run("DefaultUndefined", func(t *testing.T) { t.Parallel() @@ -983,3 +983,18 @@ func TestGlobalTimers(t *testing.T) { }) } } + +func TestTopLevelAwaitErrors(t *testing.T) { + t.Parallel() + data := ` + const delay = (delayInms) => { + return new Promise(resolve => setTimeout(resolve, delayInms)); + } + + await delay(10).then(() => {something}); + export default () => {} + ` + + _, err := getSimpleBundle(t, "/script.js", data) + require.ErrorContains(t, err, "ReferenceError: something is not defined") +} diff --git a/js/modules/require_impl.go b/js/modules/require_impl.go index 5798285f16a..21df9644e9a 100644 --- a/js/modules/require_impl.go +++ b/js/modules/require_impl.go @@ -7,6 +7,7 @@ import ( "strings" "github.com/grafana/sobek" + "go.k6.io/k6/loader" ) @@ -47,6 +48,7 @@ func (ms *ModuleSystem) Require(specifier string) (*sobek.Object, error) { } else { panic(fmt.Sprintf("expected sobek.CyclicModuleRecord, but for some reason got a %T", m)) } + promisesThenIgnore(rt, promise) switch promise.State() { case sobek.PromiseStateRejected: err = promise.Result().Export().(error) //nolint:forcetypeassert @@ -195,3 +197,11 @@ func getPreviousRequiringFile(vu VU) (string, error) { } return result, nil } + +// sets the provided promise in such way as to ignore falures +// this is mostly needed as failures are handled separately and we do not want those to lead to stopping the event loop +func promisesThenIgnore(rt *sobek.Runtime, promise *sobek.Promise) { + cal, _ := sobek.AssertFunction(rt.ToValue(promise).ToObject(rt).Get("then")) + handler := rt.ToValue(func(_ sobek.Value) {}) + _, _ = cal(rt.ToValue(promise), handler, handler) +} diff --git a/js/modules/resolution.go b/js/modules/resolution.go index 1601905fbd0..ed5015d6ca6 100644 --- a/js/modules/resolution.go +++ b/js/modules/resolution.go @@ -233,10 +233,7 @@ func NewModuleSystem(resolver *ModuleResolver, vu VU) *ModuleSystem { // RunSourceData runs the provided sourceData and adds it to the cache. // If a module with the same specifier as the source is already cached // it will be used instead of reevaluating the source from the provided SourceData. -// -// TODO: this API will likely change as native ESM support will likely not let us have the exports -// as one big sobek.Value that we can manipulate -func (ms *ModuleSystem) RunSourceData(source *loader.SourceData) (sobek.ModuleRecord, error) { +func (ms *ModuleSystem) RunSourceData(source *loader.SourceData) (*RunSourceDataResult, error) { specifier := source.URL.String() pwd := source.URL.JoinPath("../") if _, err := ms.resolver.resolveLoaded(pwd, specifier, source.Data); err != nil { @@ -256,13 +253,32 @@ func (ms *ModuleSystem) RunSourceData(source *loader.SourceData) (sobek.ModuleRe } rt := ms.vu.Runtime() promise := rt.CyclicModuleRecordEvaluate(ci, ms.resolver.sobekModuleResolver) - switch promise.State() { + + promisesThenIgnore(rt, promise) + + return &RunSourceDataResult{ + promise: promise, + mod: mod, + }, nil +} + +// RunSourceDataResult helps with the asynchronous nature of ESM +// it wraps the promise that is returned from Sobek while at the same time allowing access to the module record +type RunSourceDataResult struct { + promise *sobek.Promise + mod sobek.ModuleRecord +} + +// Result returns either the underlying module or error if the promise has been completed and true, +// or false if the promise still hasn't been completed +func (r *RunSourceDataResult) Result() (sobek.ModuleRecord, bool, error) { + switch r.promise.State() { case sobek.PromiseStateRejected: - return nil, promise.Result().Export().(error) //nolint:forcetypeassert + return nil, true, r.promise.Result().Export().(error) //nolint:forcetypeassert case sobek.PromiseStateFulfilled: - return mod, nil + return r.mod, true, nil default: - return mod, nil + return nil, false, nil } } diff --git a/js/tc39/tc39_test.go b/js/tc39/tc39_test.go index 0b7d8537302..9098de9d3cc 100644 --- a/js/tc39/tc39_test.go +++ b/js/tc39/tc39_test.go @@ -737,10 +737,17 @@ func (ctx *tc39TestCtx) runTC39Module(name, src string, includes []string, vm *s moduleRuntime.VU.InitEnvField.CWD = base early = false - _, err = ms.RunSourceData(&loader.SourceData{ + result, err := ms.RunSourceData(&loader.SourceData{ Data: []byte(src), URL: u, }) + if err == nil { + var finished bool + _, finished, err = result.Result() + if !finished { + panic("tc39 has no tests where this should happen") + } + } return early, err } From 031cc30c17bc81b7e6f06814d4714e729b1e3e53 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov <312246+mstoykov@users.noreply.github.com> Date: Sat, 26 Oct 2024 11:14:57 +0300 Subject: [PATCH 3/3] Update js/modules/require_impl.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Joan López de la Franca Beltran <5459617+joanlopez@users.noreply.github.com> --- js/modules/require_impl.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/modules/require_impl.go b/js/modules/require_impl.go index 21df9644e9a..4663d66d28e 100644 --- a/js/modules/require_impl.go +++ b/js/modules/require_impl.go @@ -201,7 +201,7 @@ func getPreviousRequiringFile(vu VU) (string, error) { // sets the provided promise in such way as to ignore falures // this is mostly needed as failures are handled separately and we do not want those to lead to stopping the event loop func promisesThenIgnore(rt *sobek.Runtime, promise *sobek.Promise) { - cal, _ := sobek.AssertFunction(rt.ToValue(promise).ToObject(rt).Get("then")) + call, _ := sobek.AssertFunction(rt.ToValue(promise).ToObject(rt).Get("then")) handler := rt.ToValue(func(_ sobek.Value) {}) - _, _ = cal(rt.ToValue(promise), handler, handler) + _, _ = call(rt.ToValue(promise), handler, handler) }