Commit ea4d04be authored by Max Woolf's avatar Max Woolf
Browse files

refactor(rate_limit): treat variadic callables as zero-arity and document rule_context key contract 

Tighten resolve_value so only arity >= 1 opts into rule_context. Variadic
lambdas (->(*args)) previously fell into the one-arg branch and silently
received [rule_context], so callers' overrides would never reach the
rule's lookup and the fallback default would fire with no error.

Also document on Limiter#check that the rule_context key contract is
owned by the rule's callable - a typo in the caller's hash falls through
to the callable's default branch without warning.

Co-Authored-By: default avatarClaude Opus 4.7 (1M context) <noreply@anthropic.com>
parent 8dce0e70

lib/labkit/rate_limit/evaluator.rb

+9 −4
Original line number Diff line number Diff line
@@ -199,9 +199,14 @@ module Labkit 
      # invoked according to their arity:

      #

      # - Zero-arity callables call with no args (e.g. -> { ApplicationSetting.current.foo }).

      # - One-arity (and variadic) callables receive +rule_context+, which may

      #   be nil if the caller didn't pass it. One-arity callables must

      #   therefore handle nil - typically with `ctx&.[](:key) || default`.

      # - Callables with arity >= 1 receive +rule_context+, which may be nil

      #   if the caller didn't pass it. They must therefore handle nil -

      #   typically with `ctx&.[](:key) || default`.

      # - Variadic callables (negative arity, e.g. ->(*args) { ... }) take

      #   the zero-arg path. Opt into rule_context by writing the lambda

      #   with exactly one required parameter: ->(ctx) { ... }. This avoids

      #   the footgun where ->(*args) silently receives [rule_context] and

      #   the caller's overrides never take effect.

      # - Callables that respond to +call+ but not +arity+ (e.g. a class with

      #   `def call` and no explicit arity) take the zero-arg path, preserving

      #   the pre-rule_context behaviour for custom callable objects.
@@ -212,7 +217,7 @@ module Labkit 
      def resolve_value(val, rule_context = nil)

        return val unless val.respond_to?(:call)



        val.respond_to?(:arity) && !val.arity.zero? ? val.call(rule_context) : val.call

        val.respond_to?(:arity) && val.arity >= 1 ? val.call(rule_context) : val.call

      end



      def encode_char_value(value)

lib/labkit/rate_limit/limiter.rb

+4 −0
Original line number Diff line number Diff line
@@ -39,6 +39,10 @@ module Labkit 
      #   one-arity callables on +limit+/+period+. Lets rules resolve dynamic

      #   configuration (e.g. per-namespace settings) without rebuilding the

      #   Rule or doing out-of-band DB queries. Zero-arity callables ignore it.

      #   The key contract is owned by the rule's callable, not validated

      #   here: if the rule reads ctx[:limit] and the caller passes

      #   ctx[:lmit], the callable's fallback branch fires silently. Keep

      #   the rule definition and the call site colocated.

      # @return [Result]

      def check(identifier, cost: 1, rule_context: nil)

        id = identifier.is_a?(Identifier) ? identifier : Identifier.new(identifier)

spec/labkit/rate_limit/evaluator_spec.rb

+5 −2
Original line number Diff line number Diff line
@@ -288,7 +288,10 @@ RSpec.describe Labkit::RateLimit::Evaluator do 
      expect(received).to eq({ source: "peek" })

    end



    it "treats a variadic callable as receiving rule_context (single arg passed)" do

    it "treats a variadic callable as zero-arity (rule_context not passed)" do

      # Variadic lambdas take the zero-arg path so ->(*args) doesn't silently

      # receive [rule_context] and ignore the caller's overrides. Opt in to

      # rule_context with exactly one required parameter: ->(ctx) { ... }.

      received = :unset

      rule = make_rule(name: "variadic", limit: lambda { |*args|

        received = args
@@ -297,7 +300,7 @@ RSpec.describe Labkit::RateLimit::Evaluator do 


      evaluator(rules: [rule]).check(identifier, rule_context: { limit: 7 })



      expect(received).to eq([{ limit: 7 }])

      expect(received).to eq([])

    end



    it "invokes a custom callable without #arity with no args (preserves legacy behaviour)" do