Skip to content

Commit

Permalink
Fix ws.Set(Timeout/Interval) panicking on float values (#1608)
Browse files Browse the repository at this point in the history
The issue is connected to dop251/goja#190 in the
sense that instead of the float to int conversation to result in either
error or a clamped float value, it results in a 0 int value which panics
the stdlib time package.

Changing the parameter for the timeout/interval to float both makes it
possible to take values such as 0.5, meaning half a ms, and also checks
if the value is positive so we can be certain that it won't panic k6.
If not this results in an exception in the script.

Co-authored-by: Ivan Mirić <ivan@loadimpact.com>
  • Loading branch information
mstoykov and Ivan Mirić committed Aug 28, 2020
1 parent c68a4eb commit df3ad1b
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 5 deletions.
23 changes: 20 additions & 3 deletions js/modules/k6/ws/ws.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import (
"context"
"crypto/tls"
"errors"
"fmt"
"io/ioutil"
"net"
"net/http"
Expand Down Expand Up @@ -380,12 +381,18 @@ func (s *Socket) trackPong(pingID string) {
})
}

func (s *Socket) SetTimeout(fn goja.Callable, timeoutMs int) {
// SetTimeout executes the provided function inside the socket's event loop after at least the provided
// timeout, which is in ms, has elapsed
func (s *Socket) SetTimeout(fn goja.Callable, timeoutMs float64) error {
// Starts a goroutine, blocks once on the timeout and pushes the callable
// back to the main loop through the scheduled channel
d := time.Duration(timeoutMs * float64(time.Millisecond))
if d <= 0 {
return fmt.Errorf("setTimeout requires a >0 timeout parameter, received %.2f", timeoutMs)
}
go func() {
select {
case <-time.After(time.Duration(timeoutMs) * time.Millisecond):
case <-time.After(d):
select {
case s.scheduled <- fn:
case <-s.done:
Expand All @@ -396,11 +403,19 @@ func (s *Socket) SetTimeout(fn goja.Callable, timeoutMs int) {
return
}
}()

return nil
}

func (s *Socket) SetInterval(fn goja.Callable, intervalMs int) {
// SetInterval executes the provided function inside the socket's event loop each interval time, which is
// in ms
func (s *Socket) SetInterval(fn goja.Callable, intervalMs float64) error {
// Starts a goroutine, blocks forever on the ticker and pushes the callable
// back to the main loop through the scheduled channel
d := time.Duration(intervalMs * float64(time.Millisecond))
if d <= 0 {
return fmt.Errorf("setInterval requires a >0 timeout parameter, received %.2f", intervalMs)
}
go func() {
ticker := time.NewTicker(time.Duration(intervalMs) * time.Millisecond)
defer ticker.Stop()
Expand All @@ -419,6 +434,8 @@ func (s *Socket) SetInterval(fn goja.Callable, intervalMs int) {
}
}
}()

return nil
}

func (s *Socket) Close(args ...goja.Value) {
Expand Down
33 changes: 31 additions & 2 deletions js/modules/k6/ws/ws_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import (
"github.com/dop251/goja"
"github.com/gorilla/websocket"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"github.com/loadimpact/k6/js/common"
"github.com/loadimpact/k6/lib"
Expand Down Expand Up @@ -87,7 +88,7 @@ func assertMetricEmitted(t *testing.T, metric *stats.Metric, sampleContainers []
}

func TestSession(t *testing.T) {
//TODO: split and paralelize tests
// TODO: split and paralelize tests
t.Parallel()
tb := httpmultibin.NewHTTPMultiBin(t)
defer tb.Cleanup()
Expand Down Expand Up @@ -193,6 +194,19 @@ func TestSession(t *testing.T) {
assert.NoError(t, err)
})
assertSessionMetricsEmitted(t, stats.GetBufferedSamples(samples), "", sr("WSBIN_URL/ws-echo"), 101, "")
t.Run("bad interval", func(t *testing.T) {
_, err := common.RunString(rt, sr(`
var counter = 0;
var res = ws.connect("WSBIN_URL/ws-echo", function(socket){
socket.setInterval(function () {
counter += 1;
if (counter > 2) { socket.close(); }
}, -1.23);
});
`))
require.Error(t, err)
require.Contains(t, err.Error(), "setInterval requires a >0 timeout parameter, received -1.23 ")
})

t.Run("timeout", func(t *testing.T) {
_, err := common.RunString(rt, sr(`
Expand All @@ -210,6 +224,21 @@ func TestSession(t *testing.T) {
`))
assert.NoError(t, err)
})

t.Run("bad timeout", func(t *testing.T) {
_, err := common.RunString(rt, sr(`
var start = new Date().getTime();
var ellapsed = new Date().getTime() - start;
var res = ws.connect("WSBIN_URL/ws-echo", function(socket){
socket.setTimeout(function () {
ellapsed = new Date().getTime() - start;
socket.close();
}, 0);
});
`))
require.Error(t, err)
require.Contains(t, err.Error(), "setTimeout requires a >0 timeout parameter, received 0.00 ")
})
assertSessionMetricsEmitted(t, stats.GetBufferedSamples(samples), "", sr("WSBIN_URL/ws-echo"), 101, "")

t.Run("ping", func(t *testing.T) {
Expand Down Expand Up @@ -438,7 +467,7 @@ func TestSystemTags(t *testing.T) {
rt := goja.New()
rt.SetFieldNameMapper(common.FieldNameMapper{})

//TODO: test for actual tag values after removing the dependency on the
// TODO: test for actual tag values after removing the dependency on the
// external service demos.kaazing.com (https://github.com/loadimpact/k6/issues/537)
testedSystemTags := []string{"group", "status", "subproto", "url", "ip"}

Expand Down

0 comments on commit df3ad1b

Please sign in to comment.