[Feldt]

What was wrong

The rate limiter rejected every request with 429 when its burst ended up at 0. That happened in two ways:

• Go embedders. Config.MaxRequestBurst is documented as "Set to 0 to use default (2x MaxRequestRate)", and NewConfig() leaves it at 0. But NewRateLimitedHandler passed the 0 straight to the token bucket. A bucket with capacity 0 never holds a token, so a Config that sets only MaxRequestRate turned away all traffic.
• CLI and YAML, low rates. When the burst is omitted (-1), SetRateLimits set it to int(maxRate * 2). For any rate below 0.5 req/s (for example --max-request-rate=0.25) that truncates to 0, with the same result.

What changed

• New helper defaultRequestBurst(rate) in rate_limiter.go. It returns 2x the rate, and at least 1, for a positive rate. For a rate of 0 it returns 0, as before.
• NewRateLimitedHandler uses the helper when MaxRequestBurst <= 0, so the documented default applies.
• SetRateLimits uses the same helper instead of int(maxRate * 2).

Rates of 0.5 req/s and above get exactly the same burst as before. Explicit bursts are unchanged, and so is the existing check that an explicit burst must be greater than the rate.

How it was tested

Two new tests in pkg/smokescreen/rate_limiter_test.go:

• TestRateLimitedHandler_DefaultBurst: MaxRequestRate: 5 with the burst unset lets 10 requests through and limits the 11th.
• TestRateLimitedHandler_DefaultBurstLowRate: SetRateLimits(0, 0.25, DefaultMaxRequestBurst) gives a burst of 1. The first request passes and the second is limited.

Both fail on master (bc3bf31): every request gets 429, and the burst is 0. Both pass with this change.

GOFLAGS=-mod=vendor go test ./pkg/smokescreen/ -run 'TestRateLimit' -v -count=1

The rest of the CI checks were run locally by the agent (Go 1.27.2, and Go 1.27.1 for staticcheck), and all passed: gofmt -l, go mod verify, go vet ./..., go test ./..., go test -race ./..., go test -race -tags=smokescreen_no_prometheus ./..., go test -race -tags=integration ./cmd/..., and staticcheck v0.8.1 with the three tag sets from the lint job. The Go 1.26 matrix leg was not run locally.

The v2 branch has the same code in both places. Happy to open a port there if that would help.

AI disclosure

This change was prepared with an AI coding agent (Claude Code), including the tests and the local runs above. I am responsible for it and will answer review comments.

🤖 Generated with Claude Code

[cla-assistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[gauthamw-stripe]

I think we can remove this comment as well now since this logic is housed elsewhere.

Suggested change

// Apply default: 2x rate when not explicitly configured or configured negative

[gauthamw-stripe]

Suggested change

	// maxRequestBurst: < 0 = use default rate based on maxRate, >=0 = must be greater than maxRate.

[gauthamw-stripe]

Thanks for contributing this! Minor edits suggested for comments but otherwise looks good.

Can you sign the CLA as well when you update this and then I can merge it? :)

[Feldt]

Thanks for the review! Applied both suggestions in the latest commit; go test ./pkg/smokescreen/ passes locally. I've flagged the CLA to the account owner, who signs it.