Skip to content
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 non zero exit status #741

Closed
wants to merge 2 commits into from
Closed

Fix non zero exit status #741

wants to merge 2 commits into from

Conversation

luizbafilho
Copy link
Contributor

When an error happened while executing the script, k6 wasn't returning a non 0 zero exit status, that is not ideal when setting up k6 in CI environment.

Another wrong behavior was that even if the script had an error k6 would not stop the execution and would run the script repeatedly until the duration or the number of iterations were complete.

@codecov-io
Copy link

Codecov Report

Merging #741 into master will increase coverage by 0.02%.
The diff coverage is 71.42%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #741      +/-   ##
==========================================
+ Coverage   64.14%   64.17%   +0.02%     
==========================================
  Files         101      101              
  Lines        8381     8393      +12     
==========================================
+ Hits         5376     5386      +10     
- Misses       2650     2652       +2     
  Partials      355      355
Impacted Files Coverage Δ
cmd/run.go 6.87% <0%> (-0.11%) ⬇️
core/local/local.go 79.13% <100%> (+1.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d4152d2...036c01c. Read the comment docs.

@na--
Copy link
Member

na-- commented Aug 14, 2018

I'm not sure if the error code fix is necessary anymore, see my reasoning in my comment in the original issue.

Regarding the runtime errors that basically busy-loop the script when you run it with a duration (because each iteration finished very quickly), I'm not sure what the best solution would be. I had a discussion in another issue about it and how it can be managed if users use try/catch to handle exceptions. But they probably have to be burned at least once before they do it, so ideally we can somehow lessen the impact by default.

My ideas here revolve around adding some small sleep period for each VU after it encounters a runtime errors, let's say 1 second by default, but modifiable with a new JS option like sleepAfterError or something. It's not a very elegant, but at least it would prevent busy-looping runs when every iterations exits with an error early. Alternatively, as I mention in the issue I linked above, we can have a minIterationDuration option and prevent iterations from being shorter than it, but this would affect other things as well.

Alternatively, albeit a much more complicated approach, I saw that we have an errors metric, but I don't think we currently use it, not sure why (@robingustafsson, any ideas?). It's a bit more involved, but if we turn it to a Rate metric instead of a Counter and actually start using it, we can implement an error threshold inside of k6, somewhat similar to what I did in JS here. And we can have a default check that fails the test if errors are above a certain percentage of the iterations, let's say 50% by default, but users would be able to modify it. This would both guard against busy-looping scripts and would also serve as a nice default CI check without requiring users to do anything complicated like the example I gave in the issue.

Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above comment for reasons

@na--
Copy link
Member

na-- commented Sep 18, 2018

Closing this in favor of a future pull request that, as described here, adds some sort of default threshold (with abortOnFail) that's triggered if the percentage of failed iterations exceeds some set amount, for example 50% by default.

And somewhat connected, but we might also add the sleepAfterError setting I described above, since there are some benefits of pausing a bit after an error, even if we can prevent the current script busy-looping with the new error threshold.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants