Token bucket denies a request that exactly matches remaining tokens
Nobody has started this yet — be first.
Business impact
Token bucket backs the pro-tier plan and any small-capacity policy (e.g. "1 password reset email per key at a time"). Losing the last unit of every configured burst means paying customers systematically get less throughput than the plan they bought -- worse for small capacities, where "capacity 1" effectively means "always denied." It shows up as support tickets from customers who have made zero real requests, and is easy to ship unnoticed because manual testing usually checks "does it deny eventually," not "does it admit exactly N."
Problem
AllowN is supposed to allow bursts up to capacity, but the request that would exactly exhaust the bucket is rejected instead of admitted -- an off-by-one in the token-sufficiency comparison.
Current behavior
A fresh bucket of capacity 3 denies the 3rd immediate request (before any refill), even though the bucket started with exactly 3 tokens and this is only the 3rd call.
Expected behavior
A request is allowed whenever the bucket holds at least as many tokens as requested. The boundary case -- tokens remaining exactly equals tokens requested -- must be allowed, not denied; only a request that needs strictly more tokens than are available should be denied.
Steps to reproduce
tb := ratelimit.NewTokenBucketMemory(3, 0 /* no refill */, time.Minute)- Call
tb.Allow(ctx, "user-1")three times in a row. - Observe: the 3rd call returns
Allowed: false,RetryAfter: 0. - Expected: all three calls return
Allowed: true, and only a 4th call is denied.
Why this matters
Any caller relying on "capacity N means N requests get through" silently loses one unit of burst allowance. For capacity 1, used to gate a single in-flight operation per key, the feature never actually admits anything.
Suggested approach
One comparison operator inside AllowN's token-sufficiency check needs to flip. Re-derive, from the doc comment above the function and the README's "allows bursts up to a capacity" description, what the boundary condition (tokens == n) should do. Do not touch the refill math or the Remaining/ResetAt calculations -- only the admission check is wrong.
Acceptance criteria
- A fresh bucket of capacity N admits exactly N immediate requests (no refill) before denying the (N+1)th
- go test ./pkg/ratelimit/... -run TokenBucket passes, including the pre-existing TestTokenBucketMemory_AllowsBurstUpToCapacity
- go test ./test/ -run TestTask01 -v passes
Verification
go test ./test/ -run TestTask01 -v
Hints (0/2)
Try it without hints first — the reading is the exercise.
Working on this ticket
Work on a branch named for the ticket — that's what you'll submit.
Branch off your fork
$git checkout -b fix/token-bucket-off-by-oneFix it and commit
Meet every acceptance criterion, and add a test that would have caught this.
Push the branch
$git push -u origin fix/token-bucket-off-by-oneSubmit it below
Paste your fork URL and the branch name, with a short write-up of the root cause.
Questions
Stuck on something?
Ask about anything unclear in the ticket — the maintainer and anyone who has solved it can answer. Please don't post full solutions.
Sign in to ask a question or reply.
Sign in