From f5afe49766facfc1fdebdc2e592dc1884d0de480 Mon Sep 17 00:00:00 2001
From: "Matthew M. Keeler" <mkeeler@launchdarkly.com>
Date: Thu, 21 Mar 2024 10:46:14 -0400
Subject: [PATCH 1/4] feat: Add initial support for hooks (#256)

---
 lib/ldclient-rb/config.rb     |  13 +++
 lib/ldclient-rb/interfaces.rb |  85 +++++++++++++++++++
 lib/ldclient-rb/ldclient.rb   | 131 ++++++++++++++++++++++++++++-
 spec/ldclient_hooks_spec.rb   | 152 ++++++++++++++++++++++++++++++++++
 spec/mock_components.rb       |  21 +++++
 5 files changed, 400 insertions(+), 2 deletions(-)
 create mode 100644 spec/ldclient_hooks_spec.rb

diff --git a/lib/ldclient-rb/config.rb b/lib/ldclient-rb/config.rb
index 7c2642be..09347e20 100644
--- a/lib/ldclient-rb/config.rb
+++ b/lib/ldclient-rb/config.rb
@@ -43,6 +43,7 @@ class Config
     # @option opts [BigSegmentsConfig] :big_segments See {#big_segments}.
     # @option opts [Hash] :application See {#application}
     # @option opts [String] :payload_filter_key See {#payload_filter_key}
+    # @option hooks [Array<Interfaces::Hooks::Hook]
     #
     def initialize(opts = {})
       @base_uri = (opts[:base_uri] || Config.default_base_uri).chomp("/")
@@ -75,6 +76,7 @@ def initialize(opts = {})
       @big_segments = opts[:big_segments] || BigSegmentsConfig.new(store: nil)
       @application = LaunchDarkly::Impl::Util.validate_application_info(opts[:application] || {}, @logger)
       @payload_filter_key = opts[:payload_filter_key]
+      @hooks = (opts[:hooks] || []).keep_if { |hook| hook.is_a? Interfaces::Hooks::Hook }
       @data_source_update_sink = nil
     end
 
@@ -372,6 +374,17 @@ def diagnostic_opt_out?
     #
     attr_reader :socket_factory
 
+    #
+    # Initial set of hooks for the client.
+    #
+    # Hooks provide entrypoints which allow for observation of SDK functions.
+    #
+    # LaunchDarkly provides integration packages, and most applications will not
+    # need to implement their own hooks. Refer to the `launchdarkly-server-sdk-otel` gem
+    # for instrumentation.
+    #
+    attr_reader :hooks
+
     #
     # The default LaunchDarkly client configuration. This configuration sets
     # reasonable defaults for most users.
diff --git a/lib/ldclient-rb/interfaces.rb b/lib/ldclient-rb/interfaces.rb
index d42f924e..c76683a9 100644
--- a/lib/ldclient-rb/interfaces.rb
+++ b/lib/ldclient-rb/interfaces.rb
@@ -885,5 +885,90 @@ def build
         end
       end
     end
+
+    module Hooks
+      #
+      # Mixin for extending SDK functionality via hooks.
+      #
+      # All provided hook implementations **MUST** include this mixin. Hooks without this mixin will be ignored.
+      #
+      # This mixin includes default implementations for all hook handlers. This allows LaunchDarkly to expand the list
+      # of hook handlers without breaking customer integrations.
+      #
+      module Hook
+        #
+        # Get metadata about the hook implementation.
+        #
+        # @return [Metadata]
+        #
+        def metadata
+          Metadata.new('UNDEFINED')
+        end
+
+        #
+        # The before method is called during the execution of a variation method before the flag value has been
+        # determined. The method is executed synchronously.
+        #
+        # @param evaluation_series_context [EvaluationSeriesContext] Contains information about the evaluation being
+        # performed. This is not mutable.
+        # @param data [Hash] A record associated with each stage of hook invocations. Each stage is called with the data
+        # of the previous stage for a series. The input record should not be modified.
+        # @return [Hash] Data to use when executing the next state of the hook in the evaluation series.
+        #
+        def before_evaluation(evaluation_series_context, data)
+          {}
+        end
+
+        #
+        # The after method is called during the execution of the variation method # after the flag value has been
+        # determined. The method is executed synchronously.
+        #
+        # @param evaluation_series_context [EvaluationSeriesContext] Contains read-only information about the evaluation
+        # being performed.
+        # @param data [Hash] A record associated with each stage of hook invocations. Each stage is called with the data
+        # of the previous stage for a series.
+        # @param detail [LaunchDarkly::EvaluationDetail] The result of the evaluation. This value should not be
+        # modified.
+        # @return [Hash] Data to use when executing the next state of the hook in the evaluation series.
+        #
+        def after_evaluation(evaluation_series_context, data, detail)
+          data
+        end
+      end
+
+      #
+      # Metadata data class used for annotating hook implementations.
+      #
+      class Metadata
+        attr_reader :name
+
+        def initialize(name)
+          @name = name
+        end
+      end
+
+      #
+      # Contextual information that will be provided to handlers during evaluation series.
+      #
+      class EvaluationSeriesContext
+        attr_reader :key
+        attr_reader :context
+        attr_reader :value
+        attr_reader :method
+
+        #
+        # @param key [String]
+        # @param context [LaunchDarkly::LDContext]
+        # @param value [any]
+        # @param method [Symbol]
+        #
+        def initialize(key, context, value, method)
+          @key = key
+          @context = context
+          @value = value
+          @method = method
+        end
+      end
+    end
   end
 end
diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb
index b97549cf..927ef1f5 100644
--- a/lib/ldclient-rb/ldclient.rb
+++ b/lib/ldclient-rb/ldclient.rb
@@ -7,6 +7,7 @@
 require "ldclient-rb/impl/flag_tracker"
 require "ldclient-rb/impl/store_client_wrapper"
 require "ldclient-rb/impl/migrations/tracker"
+require "concurrent"
 require "concurrent/atomics"
 require "digest/sha1"
 require "forwardable"
@@ -54,6 +55,7 @@ def initialize(sdk_key, config = Config.default, wait_for_sec = 5)
       end
 
       @sdk_key = sdk_key
+      @hooks = Concurrent::Array.new(config.hooks)
 
       @shared_executor = Concurrent::SingleThreadExecutor.new
 
@@ -131,6 +133,23 @@ def initialize(sdk_key, config = Config.default, wait_for_sec = 5)
       end
     end
 
+    #
+    # Add a hook to the client. In order to register a hook before the client starts, please use the `hooks` property of
+    # {#LDConfig}.
+    #
+    # Hooks provide entrypoints which allow for observation of SDK functions.
+    #
+    # @param hook [Interfaces::Hooks::Hook]
+    #
+    def add_hook(hook)
+      unless hook.is_a?(Interfaces::Hooks::Hook)
+        @config.logger.error { "[LDClient] Attempted to add a hook that does not include the LaunchDarkly::Intefaces::Hooks::Hook mixin. Ignoring." }
+        return
+      end
+
+      @hooks.push(hook)
+    end
+
     #
     # Tells the client that all pending analytics events should be delivered as soon as possible.
     #
@@ -226,10 +245,116 @@ def variation(key, context, default)
     # @return [EvaluationDetail] an object describing the result
     #
     def variation_detail(key, context, default)
-      detail, _, _ = evaluate_internal(key, context, default, true)
+      detail, _, _ = evaluate_with_hooks(key, context, default, :variation_detail) do
+        evaluate_internal(key, context, default, true)
+      end
+
       detail
     end
 
+    #
+    # evaluate_with_hook will run the provided block, wrapping it with evaluation hook support.
+    #
+    # Example:
+    #
+    # ```ruby
+    # evaluate_with_hooks(key, context, default, method) do
+    #   puts 'This is being wrapped with evaluation hooks'
+    # end
+    # ```
+    #
+    # @param key [String]
+    # @param context [String]
+    # @param default [String]
+    # @param method [Symbol]
+    # @param &block [#call] Implicit passed block
+    #
+    private def evaluate_with_hooks(key, context, default, method)
+      return yield if @hooks.empty?
+
+      hooks, evaluation_series_context = prepare_hooks(key, context, default, method)
+      hook_data = execute_before_evaluation(hooks, evaluation_series_context)
+      evaluation_detail, flag, error = yield
+      execute_after_evaluation(hooks, evaluation_series_context, hook_data, evaluation_detail)
+
+      [evaluation_detail, flag, error]
+    end
+
+    #
+    # Execute the :before_evaluation stage of the evaluation series.
+    #
+    # This method will return the results of each hook, indexed into an array in the same order as the hooks. If a hook
+    # raised an uncaught exception, the value will be nil.
+    #
+    # @param hooks [Array<Interfaces::Hooks::Hook>]
+    # @param evaluation_series_context [EvaluationSeriesContext]
+    #
+    # @return [Array<any>]
+    #
+    private def execute_before_evaluation(hooks, evaluation_series_context)
+      hooks.map do |hook|
+        try_execute_stage(:before_evaluation, hook.metadata.name) do
+          hook.before_evaluation(evaluation_series_context, {})
+        end
+      end
+    end
+
+    #
+    # Execute the :after_evaluation stage of the evaluation series.
+    #
+    # This method will return the results of each hook, indexed into an array in the same order as the hooks. If a hook
+    # raised an uncaught exception, the value will be nil.
+    #
+    # @param hooks [Array<Interfaces::Hooks::Hook>]
+    # @param evaluation_series_context [EvaluationSeriesContext]
+    # @param hook_data [Array<any>]
+    # @param evaluation_detail [EvaluationDetail]
+    #
+    # @return [Array<any>]
+    #
+    private def execute_after_evaluation(hooks, evaluation_series_context, hook_data, evaluation_detail)
+      hooks.zip(hook_data).reverse.map do |(hook, data)|
+        try_execute_stage(:after_evaluation, hook.metadata.name) do
+          hook.after_evaluation(evaluation_series_context, data, evaluation_detail)
+        end
+      end
+    end
+
+    #
+    # Try to execute the provided block. If execution raises an exception, catch and log it, then move on with
+    # execution.
+    #
+    # @return [any]
+    #
+    private def try_execute_stage(method, hook_name)
+      begin
+        yield
+      rescue => e
+        @config.logger.error { "[LDClient] An error occurred in #{method} of the hook #{hook_name}: #{e}" }
+        nil
+      end
+    end
+
+    #
+    # Return a copy of the existing hooks and a few instance of the EvaluationSeriesContext used for the evaluation series.
+    #
+    # @param key [String]
+    # @param context [LDContext]
+    # @param default [any]
+    # @param method [Symbol]
+    # @return [Array[Array<Interfaces::Hooks::Hook>, Interfaces::Hooks::EvaluationSeriesContext]]
+    #
+    private def prepare_hooks(key, context, default, method)
+      # Copy the hooks to use a consistent set during the evaluation series.
+      #
+      # Hooks can be added and we want to ensure all correct stages for a given hook execute. For example, we do not
+      # want to trigger the after_evaluation method without also triggering the before_evaluation method.
+      hooks = @hooks.dup
+      evaluation_series_context = Interfaces::Hooks::EvaluationSeriesContext.new(key, context, default, method)
+
+      [hooks, evaluation_series_context]
+    end
+
     #
     # This method returns the migration stage of the migration feature flag for the given evaluation context.
     #
@@ -508,7 +633,9 @@ def create_default_data_source(sdk_key, config, diagnostic_accumulator)
     # @return [Array<EvaluationDetail, [LaunchDarkly::Impl::Model::FeatureFlag, nil], [String, nil]>]
     #
     def variation_with_flag(key, context, default)
-      evaluate_internal(key, context, default, false)
+      evaluate_with_hooks(key, context, default, :variation_detail) do
+        evaluate_internal(key, context, default, false)
+      end
     end
 
     #
diff --git a/spec/ldclient_hooks_spec.rb b/spec/ldclient_hooks_spec.rb
new file mode 100644
index 00000000..887bd829
--- /dev/null
+++ b/spec/ldclient_hooks_spec.rb
@@ -0,0 +1,152 @@
+require "ldclient-rb"
+
+require "mock_components"
+require "model_builders"
+require "spec_helper"
+
+module LaunchDarkly
+  describe "LDClient hooks tests" do
+    context "registration" do
+      it "can register a hook on the config" do
+        count = 0
+        hook = MockHook.new(->(_, _) { count += 1 }, ->(_, _, _) { count += 2 })
+        with_client(test_config(hooks: [hook])) do |client|
+          client.variation("doesntmatter", basic_context, "default")
+          expect(count).to eq 3
+        end
+      end
+
+      it "can register a hook on the client" do
+        count = 0
+        hook = MockHook.new(->(_, _) { count += 1 }, ->(_, _, _) { count += 2 })
+        with_client(test_config()) do |client|
+          client.add_hook(hook)
+          client.variation("doesntmatter", basic_context, "default")
+
+          expect(count).to eq 3
+        end
+      end
+
+      it "can register hooks on both" do
+        count = 0
+        config_hook = MockHook.new(->(_, _) { count += 1 }, ->(_, _, _) { count += 2 })
+        client_hook = MockHook.new(->(_, _) { count += 4 }, ->(_, _, _) { count += 8 })
+
+        with_client(test_config(hooks: [config_hook])) do |client|
+          client.add_hook(client_hook)
+          client.variation("doesntmatter", basic_context, "default")
+
+          expect(count).to eq 15
+        end
+      end
+
+      it "will drop invalid hooks on config" do
+        config = test_config(hooks: [true, nil, "example thing"])
+        expect(config.hooks.count).to eq 0
+      end
+
+      it "will drop invalid hooks on client" do
+        with_client(test_config) do |client|
+          client.add_hook(true)
+          client.add_hook(nil)
+          client.add_hook("example thing")
+
+          expect(client.instance_variable_get("@hooks").count).to eq 0
+        end
+
+        config = test_config(hooks: [true, nil, "example thing"])
+        expect(config.hooks.count).to eq 0
+      end
+    end
+
+    context "execution order" do
+      it "config order is preserved" do
+        order = []
+        first_hook = MockHook.new(->(_, _) { order << "first before" }, ->(_, _, _) { order << "first after" })
+        second_hook = MockHook.new(->(_, _) { order << "second before" }, ->(_, _, _) { order << "second after" })
+
+        with_client(test_config(hooks: [first_hook, second_hook])) do |client|
+          client.variation("doesntmatter", basic_context, "default")
+          expect(order).to eq ["first before", "second before", "second after", "first after"]
+        end
+      end
+
+      it "client order is preserved" do
+        order = []
+        first_hook = MockHook.new(->(_, _) { order << "first before" }, ->(_, _, _) { order << "first after" })
+        second_hook = MockHook.new(->(_, _) { order << "second before" }, ->(_, _, _) { order << "second after" })
+
+        with_client(test_config()) do |client|
+          client.add_hook(first_hook)
+          client.add_hook(second_hook)
+          client.variation("doesntmatter", basic_context, "default")
+
+          expect(order).to eq ["first before", "second before", "second after", "first after"]
+        end
+      end
+
+      it "config hooks precede client hooks" do
+        order = []
+        config_hook = MockHook.new(->(_, _) { order << "config before" }, ->(_, _, _) { order << "config after" })
+        client_hook = MockHook.new(->(_, _) { order << "client before" }, ->(_, _, _) { order << "client after" })
+
+        with_client(test_config(hooks: [config_hook])) do |client|
+          client.add_hook(client_hook)
+          client.variation("doesntmatter", basic_context, "default")
+
+          expect(order).to eq ["config before", "client before", "client after", "config after"]
+        end
+      end
+    end
+
+    context "passing data" do
+      it "hook receives EvaluationDetail" do
+        td = Integrations::TestData.data_source
+        td.update(td.flag("flagkey").variations("value").variation_for_all(0))
+
+        detail = nil
+        config_hook = MockHook.new(->(_, _) { }, ->(_, _, d) { detail = d })
+        with_client(test_config(data_source: td, hooks: [config_hook])) do |client|
+          client.variation("flagkey", basic_context, "default")
+
+          expect(detail.value).to eq "value"
+          expect(detail.variation_index).to eq 0
+          expect(detail.reason).to eq EvaluationReason::fallthrough
+        end
+      end
+
+      it "from before evaluation to after evaluation" do
+        actual = nil
+        config_hook = MockHook.new(->(_, _) { "example string returned" }, ->(_, hook_data, _) { actual = hook_data })
+        with_client(test_config(hooks: [config_hook])) do |client|
+          client.variation("doesntmatter", basic_context, "default")
+
+          expect(actual).to eq "example string returned"
+        end
+      end
+
+      it "exception receives nil value" do
+        actual = nil
+        config_hook = MockHook.new(->(_, _) { raise "example string returned" }, ->(_, hook_data, _) { actual = hook_data })
+        with_client(test_config(hooks: [config_hook])) do |client|
+          client.variation("doesntmatter", basic_context, "default")
+
+          expect(actual).to be_nil
+        end
+      end
+
+      it "exceptions do not mess up data passing order" do
+        data = []
+        first_hook = MockHook.new(->(_, _) { "first hook" }, ->(_, hook_data, _) { data << hook_data })
+        second_hook = MockHook.new(->(_, _) { raise "second hook" }, ->(_, hook_data, _) { data << hook_data })
+        third_hook = MockHook.new(->(_, _) { "third hook" }, ->(_, hook_data, _) { data << hook_data })
+        with_client(test_config(hooks: [first_hook, second_hook, third_hook])) do |client|
+          client.variation("doesntmatter", basic_context, "default")
+
+          # NOTE: These are reversed since the push happens in the after_evaluation (when hooks are reversed)
+          expect(data).to eq ["third hook", nil, "first hook"]
+        end
+      end
+    end
+  end
+end
diff --git a/spec/mock_components.rb b/spec/mock_components.rb
index 3866b9f3..4eb0e7a8 100644
--- a/spec/mock_components.rb
+++ b/spec/mock_components.rb
@@ -97,4 +97,25 @@ def self.adding_to_queue(q)
       new(->(value) { q << value })
     end
   end
+
+  class MockHook
+    include Interfaces::Hooks::Hook
+
+    def initialize(before_evaluation, after_evaluation)
+      @before_evaluation = before_evaluation
+      @after_evaluation = after_evaluation
+    end
+
+    def metadata
+      Interfaces::Hooks::Metadata.new("mock hook")
+    end
+
+    def before_evaluation(evaluation_series_context, data)
+      @before_evaluation.call(evaluation_series_context, data)
+    end
+
+    def after_evaluation(evaluation_series_context, data, detail)
+      @after_evaluation.call(evaluation_series_context, data, detail)
+    end
+  end
 end

From f3abac0c6b91e8a7b6fe748ca62e36825488c6fe Mon Sep 17 00:00:00 2001
From: "Matthew M. Keeler" <mkeeler@launchdarkly.com>
Date: Thu, 28 Mar 2024 14:49:01 -0400
Subject: [PATCH 2/4] chore: Add hook support to contract tests (#257)

---
 contract-tests/client_entity.rb |  7 ++++
 contract-tests/hook.rb          | 68 +++++++++++++++++++++++++++++++++
 contract-tests/service.rb       |  1 +
 lib/ldclient-rb/interfaces.rb   | 10 ++---
 lib/ldclient-rb/ldclient.rb     |  9 +++--
 5 files changed, 86 insertions(+), 9 deletions(-)
 create mode 100644 contract-tests/hook.rb

diff --git a/contract-tests/client_entity.rb b/contract-tests/client_entity.rb
index 871c6b2b..dc932691 100644
--- a/contract-tests/client_entity.rb
+++ b/contract-tests/client_entity.rb
@@ -3,6 +3,7 @@
 require 'net/http'
 require 'launchdarkly-server-sdk'
 require './big_segment_store_fixture'
+require './hook'
 require 'http'
 
 class ClientEntity
@@ -62,6 +63,12 @@ def initialize(log, config)
       }
     end
 
+    if config[:hooks]
+      opts[:hooks] = config[:hooks][:hooks].map do |hook|
+        Hook.new(hook[:name], hook[:callbackUri], hook[:data] || {})
+      end
+    end
+
     startWaitTimeMs = config[:startWaitTimeMs] || 5_000
 
     @client = LaunchDarkly::LDClient.new(
diff --git a/contract-tests/hook.rb b/contract-tests/hook.rb
new file mode 100644
index 00000000..89690ce9
--- /dev/null
+++ b/contract-tests/hook.rb
@@ -0,0 +1,68 @@
+require 'ldclient-rb'
+
+class Hook
+  include LaunchDarkly::Interfaces::Hooks::Hook
+
+  #
+  # @param name [String]
+  # @param callback_uri [String]
+  # @param data [Hash]
+  #
+  def initialize(name, callback_uri, data)
+    @metadata = LaunchDarkly::Interfaces::Hooks::Metadata.new(name)
+    @callback_uri = callback_uri
+    @data = data
+    @context_filter = LaunchDarkly::Impl::ContextFilter.new(false, [])
+  end
+
+  def metadata
+    @metadata
+  end
+
+  #
+  # @param evaluation_series_context [LaunchDarkly::Interfaces::Hooks::EvaluationSeriesContext]
+  # @param data [Hash]
+  #
+  def before_evaluation(evaluation_series_context, data)
+    payload = {
+      evaluationSeriesContext: {
+        flagKey: evaluation_series_context.key,
+        context: @context_filter.filter(evaluation_series_context.context),
+        defaultValue: evaluation_series_context.default_value,
+        method: evaluation_series_context.method,
+      },
+      evaluationSeriesData: data,
+      stage: 'beforeEvaluation',
+    }
+    result = HTTP.post(@callback_uri, json: payload)
+
+    (data || {}).merge(@data[:beforeEvaluation] || {})
+  end
+
+
+  #
+  # @param evaluation_series_context [LaunchDarkly::Interfaces::Hooks::EvaluationSeriesContext]
+  # @param data [Hash]
+  # @param detail [LaunchDarkly::EvaluationDetail]
+  #
+  def after_evaluation(evaluation_series_context, data, detail)
+    payload = {
+      evaluationSeriesContext: {
+        flagKey: evaluation_series_context.key,
+        context: @context_filter.filter(evaluation_series_context.context),
+        defaultValue: evaluation_series_context.default_value,
+        method: evaluation_series_context.method,
+      },
+      evaluationSeriesData: data,
+      evaluationDetail: {
+        value: detail.value,
+        variationIndex: detail.variation_index,
+        reason: detail.reason,
+      },
+      stage: 'afterEvaluation',
+    }
+    HTTP.post(@callback_uri, json: payload)
+
+    (data || {}).merge(@data[:afterEvaluation] || {})
+  end
+end
diff --git a/contract-tests/service.rb b/contract-tests/service.rb
index 10d66e16..5f252970 100644
--- a/contract-tests/service.rb
+++ b/contract-tests/service.rb
@@ -39,6 +39,7 @@
       'polling-gzip',
       'inline-context',
       'anonymous-redaction',
+      'evaluation-hooks',
     ],
   }.to_json
 end
diff --git a/lib/ldclient-rb/interfaces.rb b/lib/ldclient-rb/interfaces.rb
index c76683a9..956a5f6a 100644
--- a/lib/ldclient-rb/interfaces.rb
+++ b/lib/ldclient-rb/interfaces.rb
@@ -916,7 +916,7 @@ def metadata
         # @return [Hash] Data to use when executing the next state of the hook in the evaluation series.
         #
         def before_evaluation(evaluation_series_context, data)
-          {}
+          data
         end
 
         #
@@ -953,19 +953,19 @@ def initialize(name)
       class EvaluationSeriesContext
         attr_reader :key
         attr_reader :context
-        attr_reader :value
+        attr_reader :default_value
         attr_reader :method
 
         #
         # @param key [String]
         # @param context [LaunchDarkly::LDContext]
-        # @param value [any]
+        # @param default_value [any]
         # @param method [Symbol]
         #
-        def initialize(key, context, value, method)
+        def initialize(key, context, default_value, method)
           @key = key
           @context = context
-          @value = value
+          @default_value = default_value
           @method = method
         end
       end
diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb
index 927ef1f5..57e0c52b 100644
--- a/lib/ldclient-rb/ldclient.rb
+++ b/lib/ldclient-rb/ldclient.rb
@@ -245,6 +245,7 @@ def variation(key, context, default)
     # @return [EvaluationDetail] an object describing the result
     #
     def variation_detail(key, context, default)
+      context = Impl::Context::make_context(context)
       detail, _, _ = evaluate_with_hooks(key, context, default, :variation_detail) do
         evaluate_internal(key, context, default, true)
       end
@@ -264,8 +265,8 @@ def variation_detail(key, context, default)
     # ```
     #
     # @param key [String]
-    # @param context [String]
-    # @param default [String]
+    # @param context [LDContext]
+    # @param default [any]
     # @param method [Symbol]
     # @param &block [#call] Implicit passed block
     #
@@ -633,6 +634,7 @@ def create_default_data_source(sdk_key, config, diagnostic_accumulator)
     # @return [Array<EvaluationDetail, [LaunchDarkly::Impl::Model::FeatureFlag, nil], [String, nil]>]
     #
     def variation_with_flag(key, context, default)
+      context = Impl::Context::make_context(context)
       evaluate_with_hooks(key, context, default, :variation_detail) do
         evaluate_internal(key, context, default, false)
       end
@@ -640,7 +642,7 @@ def variation_with_flag(key, context, default)
 
     #
     # @param key [String]
-    # @param context [Hash, LDContext]
+    # @param context [LDContext]
     # @param default [Object]
     # @param with_reasons [Boolean]
     #
@@ -657,7 +659,6 @@ def evaluate_internal(key, context, default, with_reasons)
         return detail, nil, "no context provided"
       end
 
-      context = Impl::Context::make_context(context)
       unless context.valid?
         @config.logger.error { "[LDClient] Context was invalid for evaluation of flag '#{key}' (#{context.error}); returning default value" }
         detail = Evaluator.error_result(EvaluationReason::ERROR_USER_NOT_SPECIFIED, default)

From 80aae24edffa9a179c8195f8316f79010d9f3a53 Mon Sep 17 00:00:00 2001
From: "Matthew M. Keeler" <mkeeler@launchdarkly.com>
Date: Tue, 2 Apr 2024 13:07:31 -0400
Subject: [PATCH 3/4] fix: Adjust migration variation and hook interaction
 (#264)

---
 .../impl/evaluation_with_hook_result.rb       | 34 ++++++++++++
 lib/ldclient-rb/interfaces.rb                 |  2 +-
 lib/ldclient-rb/ldclient.rb                   | 54 +++++++++++--------
 spec/ldclient_hooks_spec.rb                   | 47 ++++++++++++++++
 4 files changed, 114 insertions(+), 23 deletions(-)
 create mode 100644 lib/ldclient-rb/impl/evaluation_with_hook_result.rb

diff --git a/lib/ldclient-rb/impl/evaluation_with_hook_result.rb b/lib/ldclient-rb/impl/evaluation_with_hook_result.rb
new file mode 100644
index 00000000..b092bc06
--- /dev/null
+++ b/lib/ldclient-rb/impl/evaluation_with_hook_result.rb
@@ -0,0 +1,34 @@
+module LaunchDarkly
+  module Impl
+    #
+    # Simple helper class for returning formatted data.
+    #
+    # The variation methods make use of the new hook support. Those methods all need to return an evaluation detail, and
+    # some other unstructured bit of data.
+    #
+    class EvaluationWithHookResult
+      #
+      # Return the evaluation detail that was generated as part of the evaluation.
+      #
+      # @return [LaunchDarkly::EvaluationDetail]
+      #
+      attr_reader :evaluation_detail
+
+      #
+      # All purpose container for additional return values from the wrapping method
+      #
+      # @return [any]
+      #
+      attr_reader :results
+
+      #
+      # @param evaluation_detail [LaunchDarkly::EvaluationDetail]
+      # @param results [any]
+      #
+      def initialize(evaluation_detail, results = nil)
+        @evaluation_detail = evaluation_detail
+        @results = results
+      end
+    end
+  end
+end
diff --git a/lib/ldclient-rb/interfaces.rb b/lib/ldclient-rb/interfaces.rb
index 956a5f6a..3aa6fe68 100644
--- a/lib/ldclient-rb/interfaces.rb
+++ b/lib/ldclient-rb/interfaces.rb
@@ -920,7 +920,7 @@ def before_evaluation(evaluation_series_context, data)
         end
 
         #
-        # The after method is called during the execution of the variation method # after the flag value has been
+        # The after method is called during the execution of the variation method after the flag value has been
         # determined. The method is executed synchronously.
         #
         # @param evaluation_series_context [EvaluationSeriesContext] Contains read-only information about the evaluation
diff --git a/lib/ldclient-rb/ldclient.rb b/lib/ldclient-rb/ldclient.rb
index 57e0c52b..f3bb311a 100644
--- a/lib/ldclient-rb/ldclient.rb
+++ b/lib/ldclient-rb/ldclient.rb
@@ -4,6 +4,7 @@
 require "ldclient-rb/impl/data_store"
 require "ldclient-rb/impl/diagnostic_events"
 require "ldclient-rb/impl/evaluator"
+require "ldclient-rb/impl/evaluation_with_hook_result"
 require "ldclient-rb/impl/flag_tracker"
 require "ldclient-rb/impl/store_client_wrapper"
 require "ldclient-rb/impl/migrations/tracker"
@@ -217,8 +218,13 @@ def initialized?
     # @return the variation for the provided context, or the default value if there's an error
     #
     def variation(key, context, default)
-      detail, _, _, = variation_with_flag(key, context, default)
-      detail.value
+      context = Impl::Context::make_context(context)
+      result = evaluate_with_hooks(key, context, default, :variation) do
+        detail, _, _ = variation_with_flag(key, context, default)
+        LaunchDarkly::Impl::EvaluationWithHookResult.new(detail)
+      end
+
+      result.evaluation_detail.value
     end
 
     #
@@ -246,11 +252,12 @@ def variation(key, context, default)
     #
     def variation_detail(key, context, default)
       context = Impl::Context::make_context(context)
-      detail, _, _ = evaluate_with_hooks(key, context, default, :variation_detail) do
-        evaluate_internal(key, context, default, true)
+      result = evaluate_with_hooks(key, context, default, :variation_detail) do
+        detail, _, _ = evaluate_internal(key, context, default, true)
+        LaunchDarkly::Impl::EvaluationWithHookResult.new(detail)
       end
 
-      detail
+      result.evaluation_detail
     end
 
     #
@@ -270,15 +277,17 @@ def variation_detail(key, context, default)
     # @param method [Symbol]
     # @param &block [#call] Implicit passed block
     #
+    # @return [LaunchDarkly::Impl::EvaluationWithHookResult]
+    #
     private def evaluate_with_hooks(key, context, default, method)
       return yield if @hooks.empty?
 
       hooks, evaluation_series_context = prepare_hooks(key, context, default, method)
       hook_data = execute_before_evaluation(hooks, evaluation_series_context)
-      evaluation_detail, flag, error = yield
-      execute_after_evaluation(hooks, evaluation_series_context, hook_data, evaluation_detail)
+      evaluation_result = yield
+      execute_after_evaluation(hooks, evaluation_series_context, hook_data, evaluation_result.evaluation_detail)
 
-      [evaluation_detail, flag, error]
+      evaluation_result
     end
 
     #
@@ -375,20 +384,24 @@ def migration_variation(key, context, default_stage)
       end
 
       context = Impl::Context::make_context(context)
-      detail, flag, _ = variation_with_flag(key, context, default_stage.to_s)
+      result = evaluate_with_hooks(key, context, default_stage, :migration_variation) do
+        detail, flag, _ = variation_with_flag(key, context, default_stage.to_s)
+
+        stage = detail.value
+        stage = stage.to_sym if stage.respond_to? :to_sym
 
-      stage = detail.value
-      stage = stage.to_sym if stage.respond_to? :to_sym
+        if Migrations::VALID_STAGES.include?(stage)
+          tracker = Impl::Migrations::OpTracker.new(@config.logger, key, flag, context, detail, default_stage)
+          next LaunchDarkly::Impl::EvaluationWithHookResult.new(detail, {stage: stage, tracker: tracker})
+        end
 
-      if Migrations::VALID_STAGES.include?(stage)
+        detail = LaunchDarkly::Impl::Evaluator.error_result(LaunchDarkly::EvaluationReason::ERROR_WRONG_TYPE, default_stage.to_s)
         tracker = Impl::Migrations::OpTracker.new(@config.logger, key, flag, context, detail, default_stage)
-        return stage, tracker
-      end
 
-      detail = LaunchDarkly::Impl::Evaluator.error_result(LaunchDarkly::EvaluationReason::ERROR_WRONG_TYPE, default_stage.to_s)
-      tracker = Impl::Migrations::OpTracker.new(@config.logger, key, flag, context, detail, default_stage)
+        LaunchDarkly::Impl::EvaluationWithHookResult.new(detail, {stage: default_stage, tracker: tracker})
+      end
 
-      [default_stage, tracker]
+      [result.results[:stage], result.results[:tracker]]
     end
 
     #
@@ -628,16 +641,13 @@ def create_default_data_source(sdk_key, config, diagnostic_accumulator)
 
     #
     # @param key [String]
-    # @param context [Hash, LDContext]
+    # @param context [LDContext]
     # @param default [Object]
     #
     # @return [Array<EvaluationDetail, [LaunchDarkly::Impl::Model::FeatureFlag, nil], [String, nil]>]
     #
     def variation_with_flag(key, context, default)
-      context = Impl::Context::make_context(context)
-      evaluate_with_hooks(key, context, default, :variation_detail) do
-        evaluate_internal(key, context, default, false)
-      end
+      evaluate_internal(key, context, default, false)
     end
 
     #
diff --git a/spec/ldclient_hooks_spec.rb b/spec/ldclient_hooks_spec.rb
index 887bd829..4c403981 100644
--- a/spec/ldclient_hooks_spec.rb
+++ b/spec/ldclient_hooks_spec.rb
@@ -148,5 +148,52 @@ module LaunchDarkly
         end
       end
     end
+
+    context "migration variation" do
+      it "EvaluationDetail contains stage value" do
+        td = Integrations::TestData.data_source
+        td.update(td.flag("flagkey").variations("off").variation_for_all(0))
+
+        detail = nil
+        config_hook = MockHook.new(->(_, _) { }, ->(_, _, d) { detail = d })
+        with_client(test_config(data_source: td, hooks: [config_hook])) do |client|
+          client.migration_variation("flagkey", basic_context, LaunchDarkly::Migrations::STAGE_LIVE)
+
+          expect(detail.value).to eq LaunchDarkly::Migrations::STAGE_OFF.to_s
+          expect(detail.variation_index).to eq 0
+          expect(detail.reason).to eq EvaluationReason::fallthrough
+        end
+      end
+
+      it "EvaluationDetail gets default if flag doesn't evaluate to stage" do
+        td = Integrations::TestData.data_source
+        td.update(td.flag("flagkey").variations("nonstage").variation_for_all(0))
+
+        detail = nil
+        config_hook = MockHook.new(->(_, _) { }, ->(_, _, d) { detail = d })
+        with_client(test_config(data_source: td, hooks: [config_hook])) do |client|
+          client.migration_variation("flagkey", basic_context, LaunchDarkly::Migrations::STAGE_LIVE)
+
+          expect(detail.value).to eq LaunchDarkly::Migrations::STAGE_LIVE.to_s
+          expect(detail.variation_index).to eq nil
+          expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_WRONG_TYPE)
+        end
+      end
+
+      it "EvaluationDetail default gets converted to off if invalid" do
+        td = Integrations::TestData.data_source
+        td.update(td.flag("flagkey").variations("nonstage").variation_for_all(0))
+
+        detail = nil
+        config_hook = MockHook.new(->(_, _) { }, ->(_, _, d) { detail = d })
+        with_client(test_config(data_source: td, hooks: [config_hook])) do |client|
+          client.migration_variation("flagkey", basic_context, :invalid)
+
+          expect(detail.value).to eq LaunchDarkly::Migrations::STAGE_OFF.to_s
+          expect(detail.variation_index).to eq nil
+          expect(detail.reason).to eq EvaluationReason.error(EvaluationReason::ERROR_WRONG_TYPE)
+        end
+      end
+    end
   end
 end

From 5b0600cf98dc26f67db7748833e6929e9840200d Mon Sep 17 00:00:00 2001
From: "Matthew M. Keeler" <mkeeler@launchdarkly.com>
Date: Wed, 3 Apr 2024 13:23:53 -0400
Subject: [PATCH 4/4] chore: Update contract tests to support new hook errors
 field (#266)

---
 .github/workflows/ci.yml        | 4 ++--
 contract-tests/client_entity.rb | 2 +-
 contract-tests/hook.rb          | 8 +++++++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 846ee44a..c23038f1 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -1,11 +1,11 @@
 name: Run CI
 on:
   push:
-    branches: [ main ]
+    branches: [ main, 'feat/**' ]
     paths-ignore:
       - '**.md' # Do not need to run CI for markdown changes.
   pull_request:
-    branches: [ main ]
+    branches: [ main, 'feat/**' ]
     paths-ignore:
       - '**.md'
 
diff --git a/contract-tests/client_entity.rb b/contract-tests/client_entity.rb
index dc932691..faab8c9d 100644
--- a/contract-tests/client_entity.rb
+++ b/contract-tests/client_entity.rb
@@ -65,7 +65,7 @@ def initialize(log, config)
 
     if config[:hooks]
       opts[:hooks] = config[:hooks][:hooks].map do |hook|
-        Hook.new(hook[:name], hook[:callbackUri], hook[:data] || {})
+        Hook.new(hook[:name], hook[:callbackUri], hook[:data] || {}, hook[:errors] || {})
       end
     end
 
diff --git a/contract-tests/hook.rb b/contract-tests/hook.rb
index 89690ce9..0c9eb221 100644
--- a/contract-tests/hook.rb
+++ b/contract-tests/hook.rb
@@ -7,11 +7,13 @@ class Hook
   # @param name [String]
   # @param callback_uri [String]
   # @param data [Hash]
+  # @parm errors [Hash]
   #
-  def initialize(name, callback_uri, data)
+  def initialize(name, callback_uri, data, errors)
     @metadata = LaunchDarkly::Interfaces::Hooks::Metadata.new(name)
     @callback_uri = callback_uri
     @data = data
+    @errors = errors
     @context_filter = LaunchDarkly::Impl::ContextFilter.new(false, [])
   end
 
@@ -24,6 +26,8 @@ def metadata
   # @param data [Hash]
   #
   def before_evaluation(evaluation_series_context, data)
+    raise @errors[:beforeEvaluation] if @errors.include? :beforeEvaluation
+
     payload = {
       evaluationSeriesContext: {
         flagKey: evaluation_series_context.key,
@@ -46,6 +50,8 @@ def before_evaluation(evaluation_series_context, data)
   # @param detail [LaunchDarkly::EvaluationDetail]
   #
   def after_evaluation(evaluation_series_context, data, detail)
+    raise @errors[:afterEvaluation] if @errors.include? :afterEvaluation
+
     payload = {
       evaluationSeriesContext: {
         flagKey: evaluation_series_context.key,