TT-6075: Update rate limit implementation #7941
Merged
probelabs / Visor: architecture
failed
Apr 20, 2026 in 1m 45s
🚨 Check Failed
architecture check failed because fail_if condition was met.
Details
📊 Summary
- Total Issues: 5
- Error Issues: 4
- Warning Issues: 1
🔍 Failure Condition Results
Failed Conditions
- global_fail_if: output.issues && output.issues.some(i => i.severity === 'critical' || i.severity === 'error')
- Severity: ❌ error
Issues by Category
Architecture (4)
- ❌ gateway/mw_redis_cache.go:249 - When
rate_limit_response_headersis set torate_limits, middleware that generates its own response (like the cache, mock, and virtual endpoint middleware) incorrectly clears the rate-limit headers that were set earlier in the middleware chain. TheRateLimitAndQuotaCheckmiddleware correctly sets the headers on theResponseWriter, but this middleware then generates a new response object, callslimitHeaderFactory(...).SendQuotas(), and copies the new headers over. Inrate_limitsmode, therateLimitSender.SendQuotasmethod ensures no rate-limit headers are present on the new response object, effectively wiping out the correct headers when the copy happens. This breaks the feature for cached, mocked, or virtual responses. - ❌ gateway/mw_mock_response.go:173 - This middleware suffers from the same architectural flaw as the caching middleware. When a mock response is served in
rate_limitsmode, this line effectively causes the correct rate-limit headers (set byRateLimitAndQuotaCheck) to be cleared from the final response. - ❌ gateway/mw_virtual_endpoint.go:334 - This middleware suffers from the same architectural flaw as the caching middleware. When a virtual endpoint response is served in
rate_limitsmode, this line effectively causes the correct rate-limit headers (set byRateLimitAndQuotaCheck) to be cleared from the final response. ⚠️ gateway/mw_api_rate_limit.go:104 - The logic to prevent API-level rate limit headers from overwriting per-key headers relies on checking for the presence of theX-RateLimit-Limitheader. This creates a fragile, implicit dependency on the execution order and side effects of theRateLimitAndQuotaCheckmiddleware. A change in that middleware's header-setting behavior could break this logic. This pattern makes the interaction between middleware components less explicit and harder to reason about.
Logic (1)
- ❌ system:0 - Global failure condition met: output.issues && output.issues.some(i => i.severity === 'critical' || i.severity === 'error')
Powered by Visor from Probelabs
💡 TIP: You can chat with Visor using /visor ask <your question>
Annotations
Check failure on line 249 in gateway/mw_redis_cache.go
probelabs / Visor: architecture
architecture Issue
When `rate_limit_response_headers` is set to `rate_limits`, middleware that generates its own response (like the cache, mock, and virtual endpoint middleware) incorrectly clears the rate-limit headers that were set earlier in the middleware chain. The `RateLimitAndQuotaCheck` middleware correctly sets the headers on the `ResponseWriter`, but this middleware then generates a new response object, calls `limitHeaderFactory(...).SendQuotas()`, and copies the new headers over. In `rate_limits` mode, the `rateLimitSender.SendQuotas` method ensures no rate-limit headers are present on the new response object, effectively wiping out the correct headers when the copy happens. This breaks the feature for cached, mocked, or virtual responses.
Raw output
The middleware that generates a response should not unconditionally call `SendQuotas`. One solution is to avoid this call when in `rate_limits` mode, thereby preserving the headers already set on the `ResponseWriter`. Alternatively, the headers from the `ResponseWriter` could be merged into the generated response's headers before returning.
Check failure on line 173 in gateway/mw_mock_response.go
probelabs / Visor: architecture
architecture Issue
This middleware suffers from the same architectural flaw as the caching middleware. When a mock response is served in `rate_limits` mode, this line effectively causes the correct rate-limit headers (set by `RateLimitAndQuotaCheck`) to be cleared from the final response.
Raw output
Conditionalize the call to `SendQuotas` to only run when not in `rate_limits` mode. This will preserve the headers set by the preceding rate-limiting middleware.
Check failure on line 334 in gateway/mw_virtual_endpoint.go
probelabs / Visor: architecture
architecture Issue
This middleware suffers from the same architectural flaw as the caching middleware. When a virtual endpoint response is served in `rate_limits` mode, this line effectively causes the correct rate-limit headers (set by `RateLimitAndQuotaCheck`) to be cleared from the final response.
Raw output
Conditionalize the call to `SendQuotas` to only run when not in `rate_limits` mode. This will preserve the headers set by the preceding rate-limiting middleware.
Check warning on line 104 in gateway/mw_api_rate_limit.go
probelabs / Visor: architecture
architecture Issue
The logic to prevent API-level rate limit headers from overwriting per-key headers relies on checking for the presence of the `X-RateLimit-Limit` header. This creates a fragile, implicit dependency on the execution order and side effects of the `RateLimitAndQuotaCheck` middleware. A change in that middleware's header-setting behavior could break this logic. This pattern makes the interaction between middleware components less explicit and harder to reason about.
Raw output
Use a request context value to explicitly signal that rate limit headers have already been handled by a more specific limiter. For example, the `RateLimitAndQuotaCheck` middleware could set a flag in the context, and this middleware would check for that flag instead of inspecting the headers. This makes the contract between the middleware components explicit.
Loading