-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix ws.Set(Timeout/Interval) panicking on float values #1608
Conversation
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.
Codecov Report
@@ Coverage Diff @@
## master #1608 +/- ##
==========================================
+ Coverage 74.82% 74.89% +0.07%
==========================================
Files 164 164
Lines 14035 14043 +8
==========================================
+ Hits 10501 10518 +17
+ Misses 3002 2993 -9
Partials 532 532
Continue to review full report at Codecov.
|
Added #1305 (comment) as a reminder to use floats everywhere, when we unify the duration part of the configuration mess |
For the record, I didn't find another place where we use |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 though I'm not sure sub-ms values would be useful. :)
See how Node handles timers:
When delay is larger than 2147483647 or less than 1, the delay will be set to 1. Non-integer delays are truncated to an integer.
Co-authored-by: Ivan Mirić <ivan@loadimpact.com>
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.