From 64029261e48aa7c791391cd0d473b792613a73cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ivan=20Miri=C4=87?= Date: Mon, 14 Dec 2020 12:48:54 +0100 Subject: [PATCH] Treat errors from Throw() as plain goja values This avoids outputting "GoError" as mentioned in https://github.com/loadimpact/k6/issues/877#issuecomment-484396949 --- js/common/bridge_test.go | 36 +++++++++++++------------- js/common/util.go | 2 +- js/common/util_test.go | 4 +-- js/initcontext_test.go | 6 ++--- js/modules/k6/crypto/crypto_test.go | 11 ++++---- js/modules/k6/crypto/x509/x509_test.go | 4 +-- js/modules/k6/http/file_test.go | 5 ++-- js/modules/k6/http/response_test.go | 8 +++--- js/modules/k6/k6_test.go | 6 ++--- js/modules/k6/metrics/metrics_test.go | 2 +- 10 files changed, 41 insertions(+), 43 deletions(-) diff --git a/js/common/bridge_test.go b/js/common/bridge_test.go index 5eff8a0a1..d762169ae 100644 --- a/js/common/bridge_test.go +++ b/js/common/bridge_test.go @@ -348,7 +348,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"Error", bridgeTestErrorType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.error()`) - assert.Contains(t, err.Error(), "GoError: error") + assert.Contains(t, err.Error(), "error") ***REMOVED******REMOVED***, ***REMOVED***"JSValue", bridgeTestJSValueType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** v, err := rt.RunString(`obj.func(1234)`) @@ -358,7 +358,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"JSValueError", bridgeTestJSValueErrorType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.func()`) - assert.Contains(t, err.Error(), "GoError: missing argument") + assert.Contains(t, err.Error(), "missing argument") t.Run("Valid", func(t *testing.T) ***REMOVED*** v, err := rt.RunString(`obj.func(1234)`) @@ -369,7 +369,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"JSValueContext", bridgeTestJSValueContextType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.func()`) - assert.Contains(t, err.Error(), "GoError: func() can only be called from within default()") + assert.Contains(t, err.Error(), "func() can only be called from within default()") t.Run("Context", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() @@ -383,14 +383,14 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"JSValueContextError", bridgeTestJSValueContextErrorType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.func()`) - assert.Contains(t, err.Error(), "GoError: func() can only be called from within default()") + assert.Contains(t, err.Error(), "func() can only be called from within default()") t.Run("Context", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() defer func() ***REMOVED*** *ctxPtr = nil ***REMOVED***() _, err := rt.RunString(`obj.func()`) - assert.Contains(t, err.Error(), "GoError: missing argument") + assert.Contains(t, err.Error(), "missing argument") t.Run("Valid", func(t *testing.T) ***REMOVED*** v, err := rt.RunString(`obj.func(1234)`) @@ -408,7 +408,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"NativeFunctionError", bridgeTestNativeFunctionErrorType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.func()`) - assert.Contains(t, err.Error(), "GoError: missing argument") + assert.Contains(t, err.Error(), "missing argument") t.Run("Valid", func(t *testing.T) ***REMOVED*** v, err := rt.RunString(`obj.func(1234)`) @@ -419,7 +419,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"NativeFunctionContext", bridgeTestNativeFunctionContextType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.func()`) - assert.Contains(t, err.Error(), "GoError: func() can only be called from within default()") + assert.Contains(t, err.Error(), "func() can only be called from within default()") t.Run("Context", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() @@ -433,14 +433,14 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"NativeFunctionContextError", bridgeTestNativeFunctionContextErrorType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.func()`) - assert.Contains(t, err.Error(), "GoError: func() can only be called from within default()") + assert.Contains(t, err.Error(), "func() can only be called from within default()") t.Run("Context", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() defer func() ***REMOVED*** *ctxPtr = nil ***REMOVED***() _, err := rt.RunString(`obj.func()`) - assert.Contains(t, err.Error(), "GoError: missing argument") + assert.Contains(t, err.Error(), "missing argument") t.Run("Valid", func(t *testing.T) ***REMOVED*** v, err := rt.RunString(`obj.func(1234)`) @@ -464,7 +464,7 @@ func TestBind(t *testing.T) ***REMOVED*** t.Run("Negative", func(t *testing.T) ***REMOVED*** _, err := rt.RunString(`obj.addWithError(0, -1)`) - assert.Contains(t, err.Error(), "GoError: answer is negative") + assert.Contains(t, err.Error(), "answer is negative") ***REMOVED***) ***REMOVED******REMOVED***, ***REMOVED***"AddWithError", bridgeTestAddWithErrorType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** @@ -475,12 +475,12 @@ func TestBind(t *testing.T) ***REMOVED*** t.Run("Negative", func(t *testing.T) ***REMOVED*** _, err := rt.RunString(`obj.addWithError(0, -1)`) - assert.Contains(t, err.Error(), "GoError: answer is negative") + assert.Contains(t, err.Error(), "answer is negative") ***REMOVED***) ***REMOVED******REMOVED***, ***REMOVED***"Context", bridgeTestContextType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.context()`) - assert.Contains(t, err.Error(), "GoError: context() can only be called from within default()") + assert.Contains(t, err.Error(), "context() can only be called from within default()") t.Run("Valid", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() @@ -492,7 +492,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"ContextAdd", bridgeTestContextAddType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.contextAdd(1, 2)`) - assert.Contains(t, err.Error(), "GoError: contextAdd() can only be called from within default()") + assert.Contains(t, err.Error(), "contextAdd() can only be called from within default()") t.Run("Valid", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() @@ -506,7 +506,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"ContextAddWithError", bridgeTestContextAddWithErrorType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.contextAddWithError(1, 2)`) - assert.Contains(t, err.Error(), "GoError: contextAddWithError() can only be called from within default()") + assert.Contains(t, err.Error(), "contextAddWithError() can only be called from within default()") t.Run("Valid", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() @@ -519,7 +519,7 @@ func TestBind(t *testing.T) ***REMOVED*** t.Run("Negative", func(t *testing.T) ***REMOVED*** _, err := rt.RunString(`obj.contextAddWithError(0, -1)`) - assert.Contains(t, err.Error(), "GoError: answer is negative") + assert.Contains(t, err.Error(), "answer is negative") ***REMOVED***) ***REMOVED***) ***REMOVED******REMOVED***, @@ -529,7 +529,7 @@ func TestBind(t *testing.T) ***REMOVED*** case bridgeTestContextInjectType: assert.EqualError(t, err, "TypeError: Object has no member 'contextInject' at :1:18(3)") case *bridgeTestContextInjectType: - assert.Contains(t, err.Error(), "GoError: contextInject() can only be called from within default()") + assert.Contains(t, err.Error(), "contextInject() can only be called from within default()") assert.Equal(t, nil, impl.ctx) t.Run("Valid", func(t *testing.T) ***REMOVED*** @@ -588,7 +588,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"SumWithContext", bridgeTestSumWithContextType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.sumWithContext(1, 2)`) - assert.Contains(t, err.Error(), "GoError: sumWithContext() can only be called from within default()") + assert.Contains(t, err.Error(), "sumWithContext() can only be called from within default()") t.Run("Valid", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() @@ -626,7 +626,7 @@ func TestBind(t *testing.T) ***REMOVED*** ***REMOVED******REMOVED***, ***REMOVED***"SumWithContextAndError", bridgeTestSumWithContextAndErrorType***REMOVED******REMOVED***, func(t *testing.T, obj interface***REMOVED******REMOVED***, rt *goja.Runtime) ***REMOVED*** _, err := rt.RunString(`obj.sumWithContextAndError(1, 2)`) - assert.Contains(t, err.Error(), "GoError: sumWithContextAndError() can only be called from within default()") + assert.Contains(t, err.Error(), "sumWithContextAndError() can only be called from within default()") t.Run("Valid", func(t *testing.T) ***REMOVED*** *ctxPtr = context.Background() diff --git a/js/common/util.go b/js/common/util.go index a6696b5da..fd6be5a22 100644 --- a/js/common/util.go +++ b/js/common/util.go @@ -33,7 +33,7 @@ func Throw(rt *goja.Runtime, err error) ***REMOVED*** if e, ok := err.(*goja.Exception); ok ***REMOVED*** panic(e) ***REMOVED*** - panic(rt.NewGoError(err)) + panic(rt.ToValue(err)) ***REMOVED*** // GetReader tries to return an io.Reader value from an exported goja value. diff --git a/js/common/util_test.go b/js/common/util_test.go index c315b40c6..6900d588d 100644 --- a/js/common/util_test.go +++ b/js/common/util_test.go @@ -34,12 +34,12 @@ func TestThrow(t *testing.T) ***REMOVED*** fn1, ok := goja.AssertFunction(rt.ToValue(func() ***REMOVED*** Throw(rt, errors.New("aaaa")) ***REMOVED***)) if assert.True(t, ok, "fn1 is invalid") ***REMOVED*** _, err := fn1(goja.Undefined()) - assert.EqualError(t, err, "GoError: aaaa") + assert.EqualError(t, err, "aaaa") fn2, ok := goja.AssertFunction(rt.ToValue(func() ***REMOVED*** Throw(rt, err) ***REMOVED***)) if assert.True(t, ok, "fn1 is invalid") ***REMOVED*** _, err := fn2(goja.Undefined()) - assert.EqualError(t, err, "GoError: aaaa") + assert.EqualError(t, err, "aaaa") ***REMOVED*** ***REMOVED*** ***REMOVED*** diff --git a/js/initcontext_test.go b/js/initcontext_test.go index fea756751..e11acdb01 100644 --- a/js/initcontext_test.go +++ b/js/initcontext_test.go @@ -53,7 +53,7 @@ func TestInitContextRequire(t *testing.T) ***REMOVED*** t.Run("Nonexistent", func(t *testing.T) ***REMOVED*** t.Parallel() _, err := getSimpleBundle(t, "/script.js", `import "k6/NONEXISTENT";`) - assert.Contains(t, err.Error(), "GoError: unknown module: k6/NONEXISTENT") + assert.Contains(t, err.Error(), "unknown module: k6/NONEXISTENT") ***REMOVED***) t.Run("k6", func(t *testing.T) ***REMOVED*** @@ -318,7 +318,7 @@ func TestInitContextOpen(t *testing.T) ***REMOVED*** t.Parallel() path := filepath.FromSlash("/nonexistent.txt") _, err := getSimpleBundle(t, "/script.js", `open("/nonexistent.txt"); export default function() ***REMOVED******REMOVED***`) - assert.Contains(t, err.Error(), fmt.Sprintf("GoError: open %s: file does not exist", path)) + assert.Contains(t, err.Error(), fmt.Sprintf("open %s: file does not exist", path)) ***REMOVED***) t.Run("Directory", func(t *testing.T) ***REMOVED*** @@ -327,7 +327,7 @@ func TestInitContextOpen(t *testing.T) ***REMOVED*** fs := afero.NewMemMapFs() assert.NoError(t, fs.MkdirAll(path, 0o755)) _, err := getSimpleBundle(t, "/script.js", `open("/some/dir"); export default function() ***REMOVED******REMOVED***`, fs) - assert.Contains(t, err.Error(), fmt.Sprintf("GoError: open() can't be used with directories, path: %q", path)) + assert.Contains(t, err.Error(), fmt.Sprintf("open() can't be used with directories, path: %q", path)) ***REMOVED***) ***REMOVED*** diff --git a/js/modules/k6/crypto/crypto_test.go b/js/modules/k6/crypto/crypto_test.go index 2936c11b3..a3494b798 100644 --- a/js/modules/k6/crypto/crypto_test.go +++ b/js/modules/k6/crypto/crypto_test.go @@ -318,7 +318,7 @@ func TestOutputEncoding(t *testing.T) ***REMOVED*** hasher.update("hello world"); hasher.digest("someInvalidEncoding"); `) - assert.Contains(t, err.Error(), "GoError: Invalid output encoding: someInvalidEncoding") + assert.Contains(t, err.Error(), "Invalid output encoding: someInvalidEncoding") ***REMOVED***) ***REMOVED*** @@ -409,7 +409,7 @@ func TestHMac(t *testing.T) ***REMOVED*** throw new Error("Hex encoding mismatch: " + resultHex); ***REMOVED***`) - assert.Contains(t, err.Error(), "GoError: Invalid algorithm: "+algorithm) + assert.Contains(t, err.Error(), "Invalid algorithm: "+algorithm) ***REMOVED***) t.Run(algorithm+" wrapper: invalid", func(t *testing.T) ***REMOVED*** @@ -419,7 +419,7 @@ func TestHMac(t *testing.T) ***REMOVED*** throw new Error("Hex encoding mismatch: " + resultHex); ***REMOVED***`) - assert.Contains(t, err.Error(), "GoError: Invalid algorithm: "+algorithm) + assert.Contains(t, err.Error(), "Invalid algorithm: "+algorithm) ***REMOVED***) ***REMOVED*** ***REMOVED*** @@ -450,9 +450,8 @@ func TestHexEncodeError(t *testing.T) ***REMOVED*** err := recover() require.NotNil(t, err) require.IsType(t, &goja.Object***REMOVED******REMOVED***, err) - require.IsType(t, map[string]interface***REMOVED******REMOVED******REMOVED******REMOVED***, err.(*goja.Object).Export()) - val := err.(*goja.Object).Export().(map[string]interface***REMOVED******REMOVED***) - assert.Equal(t, expErr, fmt.Sprintf("%s", val["value"])) + val := err.(*goja.Object).Export() + require.EqualError(t, val.(error), expErr) ***REMOVED***() c := New() diff --git a/js/modules/k6/crypto/x509/x509_test.go b/js/modules/k6/crypto/x509/x509_test.go index 31f9ab022..63966b5e8 100644 --- a/js/modules/k6/crypto/x509/x509_test.go +++ b/js/modules/k6/crypto/x509/x509_test.go @@ -150,7 +150,7 @@ func TestParse(t *testing.T) ***REMOVED*** _, err := rt.RunString(` x509.parse("bad-certificate");`) assert.Contains( - t, err.Error(), "GoError: failed to decode certificate PEM file") + t, err.Error(), "failed to decode certificate PEM file") ***REMOVED***) t.Run("ParseFailure", func(t *testing.T) ***REMOVED*** @@ -160,7 +160,7 @@ func TestParse(t *testing.T) ***REMOVED*** if assert.Error(t, err) ***REMOVED*** assert.Contains(t, err.Error(), - "GoError: failed to parse certificate", + "failed to parse certificate", ) ***REMOVED*** ***REMOVED***) diff --git a/js/modules/k6/http/file_test.go b/js/modules/k6/http/file_test.go index 6750f3f36..f1c2e2978 100644 --- a/js/modules/k6/http/file_test.go +++ b/js/modules/k6/http/file_test.go @@ -75,9 +75,8 @@ func TestHTTPFile(t *testing.T) ***REMOVED*** err := recover() require.NotNil(t, err) require.IsType(t, &goja.Object***REMOVED******REMOVED***, err) - require.IsType(t, map[string]interface***REMOVED******REMOVED******REMOVED******REMOVED***, err.(*goja.Object).Export()) - val := err.(*goja.Object).Export().(map[string]interface***REMOVED******REMOVED***) - assert.Equal(t, tc.expErr, fmt.Sprintf("%s", val["value"])) + val := err.(*goja.Object).Export() + require.EqualError(t, val.(error), tc.expErr) ***REMOVED***() ***REMOVED*** h := new(GlobalHTTP).NewModuleInstancePerVU().(*HTTP) diff --git a/js/modules/k6/http/response_test.go b/js/modules/k6/http/response_test.go index 08e9a1b53..ec6bdd484 100644 --- a/js/modules/k6/http/response_test.go +++ b/js/modules/k6/http/response_test.go @@ -197,13 +197,13 @@ func TestResponse(t *testing.T) ***REMOVED*** t.Run("Invalid", func(t *testing.T) ***REMOVED*** _, err := rt.RunString(sr(`http.request("GET", "HTTPBIN_URL/html").json();`)) //nolint:lll - assert.Contains(t, err.Error(), "GoError: cannot parse json due to an error at line 1, character 2 , error: invalid character '<' looking for beginning of value") + assert.Contains(t, err.Error(), "cannot parse json due to an error at line 1, character 2 , error: invalid character '<' looking for beginning of value") ***REMOVED***) t.Run("Invalid", func(t *testing.T) ***REMOVED*** _, err := rt.RunString(sr(`http.request("GET", "HTTPBIN_URL/invalidjson").json();`)) //nolint:lll - assert.Contains(t, err.Error(), "GoError: cannot parse json due to an error at line 3, character 9 , error: invalid character 'e' in literal true (expecting 'r')") + assert.Contains(t, err.Error(), "cannot parse json due to an error at line 3, character 9 , error: invalid character 'e' in literal true (expecting 'r')") ***REMOVED***) ***REMOVED***) t.Run("JsonSelector", func(t *testing.T) ***REMOVED*** @@ -326,7 +326,7 @@ func TestResponse(t *testing.T) ***REMOVED*** if (res.status != 200) ***REMOVED*** throw new Error("wrong status: " + res.status); ***REMOVED*** res.submitForm(***REMOVED*** formSelector: "#doesNotExist" ***REMOVED***) `)) - assert.Contains(t, err.Error(), sr("GoError: no form found for selector '#doesNotExist' in response 'HTTPBIN_URL/forms/post'")) + assert.Contains(t, err.Error(), sr("no form found for selector '#doesNotExist' in response 'HTTPBIN_URL/forms/post'")) ***REMOVED***) t.Run("withGetMethod", func(t *testing.T) ***REMOVED*** @@ -377,7 +377,7 @@ func TestResponse(t *testing.T) ***REMOVED*** if (res.status != 200) ***REMOVED*** throw new Error("wrong status: " + res.status); ***REMOVED*** res = res.clickLink(***REMOVED*** selector: 'a#doesNotExist' ***REMOVED***) `)) - assert.Contains(t, err.Error(), sr("GoError: no element found for selector 'a#doesNotExist' in response 'HTTPBIN_URL/links/10/0'")) + assert.Contains(t, err.Error(), sr("no element found for selector 'a#doesNotExist' in response 'HTTPBIN_URL/links/10/0'")) ***REMOVED***) t.Run("withRequestParams", func(t *testing.T) ***REMOVED*** diff --git a/js/modules/k6/k6_test.go b/js/modules/k6/k6_test.go index ba1dd7f32..e24d7d16e 100644 --- a/js/modules/k6/k6_test.go +++ b/js/modules/k6/k6_test.go @@ -41,7 +41,7 @@ func TestFail(t *testing.T) ***REMOVED*** rt := goja.New() rt.Set("k6", common.Bind(rt, New(), nil)) _, err := rt.RunString(`k6.fail("blah")`) - assert.Contains(t, err.Error(), "GoError: blah") + assert.Contains(t, err.Error(), "blah") ***REMOVED*** func TestSleep(t *testing.T) ***REMOVED*** @@ -132,7 +132,7 @@ func TestGroup(t *testing.T) ***REMOVED*** t.Run("Invalid", func(t *testing.T) ***REMOVED*** _, err := rt.RunString(`k6.group("::", function() ***REMOVED*** throw new Error("nooo") ***REMOVED***)`) - assert.Contains(t, err.Error(), "GoError: group and check names may not contain '::'") + assert.Contains(t, err.Error(), "group and check names may not contain '::'") ***REMOVED***) ***REMOVED*** @@ -212,7 +212,7 @@ func TestCheck(t *testing.T) ***REMOVED*** t.Run("Invalid", func(t *testing.T) ***REMOVED*** _, err := rt.RunString(`k6.check(null, ***REMOVED*** "::": true ***REMOVED***)`) - assert.Contains(t, err.Error(), "GoError: group and check names may not contain '::'") + assert.Contains(t, err.Error(), "group and check names may not contain '::'") ***REMOVED***) ***REMOVED***) diff --git a/js/modules/k6/metrics/metrics_test.go b/js/modules/k6/metrics/metrics_test.go index 01af49272..62d510788 100644 --- a/js/modules/k6/metrics/metrics_test.go +++ b/js/modules/k6/metrics/metrics_test.go @@ -86,7 +86,7 @@ func TestMetrics(t *testing.T) ***REMOVED*** t.Run("ExitInit", func(t *testing.T) ***REMOVED*** *ctxPtr = lib.WithState(*ctxPtr, state) _, err := rt.RunString(fmt.Sprintf(`new metrics.%s("my_metric")`, fn)) - assert.EqualError(t, err, "GoError: metrics must be declared in the init context at apply (native)") + assert.EqualError(t, err, "metrics must be declared in the init context at apply (native)") ***REMOVED***) groups := map[string]*lib.Group***REMOVED***