Skip to content

Commit

Permalink
Treat errors from Throw() as plain goja values
Browse files Browse the repository at this point in the history
This avoids outputting "GoError" as mentioned in
grafana/k6#877 (comment)
  • Loading branch information
Ivan Mirić authored and imiric committed Jun 8, 2021
1 parent 5ff125e commit 6402926
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 43 deletions.
36 changes: 18 additions & 18 deletions js/common/bridge_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)`)
Expand All @@ -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)`)
Expand All @@ -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()
Expand All @@ -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)`)
Expand All @@ -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)`)
Expand All @@ -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()
Expand All @@ -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)`)
Expand All @@ -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***
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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***,
Expand All @@ -529,7 +529,7 @@ func TestBind(t *testing.T) ***REMOVED***
case bridgeTestContextInjectType:
assert.EqualError(t, err, "TypeError: Object has no member 'contextInject' at <eval>: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***
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 1 addition & 1 deletion js/common/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
4 changes: 2 additions & 2 deletions js/common/util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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***
Expand Down
6 changes: 3 additions & 3 deletions js/initcontext_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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***
Expand Down Expand Up @@ -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***
Expand All @@ -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***

Expand Down
11 changes: 5 additions & 6 deletions js/modules/k6/crypto/crypto_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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***

Expand Down Expand Up @@ -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***
Expand All @@ -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***
Expand Down Expand Up @@ -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()
Expand Down
4 changes: 2 additions & 2 deletions js/modules/k6/crypto/x509/x509_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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***
Expand All @@ -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***)
Expand Down
5 changes: 2 additions & 3 deletions js/modules/k6/http/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
8 changes: 4 additions & 4 deletions js/modules/k6/http/response_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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***
Expand Down Expand Up @@ -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***
Expand Down Expand Up @@ -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***
Expand Down
6 changes: 3 additions & 3 deletions js/modules/k6/k6_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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***
Expand Down Expand Up @@ -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***

Expand Down Expand Up @@ -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***)

Expand Down
2 changes: 1 addition & 1 deletion js/modules/k6/metrics/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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***
Expand Down

0 comments on commit 6402926

Please sign in to comment.