feat: stabilize rate limiting behaviour

app/web/errors/error_responder.rb

@@ -25,7 +25,7 @@ class << self # rubocop:disable Metrics/ClassLength
         # @param response [Rack::Response]
         # @param error [StandardError]
         # @return [String]
-        # rubocop:disable Metrics/AbcSize, Metrics/MethodLength
+        # rubocop:disable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
         def respond(request:, response:, error:)
           status = resolve_status(error)
           code = resolve_error_code(error)
@@ -34,7 +34,7 @@ def respond(request:, response:, error:)
           if status == 429
             response['Retry-After'] ||= Flags.rate_limit_window_seconds.to_s
           elsif [503, 504].include?(status)
-            response['Retry-After'] = Flags.retry_after_timeout_seconds.to_s
+            response['Retry-After'] ||= Flags.retry_after_timeout_seconds.to_s
           end
 
           emit_error_event(error, code, response.status)
@@ -48,7 +48,7 @@ def respond(request:, response:, error:)
 
           render_xml_error(response, error)
         end
-        # rubocop:enable Metrics/AbcSize, Metrics/MethodLength
+        # rubocop:enable Metrics/AbcSize, Metrics/MethodLength, Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
 
         private
 
@@ -74,9 +74,8 @@ def render_xml_error(response, error)
         def resolve_error_code(error)
           case error
           when ->(e) { extraction_empty_failure?(e) } then EXTRACTION_EMPTY_CODE
-          when TooManyRequestsError then 'TOO_MANY_REQUESTS'
-          when ServiceUnavailableError, ->(e) { server_timeout?(e) } then 'SERVICE_UNAVAILABLE'
-          when GatewayTimeoutError, ->(e) { gateway_timeout?(e) } then 'GATEWAY_TIMEOUT'
+          when ->(e) { server_timeout?(e) } then ServiceUnavailableError::CODE
+          when ->(e) { gateway_timeout?(e) } then GatewayTimeoutError::CODE
           else error.respond_to?(:code) ? error.code : INTERNAL_ERROR_CODE
           end
         end

app/web/request/rate_limiter.rb

@@ -200,8 +200,13 @@ def handle_overflow(now)
           { size: post_prune_size, action: 'prune_to_limit' }
         )
 
-        keys_to_evict = @history.keys.sample(post_prune_size - 10_000)
-        keys_to_evict.each { |k| @history.delete(k) }
+        needed = post_prune_size - 10_000
+        evicted = 0
+        @history.keys.shuffle.each do |key|
+          break if evicted >= needed
+
+          evicted += 1 if @history.delete(key)
+        end
       end
       # rubocop:enable Metrics/MethodLength
 

spec/html2rss/web/error_responder_spec.rb

@@ -150,6 +150,32 @@ def expected_api_error_response # rubocop:disable Metrics/MethodLength
       expect(response.status).to eq(504)
       expect(response['Retry-After']).to eq('300')
     end
+
+    it 'preserves existing Retry-After header for 503 errors', :aggregate_failures do
+      existing_response = Rack::Response.new
+      existing_response['Retry-After'] = '10'
+      response, _body = respond_with(
+        error: Html2rss::Web::ServiceUnavailableError.new('service down'),
+        path: '/api/v1/feeds',
+        target: Html2rss::Web::RequestTarget::API,
+        response: existing_response
+      )
+      expect(response.status).to eq(503)
+      expect(response['Retry-After']).to eq('10')
+    end
+
+    it 'preserves existing Retry-After header for 504 errors', :aggregate_failures do
+      existing_response = Rack::Response.new
+      existing_response['Retry-After'] = '10'
+      response, _body = respond_with(
+        error: Html2rss::Web::GatewayTimeoutError.new('gateway down'),
+        path: '/api/v1/feeds',
+        target: Html2rss::Web::RequestTarget::API,
+        response: existing_response
+      )
+      expect(response.status).to eq(504)
+      expect(response['Retry-After']).to eq('10')
+    end
   end
 
   # @return [Hash{String=>Object}]

spec/html2rss/web/rate_limiter_spec.rb

@@ -108,6 +108,28 @@
       # Should prune down to 10,000 plus the active client request track = 10,001
       expect(history_map.size).to eq(10_001)
     end
+
+    it 'evicts keys randomly on overflow' do
+      history_map = middleware.instance_variable_get(:@history)
+      now = Time.now.to_i
+
+      # Helper to fill history and trigger overflow
+      perform_overflow_eviction = lambda do
+        history_map.clear
+        20_100.times do |i|
+          track = described_class::RequestTrack.new
+          track.instance_variable_set(:@timestamps, [now])
+          history_map["ip-#{i}"] = track
+        end
+        middleware.send(:handle_overflow, now)
+        history_map.keys.sort
+      end
+      # rubocop:disable Naming/VariableNumber
+      remaining_keys_1 = perform_overflow_eviction.call
+      remaining_keys_2 = perform_overflow_eviction.call
+      # rubocop:enable Naming/VariableNumber
+      expect(remaining_keys_1).not_to eq(remaining_keys_2)
+    end
     # rubocop:enable RSpec/ExampleLength
 
     it 'logs rate limit exceeded events to SecurityLogger' do