Code Review of a Go Token Bucket Rate Limiter Used as HTTP Middleware

In a code review round you are given a pull request and asked to review it as you would for a teammate: find what is wrong, decide what blocks the merge, and explain how you would fix and test it. The reported exercise was a token bucket rate limiter written in Go. Its code is not reproduced in the report, so practice with the stand-in below, written for this purpose. It is meant to be used as HTTP middleware in a Go web service, giving each client a burst of up to capacity requests that refills at refillPerSec tokens per second.

package ratelimit

import (
	"net/http"
	"time"
)

// Limiter gives each client a burst of capacity requests,
// refilled at refillPerSec tokens per second.
type Limiter struct {
	capacity int
	refillPerSec int
	buckets map[string]*bucket
}

type bucket struct {
	tokens int
	lastRefill time.Time
}

func NewLimiter(capacity, refillPerSec int) *Limiter {
	return &Limiter{capacity: capacity, refillPerSec: refillPerSec}
}

// Allow reports whether one more request from key may proceed.
func (l *Limiter) Allow(key string) bool {
	b, ok := l.buckets[key]
	if !ok {
		b = &bucket{tokens: l.capacity, lastRefill: time.Now()}
		l.buckets[key] = b
	}

	now := time.Now()
	elapsed := now.Sub(b.lastRefill)
	b.tokens += int(elapsed.Seconds()) * l.refillPerSec
	b.lastRefill = now

	if b.tokens > 0 {
		b.tokens--
		return true
	}
	return false
}

// Middleware rejects requests from clients that are over their limit.
func (l *Limiter) Middleware(next http.Handler) http.Handler {
	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
		if !l.Allow(r.RemoteAddr) {
			w.WriteHeader(http.StatusTooManyRequests)
		}
		next.ServeHTTP(w, r)
	})
}

Constraints and Clarifications

• Assume Go 1.21 or later and the standard library only.
• The middleware wraps handlers of a net/http server.
• Review the code as submitted; you may propose any change to it, including to its exported API.

Clarifying Questions

• Should the limit apply per client address, per authenticated user, or per API key?
• Is the limit meant per server instance or across all replicas of the service?
• Does traffic reach the service directly from clients, or through a load balancer or proxy?
• Roughly how many distinct clients does the service see, and can some of them be hostile?

Part 1 — Review the code

Find the defects, classify each as blocking or non-blocking with a reason, and write the review comments you would leave, ending with a verdict.

What This Part Should Cover

• Behavior at runtime once the server handles real traffic
• Whether the token arithmetic honors the limiter's contract under realistic call patterns
• Middleware control flow and the choice of client key
• Resource growth, configuration safety, and a clear verdict

Part 2 — Fix and test it

Write or describe the corrected limiter and middleware, then the tests you would require before approving.

What This Part Should Cover

• A corrected limiter and middleware that resolve every blocking finding
• Correct behavior under production traffic and long uptime
• Tests that would fail on the original code and control time instead of sleeping
• The limits of a per-instance limiter