Fix: IntensityTracker properly returns error when limit is exceeded #55
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary: The IntensityTracker does not short-circuit due to a logic bug in recursion.
Expected behavior: If I set a limit of 5 restarts per minute, I expect six restarts to throw the error
TooIntense
Actual behavior: The error does not throw, no matter how many times the supervisor restarts a child.
Explanation:
On the first event sent to the intensity_tracker:
now == event
So the subsequent logical check into recursion:
now >= event + period
always evaluated
False
given the default restart period. The event was always stripped from internal state, so the intensity_tracker never held more than one event at a time. The code in question:otp/src/gleam/otp/intensity_tracker.gleam
Lines 26 to 35 in 45deedf
This patch also includes tests for expected behavior. When testing, I discovered event maximum limit check was off by one, so this patch corrects and tests that edge case.
This was one of those pull-your-hair-out kind of bugs 🐛 Happy to make any changes!