From 2d2ea3cda2237ebb409b8ffe62b0baab528831d3 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Thu, 30 Apr 2026 11:14:49 -0400 Subject: [PATCH 1/4] fix: Honor x-ld-fd-fallback header in fdv2 initializer phase Prior to this change, the Ruby SDK only inspected the x-ld-fd-fallback response header on FDv2 synchronizer responses. If an initializer received the header, the signal was silently dropped and the SDK would continue attempting subsequent initializers and FDv2 synchronizers rather than reverting to FDv1. The Initializer fetch contract now returns a FetchResult that pairs the existing Result with a fallback_to_fdv1 boolean. The FDv2 data system branches on the new flag, applying any accompanying Basis before swapping the synchronizer list for the FDv1 fallback builder, so evaluations can serve the server-provided payload while FDv1 spins up. When no FDv1 fallback is configured, the data system logs and clears the synchronizer list, mirroring the synchronizer-triggered path. Update.revert_to_fdv1 is renamed to Update.fallback_to_fdv1 with an alias retained for backwards compatibility while FDv2 is in early access. A shared LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested? helper replaces the duplicated header-string checks across the polling and streaming data sources. --- lib/ldclient-rb/impl/data_system.rb | 15 +- lib/ldclient-rb/impl/data_system/fdv2.rb | 72 ++++++++-- lib/ldclient-rb/impl/data_system/polling.rb | 74 +++++++--- lib/ldclient-rb/impl/data_system/streaming.rb | 11 +- .../impl/integrations/file_data_source_v2.rb | 51 ++++--- .../test_data/test_data_source_v2.rb | 101 ++++++------- lib/ldclient-rb/interfaces/data_system.rb | 75 +++++++++- spec/impl/data_system/fdv2_datasystem_spec.rb | 135 ++++++++++++++++-- .../data_system/polling_initializer_spec.rb | 53 +++++++ .../data_system/polling_synchronizer_spec.rb | 28 ++-- .../data_system/streaming_headers_spec.rb | 4 +- .../streaming_synchronizer_spec.rb | 8 +- spec/impl/datasystem_spec.rb | 6 +- 13 files changed, 481 insertions(+), 152 deletions(-) diff --git a/lib/ldclient-rb/impl/data_system.rb b/lib/ldclient-rb/impl/data_system.rb index 53f434a1..40100731 100644 --- a/lib/ldclient-rb/impl/data_system.rb +++ b/lib/ldclient-rb/impl/data_system.rb @@ -247,8 +247,9 @@ class Update # @return [LaunchDarkly::Interfaces::DataSource::ErrorInfo, nil] Error information if applicable attr_reader :error - # @return [Boolean] Whether to revert to FDv1 - attr_reader :revert_to_fdv1 + # @return [Boolean] Whether the LaunchDarkly server has instructed the SDK to + # fall back to the FDv1 protocol. + attr_reader :fallback_to_fdv1 # @return [String, nil] The environment ID if available attr_reader :environment_id @@ -257,16 +258,20 @@ class Update # @param state [Symbol] The state of the data source # @param change_set [ChangeSet, nil] The change set if available # @param error [LaunchDarkly::Interfaces::DataSource::ErrorInfo, nil] Error information if applicable - # @param revert_to_fdv1 [Boolean] Whether to revert to FDv1 + # @param fallback_to_fdv1 [Boolean] Whether to fall back to FDv1 # @param environment_id [String, nil] The environment ID if available # - def initialize(state:, change_set: nil, error: nil, revert_to_fdv1: false, environment_id: nil) + def initialize(state:, change_set: nil, error: nil, fallback_to_fdv1: false, environment_id: nil) @state = state @change_set = change_set @error = error - @revert_to_fdv1 = revert_to_fdv1 + @fallback_to_fdv1 = fallback_to_fdv1 @environment_id = environment_id end + + # Deprecated alias retained for backwards compatibility while FDv2 is in early access. + # @deprecated Prefer {#fallback_to_fdv1}. + alias_method :revert_to_fdv1, :fallback_to_fdv1 end # diff --git a/lib/ldclient-rb/impl/data_system/fdv2.rb b/lib/ldclient-rb/impl/data_system/fdv2.rb index 0194cc35..c65cd951 100644 --- a/lib/ldclient-rb/impl/data_system/fdv2.rb +++ b/lib/ldclient-rb/impl/data_system/fdv2.rb @@ -214,8 +214,23 @@ def run_main_loop nil ) - # Run initializers first - run_initializers + # Run initializers first. If an initializer signals the + # server-directed FDv1 Fallback Directive, switch terminally to + # the FDv1 Fallback Synchronizer (or transition to OFF if none + # is configured) before entering the synchronizer phase. + if run_initializers + if @fdv1_fallback_synchronizer_builder + @logger.warn { "[LDClient] Falling back to FDv1 protocol" } + @synchronizer_builders = [@fdv1_fallback_synchronizer_builder] + else + @logger.warn { "[LDClient] Initializer requested FDv1 fallback but none configured" } + @synchronizer_builders = [] + @data_source_status_provider.update_status( + LaunchDarkly::Interfaces::DataSource::Status::OFF, + @data_source_status_provider.status.last_error + ) + end + end # Run synchronizers run_synchronizers @@ -228,39 +243,76 @@ def run_main_loop # # Run initializers to get initial data. # - # @return [void] + # Each initializer is tried in order until one succeeds, the system + # is stopped, or an initializer signals the server-directed FDv1 + # Fallback Directive. When fallback is signalled alongside a valid + # payload, that payload is applied before returning so evaluations + # can serve the server-provided data while the FDv1 synchronizer + # spins up. The method returns true when fallback was requested so + # that the caller can switch the synchronizer list. + # + # @return [Boolean] true when an initializer requested FDv1 fallback. # def run_initializers - return unless @data_system_config.initializers + return false unless @data_system_config.initializers @data_system_config.initializers.each do |initializer_builder| - return if @stop_event.set? + return false if @stop_event.set? begin initializer = initializer_builder.build(@sdk_key, @config) @logger.info { "[LDClient] Attempting to initialize via #{initializer.name}" } - basis_result = initializer.fetch(@store) + fetch_result = initializer.fetch(@store) + fallback = fetch_result.respond_to?(:fallback_to_fdv1) && fetch_result.fallback_to_fdv1 + # Support legacy implementations that return a bare Result. + basis_result = fetch_result.respond_to?(:result) ? fetch_result.result : fetch_result if basis_result.success? basis = basis_result.value @logger.info { "[LDClient] Initialized via #{initializer.name}" } - # Apply the basis to the store + # Apply the basis to the store regardless of whether + # fallback was signalled -- if the server returned a valid + # payload alongside the directive we still want evaluations + # to serve that data. @store.apply(basis.change_set, basis.persist) - # Set ready event if and only if a selector is defined for the changeset + # Set ready event if and only if a selector is defined for the changeset. + # Even when fallback is requested, the payload that arrived with the directive + # has been applied to the store, so evaluations can serve it while the FDv1 + # synchronizer spins up. if basis.change_set.selector && basis.change_set.selector.defined? @ready_event.set - return + return fallback ? true : false end else @logger.warn { "[LDClient] Initializer #{initializer.name} failed: #{basis_result.error}" } + if fallback + # Record the underlying initializer error so that, if no FDv1 fallback is + # configured, the subsequent transition to OFF carries it as last_error. + @data_source_status_provider.update_status( + LaunchDarkly::Interfaces::DataSource::Status::INITIALIZING, + LaunchDarkly::Interfaces::DataSource::ErrorInfo.new( + LaunchDarkly::Interfaces::DataSource::ErrorInfo::UNKNOWN, + 0, + basis_result.error || "", + Time.now + ) + ) + end end + + # Honor the FDv1 Fallback Directive even on an error or undefined-selector path: + # the directive takes precedence over the regular failover algorithm, so we must + # not fall through to the next initializer. + return true if fallback rescue => e @logger.error { "[LDClient] Initializer failed with exception: #{e.message}" } end end + + false end # @@ -410,7 +462,7 @@ def consume_synchronizer_results(synchronizer, check_recovery: false) # Update status @data_source_status_provider.update_status(update.state, update.error) - return SyncResult::FDV1 if update.revert_to_fdv1 + return SyncResult::FDV1 if update.fallback_to_fdv1 return SyncResult::REMOVE if update.state == LaunchDarkly::Interfaces::DataSource::Status::OFF end diff --git a/lib/ldclient-rb/impl/data_system/polling.rb b/lib/ldclient-rb/impl/data_system/polling.rb index fe680f54..0957dcbb 100644 --- a/lib/ldclient-rb/impl/data_system/polling.rb +++ b/lib/ldclient-rb/impl/data_system/polling.rb @@ -21,6 +21,18 @@ module DataSystem LD_ENVID_HEADER = "X-LD-EnvID" LD_FD_FALLBACK_HEADER = "X-LD-FD-Fallback" + # + # Reports whether the response headers signal that the SDK should fall + # back to the FDv1 protocol. + # + # @param headers [Hash, nil] + # @return [Boolean] + # + def self.fdv1_fallback_requested?(headers) + return false unless headers + headers[LD_FD_FALLBACK_HEADER] == 'true' + end + # # PollingDataSource is a data source that can retrieve information from # LaunchDarkly either as an Initializer or as a Synchronizer. @@ -46,10 +58,12 @@ def initialize(poll_interval, requester, logger) end # - # Fetch returns a Basis, or an error if the Basis could not be retrieved. + # Fetch returns a {LaunchDarkly::Interfaces::DataSystem::FetchResult} + # wrapping a Basis (or an error) and the FDv1 Fallback Directive + # signal carried on the server response. # # @param ss [LaunchDarkly::Interfaces::DataSystem::SelectorStore] - # @return [LaunchDarkly::Interfaces::DataSystem::Basis, nil] + # @return [LaunchDarkly::Interfaces::DataSystem::FetchResult] # def fetch(ss) poll(ss) @@ -73,13 +87,8 @@ def sync(ss) result = @requester.fetch(ss.selector) if !result.success? - fallback = false - envid = nil - - if result.headers - fallback = result.headers[LD_FD_FALLBACK_HEADER] == 'true' - envid = result.headers[LD_ENVID_HEADER] - end + fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(result.headers) + envid = result.headers ? result.headers[LD_ENVID_HEADER] : nil if result.exception.is_a?(LaunchDarkly::Impl::DataSource::UnexpectedResponseError) error_info = LaunchDarkly::Interfaces::DataSource::ErrorInfo.new( @@ -99,7 +108,7 @@ def sync(ss) state: LaunchDarkly::Interfaces::DataSource::Status::OFF, error: error_info, environment_id: envid, - revert_to_fdv1: true + fallback_to_fdv1: true ) break end @@ -108,7 +117,7 @@ def sync(ss) state: LaunchDarkly::Interfaces::DataSource::Status::INTERRUPTED, error: error_info, environment_id: envid, - revert_to_fdv1: false + fallback_to_fdv1: false ) @interrupt_event.wait(@poll_interval) next @@ -118,7 +127,7 @@ def sync(ss) state: LaunchDarkly::Interfaces::DataSource::Status::OFF, error: error_info, environment_id: envid, - revert_to_fdv1: fallback + fallback_to_fdv1: fallback ) break end @@ -136,7 +145,7 @@ def sync(ss) state: LaunchDarkly::Interfaces::DataSource::Status::OFF, error: error_info, environment_id: envid, - revert_to_fdv1: true + fallback_to_fdv1: true ) break end @@ -145,16 +154,16 @@ def sync(ss) state: LaunchDarkly::Interfaces::DataSource::Status::INTERRUPTED, error: error_info, environment_id: envid, - revert_to_fdv1: false + fallback_to_fdv1: false ) else change_set, headers = result.value - fallback = headers[LD_FD_FALLBACK_HEADER] == 'true' + fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers) yield LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, change_set: change_set, environment_id: headers[LD_ENVID_HEADER], - revert_to_fdv1: fallback + fallback_to_fdv1: fallback ) end @@ -177,11 +186,22 @@ def stop # # @param ss [LaunchDarkly::Interfaces::DataSystem::SelectorStore] - # @return [LaunchDarkly::Result] + # @return [LaunchDarkly::Interfaces::DataSystem::FetchResult] # private def poll(ss) result = @requester.fetch(ss.selector) + # On success, the requester returns headers as the second element of the value tuple; + # on failure, headers ride on Result.headers. Check both so the fallback signal is + # surfaced regardless of outcome. + response_headers = nil + if result.success? + _, response_headers = result.value + else + response_headers = result.headers + end + fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(response_headers) + unless result.success? if result.exception.is_a?(LaunchDarkly::Impl::DataSource::UnexpectedResponseError) status_code = result.exception.status @@ -189,10 +209,16 @@ def stop status_code, "polling request", "will retry" ) @logger.warn { "[LDClient] #{http_error_message_result}" } if Impl::Util.http_error_recoverable?(status_code) - return LaunchDarkly::Result.fail(http_error_message_result, result.exception) + return LaunchDarkly::Interfaces::DataSystem::FetchResult.new( + result: LaunchDarkly::Result.fail(http_error_message_result, result.exception), + fallback_to_fdv1: fallback + ) end - return LaunchDarkly::Result.fail(result.error || 'Failed to request payload', result.exception) + return LaunchDarkly::Interfaces::DataSystem::FetchResult.new( + result: LaunchDarkly::Result.fail(result.error || 'Failed to request payload', result.exception), + fallback_to_fdv1: fallback + ) end change_set, headers = result.value @@ -206,12 +232,18 @@ def stop environment_id: env_id ) - LaunchDarkly::Result.success(basis) + LaunchDarkly::Interfaces::DataSystem::FetchResult.new( + result: LaunchDarkly::Result.success(basis), + fallback_to_fdv1: fallback + ) rescue => e msg = "Error: Exception encountered when updating flags. #{e}" @logger.error { "[LDClient] #{msg}" } @logger.debug { "[LDClient] Exception trace: #{e.backtrace}" } - LaunchDarkly::Result.fail(msg, e) + LaunchDarkly::Interfaces::DataSystem::FetchResult.new( + result: LaunchDarkly::Result.fail(msg, e), + fallback_to_fdv1: false + ) end end diff --git a/lib/ldclient-rb/impl/data_system/streaming.rb b/lib/ldclient-rb/impl/data_system/streaming.rb index 05a39e47..ad00a129 100644 --- a/lib/ldclient-rb/impl/data_system/streaming.rb +++ b/lib/ldclient-rb/impl/data_system/streaming.rb @@ -90,11 +90,11 @@ def sync(ss) envid = headers[LD_ENVID_HEADER] || envid # Check for fallback header on connection - if headers[LD_FD_FALLBACK_HEADER] == 'true' + if LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers) log_connection_result(true) yield LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::OFF, - revert_to_fdv1: true, + fallback_to_fdv1: true, environment_id: envid ) stop @@ -150,10 +150,7 @@ def sync(ss) # Extract envid and fallback from error headers if available if error.respond_to?(:headers) && error.headers envid = error.headers[LD_ENVID_HEADER] || envid - - if error.headers[LD_FD_FALLBACK_HEADER] == 'true' - fallback = true - end + fallback = true if LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(error.headers) end update = handle_error(error, envid, fallback) @@ -286,7 +283,7 @@ def stop update = LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::OFF, error: error_info, - revert_to_fdv1: true, + fallback_to_fdv1: true, environment_id: envid ) stop diff --git a/lib/ldclient-rb/impl/integrations/file_data_source_v2.rb b/lib/ldclient-rb/impl/integrations/file_data_source_v2.rb index b1153992..3e9753bd 100644 --- a/lib/ldclient-rb/impl/integrations/file_data_source_v2.rb +++ b/lib/ldclient-rb/impl/integrations/file_data_source_v2.rb @@ -71,31 +71,38 @@ def name # Implementation of the Initializer.fetch method. # # Reads all configured files once and returns their contents as a Basis. + # File-based data sources never request the FDv1 Fallback Directive, + # so the returned {FetchResult} always reports `fallback_to_fdv1: false`. # # @param selector_store [LaunchDarkly::Interfaces::DataSystem::SelectorStore] Provides the Selector (unused for file data) - # @return [LaunchDarkly::Result] A Result containing either a Basis or an error message + # @return [LaunchDarkly::Interfaces::DataSystem::FetchResult] # def fetch(selector_store) - @lock.synchronize do - if @closed - return LaunchDarkly::Result.fail('FileDataV2 source has been closed') - end + result = + begin + @lock.synchronize do + if @closed + next LaunchDarkly::Result.fail('FileDataV2 source has been closed') + end - result = load_all_to_changeset - return result unless result.success? + load_result = load_all_to_changeset + next load_result unless load_result.success? - change_set = result.value - basis = LaunchDarkly::Interfaces::DataSystem::Basis.new( - change_set: change_set, - persist: false, - environment_id: nil - ) + change_set = load_result.value + basis = LaunchDarkly::Interfaces::DataSystem::Basis.new( + change_set: change_set, + persist: false, + environment_id: nil + ) - LaunchDarkly::Result.success(basis) - end - rescue => e - @logger.error { "[LDClient] Error fetching file data: #{e.message}" } - LaunchDarkly::Result.fail("Error fetching file data: #{e.message}", e) + LaunchDarkly::Result.success(basis) + end + rescue => e + @logger.error { "[LDClient] Error fetching file data: #{e.message}" } + LaunchDarkly::Result.fail("Error fetching file data: #{e.message}", e) + end + + LaunchDarkly::Interfaces::DataSystem::FetchResult.new(result: result, fallback_to_fdv1: false) end # @@ -110,14 +117,14 @@ def fetch(selector_store) # def sync(selector_store) # First yield initial data - initial_result = fetch(selector_store) - unless initial_result.success? + initial_fetch = fetch(selector_store) + unless initial_fetch.success? yield LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::OFF, error: LaunchDarkly::Interfaces::DataSource::ErrorInfo.new( LaunchDarkly::Interfaces::DataSource::ErrorInfo::INVALID_DATA, 0, - initial_result.error, + initial_fetch.error, Time.now ) ) @@ -126,7 +133,7 @@ def sync(selector_store) yield LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, - change_set: initial_result.value.change_set + change_set: initial_fetch.value.change_set ) # Start watching for file changes diff --git a/lib/ldclient-rb/impl/integrations/test_data/test_data_source_v2.rb b/lib/ldclient-rb/impl/integrations/test_data/test_data_source_v2.rb index 95dbdf29..b0b91071 100644 --- a/lib/ldclient-rb/impl/integrations/test_data/test_data_source_v2.rb +++ b/lib/ldclient-rb/impl/integrations/test_data/test_data_source_v2.rb @@ -46,56 +46,61 @@ def name # Implementation of the Initializer.fetch method. # # Returns the current test data as a Basis for initial data loading. + # Test data sources never request the FDv1 Fallback Directive, so the + # returned {FetchResult} always reports `fallback_to_fdv1: false`. # # @param selector_store [LaunchDarkly::Interfaces::DataSystem::SelectorStore] Provides the Selector (unused for test data) - # @return [LaunchDarkly::Result] A Result containing either a Basis or an error message + # @return [LaunchDarkly::Interfaces::DataSystem::FetchResult] # def fetch(selector_store) - begin - @lock.synchronize do - if @closed - return LaunchDarkly::Result.fail('TestDataV2 source has been closed') - end - - # Get all current flags and segments from test data - init_data = @test_data.make_init_data - version = @test_data.get_version - - # Build a full transfer changeset - builder = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.new - builder.start(LaunchDarkly::Interfaces::DataSystem::IntentCode::TRANSFER_FULL) - - # Add all flags to the changeset - init_data[:flags].each do |key, flag_data| - builder.add_put( - LaunchDarkly::Interfaces::DataSystem::ObjectKind::FLAG, - key, - flag_data[:version] || 1, - flag_data - ) - end - - # Add all segments to the changeset - init_data[:segments].each do |key, segment_data| - builder.add_put( - LaunchDarkly::Interfaces::DataSystem::ObjectKind::SEGMENT, - key, - segment_data[:version] || 1, - segment_data - ) + result = + begin + @lock.synchronize do + if @closed + next LaunchDarkly::Result.fail('TestDataV2 source has been closed') + end + + # Get all current flags and segments from test data + init_data = @test_data.make_init_data + version = @test_data.get_version + + # Build a full transfer changeset + builder = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.new + builder.start(LaunchDarkly::Interfaces::DataSystem::IntentCode::TRANSFER_FULL) + + # Add all flags to the changeset + init_data[:flags].each do |key, flag_data| + builder.add_put( + LaunchDarkly::Interfaces::DataSystem::ObjectKind::FLAG, + key, + flag_data[:version] || 1, + flag_data + ) + end + + # Add all segments to the changeset + init_data[:segments].each do |key, segment_data| + builder.add_put( + LaunchDarkly::Interfaces::DataSystem::ObjectKind::SEGMENT, + key, + segment_data[:version] || 1, + segment_data + ) + end + + # Create selector for this version + selector = LaunchDarkly::Interfaces::DataSystem::Selector.new_selector(version.to_s, version) + change_set = builder.finish(selector) + + basis = LaunchDarkly::Interfaces::DataSystem::Basis.new(change_set: change_set, persist: false, environment_id: nil) + + LaunchDarkly::Result.success(basis) end - - # Create selector for this version - selector = LaunchDarkly::Interfaces::DataSystem::Selector.new_selector(version.to_s, version) - change_set = builder.finish(selector) - - basis = LaunchDarkly::Interfaces::DataSystem::Basis.new(change_set: change_set, persist: false, environment_id: nil) - - LaunchDarkly::Result.success(basis) + rescue => e + LaunchDarkly::Result.fail("Error fetching test data: #{e.message}", e) end - rescue => e - LaunchDarkly::Result.fail("Error fetching test data: #{e.message}", e) - end + + LaunchDarkly::Interfaces::DataSystem::FetchResult.new(result: result, fallback_to_fdv1: false) end # @@ -109,14 +114,14 @@ def fetch(selector_store) # def sync(selector_store) # First yield initial data - initial_result = fetch(selector_store) - unless initial_result.success? + initial_fetch = fetch(selector_store) + unless initial_fetch.success? yield LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::OFF, error: LaunchDarkly::Interfaces::DataSource::ErrorInfo.new( LaunchDarkly::Interfaces::DataSource::ErrorInfo::STORE_ERROR, 0, - initial_result.error, + initial_fetch.error, Time.now ) ) @@ -126,7 +131,7 @@ def sync(selector_store) # Yield the initial successful state yield LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, - change_set: initial_result.value.change_set + change_set: initial_fetch.value.change_set ) # Continue yielding updates as they arrive diff --git a/lib/ldclient-rb/interfaces/data_system.rb b/lib/ldclient-rb/interfaces/data_system.rb index 92479b75..5fa3337a 100644 --- a/lib/ldclient-rb/interfaces/data_system.rb +++ b/lib/ldclient-rb/interfaces/data_system.rb @@ -559,8 +559,9 @@ class Update # @return [LaunchDarkly::Interfaces::DataSource::ErrorInfo, nil] Error information attr_reader :error - # @return [Boolean] Whether to revert to FDv1 - attr_reader :revert_to_fdv1 + # @return [Boolean] Whether the LaunchDarkly server has instructed the SDK to fall + # back to the FDv1 protocol (signalled via the `X-LD-FD-Fallback` response header). + attr_reader :fallback_to_fdv1 # @return [String, nil] The environment ID attr_reader :environment_id @@ -569,16 +570,72 @@ class Update # @param state [Symbol] The data source state ({LaunchDarkly::Interfaces::DataSource::Status}) # @param change_set [ChangeSet, nil] The change set # @param error [LaunchDarkly::Interfaces::DataSource::ErrorInfo, nil] Error information - # @param revert_to_fdv1 [Boolean] Whether to revert to FDv1 + # @param fallback_to_fdv1 [Boolean] Whether to fall back to FDv1 # @param environment_id [String, nil] The environment ID # - def initialize(state:, change_set: nil, error: nil, revert_to_fdv1: false, environment_id: nil) + def initialize(state:, change_set: nil, error: nil, fallback_to_fdv1: false, environment_id: nil) @state = state @change_set = change_set @error = error - @revert_to_fdv1 = revert_to_fdv1 + @fallback_to_fdv1 = fallback_to_fdv1 @environment_id = environment_id end + + # Deprecated alias retained so that existing callers continue to work + # while the FDv2 data system is in early access. Prefer + # {#fallback_to_fdv1}. + # @deprecated + alias_method :revert_to_fdv1, :fallback_to_fdv1 + end + + # + # FetchResult pairs the result of an {Initializer#fetch} call with the + # server-directed FDv1 Fallback Directive signal. + # + # When the LaunchDarkly server returns the `X-LD-FD-Fallback: true` + # response header on an initializer response, the SDK must apply any + # accompanying payload and then switch to the FDv1 Fallback Synchronizer. + # Surfacing this signal alongside the {LaunchDarkly::Result} ensures + # callers cannot silently drop it. + # + # This type is not stable, and not subject to any backwards compatibility guarantees or semantic versioning. + # It is in early access. If you want access to this feature please join the EAP. https://launchdarkly.com/docs/sdk/features/data-saving-mode + # + class FetchResult + # @return [LaunchDarkly::Result] A Result containing either a {Basis} or an error. + attr_reader :result + + # @return [Boolean] Whether the server has instructed the SDK to fall back to the FDv1 protocol. + attr_reader :fallback_to_fdv1 + + # + # @param result [LaunchDarkly::Result] A Result containing either a Basis or an error. + # @param fallback_to_fdv1 [Boolean] Whether to fall back to FDv1. + # + def initialize(result:, fallback_to_fdv1: false) + @result = result + @fallback_to_fdv1 = fallback_to_fdv1 + end + + # @return [Boolean] true when the underlying Result was successful. + def success? + @result.success? + end + + # @return [Object, nil] The {Basis} returned from a successful fetch, or nil. + def value + @result.value + end + + # @return [String, nil] An error description, or nil on success. + def error + @result.error + end + + # @return [Exception, nil] An optional exception describing the failure. + def exception + @result.exception + end end # @@ -655,8 +712,14 @@ def name # # Retrieves the initial data set for the data source. # + # If the LaunchDarkly server has instructed the SDK to fall back to + # the FDv1 protocol, the returned {FetchResult#fallback_to_fdv1} is + # true. The wrapped result may still carry a successful {Basis} when + # the directive accompanied a valid payload, in which case callers + # should apply the payload before switching protocols. + # # @param selector_store [SelectorStore] Provides the Selector - # @return [LaunchDarkly::Result] + # @return [FetchResult] # def fetch(selector_store) raise NotImplementedError, "#{self.class} must implement #fetch" diff --git a/spec/impl/data_system/fdv2_datasystem_spec.rb b/spec/impl/data_system/fdv2_datasystem_spec.rb index 6aabfc3c..3cff8ec1 100644 --- a/spec/impl/data_system/fdv2_datasystem_spec.rb +++ b/spec/impl/data_system/fdv2_datasystem_spec.rb @@ -220,15 +220,15 @@ def build(_sdk_key, _config) end describe "FDv1 fallback on polling error with header" do - it "falls back to FDv1 when synchronizer signals revert_to_fdv1" do + it "falls back to FDv1 when synchronizer signals fallback_to_fdv1" do mock_primary = double("primary_synchronizer") allow(mock_primary).to receive(:name).and_return("mock-primary") allow(mock_primary).to receive(:stop) - # Simulate a synchronizer that yields an OFF state with revert_to_fdv1=true + # Simulate a synchronizer that yields an OFF state with fallback_to_fdv1=true update = LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::OFF, - revert_to_fdv1: true + fallback_to_fdv1: true ) allow(mock_primary).to receive(:sync).and_yield(update) @@ -270,14 +270,14 @@ def build(_sdk_key, _config) end describe "FDv1 fallback on polling success with header" do - it "falls back to FDv1 even when primary yields valid data with revert_to_fdv1" do + it "falls back to FDv1 even when primary yields valid data with fallback_to_fdv1" do mock_primary = double("primary_synchronizer") allow(mock_primary).to receive(:name).and_return("mock-primary") allow(mock_primary).to receive(:stop) update = LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, - revert_to_fdv1: true + fallback_to_fdv1: true ) allow(mock_primary).to receive(:sync).and_yield(update) @@ -337,7 +337,7 @@ def build(_sdk_key, _config) update = LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::OFF, - revert_to_fdv1: true + fallback_to_fdv1: true ) allow(mock_primary).to receive(:sync).and_yield(update) @@ -377,14 +377,14 @@ def build(_sdk_key, _config) end describe "no fallback without header" do - it "does not fall back to FDv1 when revert_to_fdv1 is false" do + it "does not fall back to FDv1 when fallback_to_fdv1 is false" do mock_primary = double("primary_synchronizer") allow(mock_primary).to receive(:name).and_return("mock-primary") allow(mock_primary).to receive(:stop) update = LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::INTERRUPTED, - revert_to_fdv1: false + fallback_to_fdv1: false ) allow(mock_primary).to receive(:sync).and_yield(update) @@ -395,7 +395,7 @@ def build(_sdk_key, _config) valid_update = LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, - revert_to_fdv1: false + fallback_to_fdv1: false ) allow(mock_secondary).to receive(:sync).and_yield(valid_update) @@ -437,7 +437,7 @@ def build(_sdk_key, _config) update = LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::OFF, - revert_to_fdv1: true + fallback_to_fdv1: true ) allow(mock_primary).to receive(:sync).and_yield(update) @@ -470,6 +470,121 @@ def build(_sdk_key, _config) fdv2.stop end end + + describe "FDv1 fallback signalled by initializer" do + # Stub initializer that returns whatever FetchResult we provide, exactly once. + class StubInitializer + include LaunchDarkly::Interfaces::DataSystem::Initializer + + def initialize(fetch_result) + @fetch_result = fetch_result + end + + def name + "StubInitializer" + end + + def fetch(_selector_store) + @fetch_result + end + end + + class StubInitializerBuilder + def initialize(fetch_result) + @fetch_result = fetch_result + end + + def build(_sdk_key, _config) + StubInitializer.new(@fetch_result) + end + end + + it "switches to the FDv1 fallback synchronizer when an initializer requests fallback" do + # Initializer returns a successful payload AND fallback_to_fdv1 -- the SDK should + # apply the payload, then run only the FDv1 fallback synchronizer. + td_initializer = LaunchDarkly::Integrations::TestDataV2.data_source + td_initializer.update(td_initializer.flag("initialflag").on(true)) + initializer_fetch = td_initializer.test_data_ds_builder.build(sdk_key, config).fetch(nil) + fallback_fetch = LaunchDarkly::Interfaces::DataSystem::FetchResult.new( + result: initializer_fetch.result, + fallback_to_fdv1: true + ) + + # Mock primary synchronizer must not be invoked because the directive switches the + # synchronizer list to the FDv1 fallback before sync runs. + mock_primary = double("primary_synchronizer") + allow(mock_primary).to receive(:name).and_return("mock-primary") + allow(mock_primary).to receive(:stop) + allow(mock_primary).to receive(:sync) + + td_fdv1 = LaunchDarkly::Integrations::TestDataV2.data_source + td_fdv1.update(td_fdv1.flag("fdv1flag").on(true)) + + data_system_config = LaunchDarkly::DataSystem::ConfigBuilder.new + .initializers([StubInitializerBuilder.new(fallback_fetch)]) + .synchronizers([MockBuilder.new(mock_primary)]) + .fdv1_compatible_synchronizer(td_fdv1.test_data_ds_builder) + .build + + changed = Concurrent::Event.new + seen_keys = [] + + listener = Object.new + listener.define_singleton_method(:update) do |flag_change| + seen_keys << flag_change.key + changed.set if seen_keys.include?("fdv1flag") + end + + fdv2 = FDv2.new(sdk_key, config, data_system_config) + fdv2.flag_change_broadcaster.add_listener(listener) + + ready_event = fdv2.start + expect(ready_event.wait(2)).to be true + expect(changed.wait(2)).to be true + + # Initializer payload must have been applied -- the FDv1 fallback synchronizer is then + # responsible for continued updates. + expect(seen_keys).to include("initialflag") + expect(seen_keys).to include("fdv1flag") + expect(mock_primary).not_to have_received(:sync) + + fdv2.stop + end + + it "transitions the data source status to OFF when fallback is requested but no FDv1 fallback configured" do + # An initializer error accompanied by fallback_to_fdv1 with no FDv1 fallback configured + # must produce an OFF status -- the directive takes precedence over the regular failover + # algorithm, which would otherwise leave the system stuck in INITIALIZING. + error_fetch = LaunchDarkly::Interfaces::DataSystem::FetchResult.new( + result: LaunchDarkly::Result.fail( + "boom", + LaunchDarkly::Impl::DataSource::UnexpectedResponseError.new(500) + ), + fallback_to_fdv1: true + ) + + data_system_config = LaunchDarkly::DataSystem::ConfigBuilder.new + .initializers([StubInitializerBuilder.new(error_fetch)]) + .synchronizers(nil) + .build + + off_status = Concurrent::Event.new + listener = Object.new + listener.define_singleton_method(:update) do |status| + off_status.set if status.state == LaunchDarkly::Interfaces::DataSource::Status::OFF + end + + fdv2 = FDv2.new(sdk_key, config, data_system_config) + fdv2.data_source_status_provider.add_listener(listener) + + ready_event = fdv2.start + expect(ready_event.wait(2)).to be true + expect(off_status.wait(2)).to be true + expect(fdv2.data_source_status_provider.status.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::OFF) + + fdv2.stop + end + end end end end diff --git a/spec/impl/data_system/polling_initializer_spec.rb b/spec/impl/data_system/polling_initializer_spec.rb index dc699c36..8a27dd97 100644 --- a/spec/impl/data_system/polling_initializer_spec.rb +++ b/spec/impl/data_system/polling_initializer_spec.rb @@ -134,6 +134,59 @@ def selector expect(result.value.persist).to eq(true) end + it "surfaces fallback_to_fdv1 on a successful response with the fallback header" do + # Server-directed FDv1 Fallback Directive may ride along on a 200 response that also + # carries a valid payload. The SDK must apply the payload AND surface the fallback + # signal so the data system can transition to the FDv1 Fallback Synchronizer. + change_set = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes + headers = { LD_FD_FALLBACK_HEADER => 'true' } + mock_requester = MockPollingRequester.new( + LaunchDarkly::Result.success([change_set, headers]) + ) + ds = PollingDataSource.new(1.0, mock_requester, logger) + + fetch_result = ds.fetch(MockSelectorStore.new(LaunchDarkly::Interfaces::DataSystem::Selector.no_selector)) + + expect(fetch_result).to be_a(LaunchDarkly::Interfaces::DataSystem::FetchResult) + expect(fetch_result.success?).to be true + expect(fetch_result.fallback_to_fdv1).to be true + expect(fetch_result.value).to be_a(LaunchDarkly::Interfaces::DataSystem::Basis) + end + + it "surfaces fallback_to_fdv1 on an error response with the fallback header" do + # Even on a 500 response, the fallback header should be surfaced so the caller can + # branch on the directive before the recoverable-error logic kicks in. + headers_with_fallback = { LD_FD_FALLBACK_HEADER => 'true' } + error_result = LaunchDarkly::Result.fail( + "failure message", + LaunchDarkly::Impl::DataSource::UnexpectedResponseError.new(500), + headers_with_fallback + ) + mock_requester = MockPollingRequester.new(error_result) + ds = PollingDataSource.new(1.0, mock_requester, logger) + + fetch_result = ds.fetch(MockSelectorStore.new(LaunchDarkly::Interfaces::DataSystem::Selector.no_selector)) + + expect(fetch_result).to be_a(LaunchDarkly::Interfaces::DataSystem::FetchResult) + expect(fetch_result.success?).to be false + expect(fetch_result.fallback_to_fdv1).to be true + end + + it "reports fallback_to_fdv1 as false when the header is absent on error" do + mock_requester = MockPollingRequester.new( + LaunchDarkly::Result.fail( + "failure message", + LaunchDarkly::Impl::DataSource::UnexpectedResponseError.new(500) + ) + ) + ds = PollingDataSource.new(1.0, mock_requester, logger) + + fetch_result = ds.fetch(MockSelectorStore.new(LaunchDarkly::Interfaces::DataSystem::Selector.no_selector)) + + expect(fetch_result.success?).to be false + expect(fetch_result.fallback_to_fdv1).to be false + end + it "handles transfer changes" do payload_str = '{"events":[{"event": "server-intent","data": {"payloads":[{"id":"5A46PZ79FQ9D08YYKT79DECDNV","target":462,"intentCode":"xfer-changes","reason":"stale"}]}},{"event": "put-object","data": {"key":"sample-feature","kind":"flag","version":462,"object":{"key":"sample-feature","on":true,"prerequisites":[],"targets":[],"contextTargets":[],"rules":[],"fallthrough":{"variation":0},"offVariation":1,"variations":[true,false],"clientSideAvailability":{"usingMobileKey":false,"usingEnvironmentId":false},"clientSide":false,"salt":"9945e63a79a44787805b79728fee1926","trackEvents":false,"trackEventsFallthrough":false,"debugEventsUntilDate":null,"version":113,"deleted":false}}},{"event": "payload-transferred","data": {"state":"(p:5A46PZ79FQ9D08YYKT79DECDNV:462)","id":"5A46PZ79FQ9D08YYKT79DECDNV","version":462}}]}' # rubocop:disable Layout/LineLength change_set_result = LaunchDarkly::Impl::DataSystem.polling_payload_to_changeset(JSON.parse(payload_str, symbolize_names: true)) diff --git a/spec/impl/data_system/polling_synchronizer_spec.rb b/spec/impl/data_system/polling_synchronizer_spec.rb index f7d9031e..19edc130 100644 --- a/spec/impl/data_system/polling_synchronizer_spec.rb +++ b/spec/impl/data_system/polling_synchronizer_spec.rb @@ -79,7 +79,7 @@ def selector expect(valid.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(valid.error).to be_nil - expect(valid.revert_to_fdv1).to eq(false) + expect(valid.fallback_to_fdv1).to eq(false) expect(valid.environment_id).to be_nil expect(valid.change_set).not_to be_nil expect(valid.change_set.intent_code).to eq(LaunchDarkly::Interfaces::DataSystem::IntentCode::TRANSFER_NONE) @@ -111,7 +111,7 @@ def selector expect(valid.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(valid.error).to be_nil - expect(valid.revert_to_fdv1).to eq(false) + expect(valid.fallback_to_fdv1).to eq(false) expect(valid.environment_id).to be_nil expect(valid.change_set).not_to be_nil expect(valid.change_set.changes.length).to eq(0) @@ -152,7 +152,7 @@ def selector expect(valid.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(valid.error).to be_nil - expect(valid.revert_to_fdv1).to eq(false) + expect(valid.fallback_to_fdv1).to eq(false) expect(valid.environment_id).to be_nil expect(valid.change_set).not_to be_nil expect(valid.change_set.changes.length).to eq(1) @@ -193,7 +193,7 @@ def selector expect(valid.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(valid.error).to be_nil - expect(valid.revert_to_fdv1).to eq(false) + expect(valid.fallback_to_fdv1).to eq(false) expect(valid.environment_id).to be_nil expect(valid.change_set).not_to be_nil expect(valid.change_set.changes.length).to eq(1) @@ -244,7 +244,7 @@ def selector expect(interrupted.error.kind).to eq(LaunchDarkly::Interfaces::DataSource::ErrorInfo::NETWORK_ERROR) expect(interrupted.error.status_code).to eq(0) expect(interrupted.error.message).to eq("error for test") - expect(interrupted.revert_to_fdv1).to eq(false) + expect(interrupted.fallback_to_fdv1).to eq(false) expect(interrupted.environment_id).to be_nil expect(valid.change_set).not_to be_nil @@ -291,12 +291,12 @@ def selector expect(interrupted.error).not_to be_nil expect(interrupted.error.kind).to eq(LaunchDarkly::Interfaces::DataSource::ErrorInfo::ERROR_RESPONSE) expect(interrupted.error.status_code).to eq(408) - expect(interrupted.revert_to_fdv1).to eq(false) + expect(interrupted.fallback_to_fdv1).to eq(false) expect(interrupted.environment_id).to be_nil expect(valid.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(valid.error).to be_nil - expect(valid.revert_to_fdv1).to eq(false) + expect(valid.fallback_to_fdv1).to eq(false) expect(valid.environment_id).to be_nil expect(valid.change_set).not_to be_nil @@ -333,7 +333,7 @@ def selector expect(off.error).not_to be_nil expect(off.error.kind).to eq(LaunchDarkly::Interfaces::DataSource::ErrorInfo::ERROR_RESPONSE) expect(off.error.status_code).to eq(401) - expect(off.revert_to_fdv1).to eq(false) + expect(off.fallback_to_fdv1).to eq(false) expect(off.environment_id).to be_nil expect(off.change_set).to be_nil end @@ -361,7 +361,7 @@ def selector expect(valid.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(valid.error).to be_nil - expect(valid.revert_to_fdv1).to eq(false) + expect(valid.fallback_to_fdv1).to eq(false) expect(valid.environment_id).to eq('test-env-polling-123') end @@ -399,7 +399,7 @@ def selector expect(valid.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(valid.environment_id).to eq('test-env-456') - expect(valid.revert_to_fdv1).to eq(true) + expect(valid.fallback_to_fdv1).to eq(true) expect(valid.change_set).not_to be_nil expect(valid.change_set.changes.length).to eq(1) end @@ -514,7 +514,7 @@ def selector # When fallback header is present, status is OFF (not INTERRUPTED) expect(off.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::OFF) - expect(off.revert_to_fdv1).to eq(true) + expect(off.fallback_to_fdv1).to eq(true) expect(off.environment_id).to eq('test-env-503') end @@ -591,7 +591,7 @@ def selector # When fallback header is present on parse error, status is OFF expect(off.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::OFF) - expect(off.revert_to_fdv1).to eq(true) + expect(off.fallback_to_fdv1).to eq(true) expect(off.environment_id).to eq('test-env-parse-error') expect(off.error).not_to be_nil expect(off.error.kind).to eq(LaunchDarkly::Interfaces::DataSource::ErrorInfo::NETWORK_ERROR) @@ -632,7 +632,7 @@ def selector # When fallback header is present on recoverable error, status is OFF expect(off.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::OFF) - expect(off.revert_to_fdv1).to eq(true) + expect(off.fallback_to_fdv1).to eq(true) expect(off.environment_id).to eq('test-env-408') expect(off.error).not_to be_nil expect(off.error.kind).to eq(LaunchDarkly::Interfaces::DataSource::ErrorInfo::ERROR_RESPONSE) @@ -674,7 +674,7 @@ def selector # Should use the data (VALID state) but signal future fallback expect(valid.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) - expect(valid.revert_to_fdv1).to eq(true) + expect(valid.fallback_to_fdv1).to eq(true) expect(valid.environment_id).to eq('test-env-success-fallback') expect(valid.error).to be_nil expect(valid.change_set).not_to be_nil # Data is provided diff --git a/spec/impl/data_system/streaming_headers_spec.rb b/spec/impl/data_system/streaming_headers_spec.rb index 2b059b9b..707db3a2 100644 --- a/spec/impl/data_system/streaming_headers_spec.rb +++ b/spec/impl/data_system/streaming_headers_spec.rb @@ -37,7 +37,7 @@ module DataSystem update = synchronizer.send(:handle_error, error_with_fallback, "test-env-123", true) - expect(update.revert_to_fdv1).to be true + expect(update.fallback_to_fdv1).to be true expect(update.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::OFF) expect(update.environment_id).to eq("test-env-123") end @@ -53,7 +53,7 @@ module DataSystem update = synchronizer.send(:handle_error, error_without_fallback, "test-env-456", false) - expect(update.revert_to_fdv1).to be_falsy + expect(update.fallback_to_fdv1).to be_falsy expect(update.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::INTERRUPTED) end diff --git a/spec/impl/data_system/streaming_synchronizer_spec.rb b/spec/impl/data_system/streaming_synchronizer_spec.rb index 11351e76..d00ac22a 100644 --- a/spec/impl/data_system/streaming_synchronizer_spec.rb +++ b/spec/impl/data_system/streaming_synchronizer_spec.rb @@ -69,7 +69,7 @@ def initialize(type, data = nil) expect(update).not_to be_nil expect(update.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(update.error).to be_nil - expect(update.revert_to_fdv1).to eq(false) + expect(update.fallback_to_fdv1).to eq(false) expect(update.environment_id).to be_nil expect(update.change_set).to be_nil end @@ -102,7 +102,7 @@ def initialize(type, data = nil) expect(update).not_to be_nil expect(update.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(update.error).to be_nil - expect(update.revert_to_fdv1).to eq(false) + expect(update.fallback_to_fdv1).to eq(false) expect(update.environment_id).to be_nil expect(update.change_set).not_to be_nil expect(update.change_set.changes.length).to eq(0) @@ -153,7 +153,7 @@ def initialize(type, data = nil) expect(update).not_to be_nil expect(update.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(update.error).to be_nil - expect(update.revert_to_fdv1).to eq(false) + expect(update.fallback_to_fdv1).to eq(false) expect(update.environment_id).to be_nil expect(update.change_set).not_to be_nil expect(update.change_set.changes.length).to eq(1) @@ -204,7 +204,7 @@ def initialize(type, data = nil) expect(update).not_to be_nil expect(update.state).to eq(LaunchDarkly::Interfaces::DataSource::Status::VALID) expect(update.error).to be_nil - expect(update.revert_to_fdv1).to eq(false) + expect(update.fallback_to_fdv1).to eq(false) expect(update.environment_id).to be_nil expect(update.change_set).not_to be_nil expect(update.change_set.changes.length).to eq(1) diff --git a/spec/impl/datasystem_spec.rb b/spec/impl/datasystem_spec.rb index 7805900b..3949170b 100644 --- a/spec/impl/datasystem_spec.rb +++ b/spec/impl/datasystem_spec.rb @@ -103,7 +103,7 @@ module Impl expect(update.state).to eq(:valid) expect(update.change_set).to be_nil expect(update.error).to be_nil - expect(update.revert_to_fdv1).to be false + expect(update.fallback_to_fdv1).to be false expect(update.environment_id).to be_nil end @@ -115,14 +115,14 @@ module Impl state: :interrupted, change_set: change_set, error: error, - revert_to_fdv1: true, + fallback_to_fdv1: true, environment_id: "env-123" ) expect(update.state).to eq(:interrupted) expect(update.change_set).to eq(change_set) expect(update.error).to eq(error) - expect(update.revert_to_fdv1).to be true + expect(update.fallback_to_fdv1).to be true expect(update.environment_id).to eq("env-123") end end From e7b75a5ed5b4aa0d027f9246a12a989ec929fa36 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Thu, 30 Apr 2026 11:15:00 -0400 Subject: [PATCH 2/4] test: Wire FDv1 Fallback Directive contract-test capability The contract harness now treats the FDv1 Fallback Synchronizer as a distinct field on the data system (DataSystem.FDv1Fallback) rather than deriving it from the FDv2 synchronizer chain, and gates the directive subtests on a new fdv1-fallback capability. Wire the test service to match: - declare the fdv1-fallback capability - accept the new dataSystem.fdv1Fallback config field - build the FDv1 fallback synchronizer from that field directly, instead of inferring it from the last polling synchronizer Also bump the contract-tests pin from v3.0.0-alpha.4 to v3.0.0-alpha.6 so the harness ships the new directive subtests. --- .github/actions/check/action.yml | 2 +- contract-tests/client_entity.rb | 27 ++++++++++++++------------- contract-tests/service.rb | 1 + 3 files changed, 16 insertions(+), 14 deletions(-) diff --git a/.github/actions/check/action.yml b/.github/actions/check/action.yml index 7f6539df..48dc37e8 100644 --- a/.github/actions/check/action.yml +++ b/.github/actions/check/action.yml @@ -51,4 +51,4 @@ runs: test_service_port: 9000 enable_persistence_tests: true token: ${{ inputs.token }} - version: v3.0.0-alpha.4 + version: v3.0.0-alpha.6 diff --git a/contract-tests/client_entity.rb b/contract-tests/client_entity.rb index 0add362f..46194a16 100644 --- a/contract-tests/client_entity.rb +++ b/contract-tests/client_entity.rb @@ -42,9 +42,14 @@ def initialize(log, config) if sync_configs && !sync_configs.empty? synchronizer_builders = sync_configs.map { |sync_config| build_synchronizer_builder(sync_config) }.compact data_system.synchronizers(synchronizer_builders) unless synchronizer_builders.empty? + end - fallback_builder = build_fdv1_fallback_builder(sync_configs) - data_system.fdv1_compatible_synchronizer(fallback_builder) + # The FDv1 Fallback Synchronizer is wired directly from a top-level + # dataSystem.fdv1Fallback config block -- the test harness no longer + # infers it from the FDv2 synchronizer chain. + fdv1_fallback_config = data_system_config[:fdv1Fallback] + if fdv1_fallback_config + data_system.fdv1_compatible_synchronizer(build_fdv1_fallback_builder(fdv1_fallback_config)) end if data_system_config[:payloadFilter] @@ -342,21 +347,17 @@ def close end # - # Builds an FDv1 fallback polling data source builder using the first available polling config. + # Builds an FDv1 fallback polling data source builder from the dedicated + # `dataSystem.fdv1Fallback` config block. This block has the same shape as + # a polling config (`baseUri`, `pollIntervalMs`). # - # @param sync_configs [Array] Array of synchronizer configurations + # @param fdv1_fallback_config [Hash] The FDv1 fallback configuration # @return [Object] Returns the configured FDv1 fallback builder # - private def build_fdv1_fallback_builder(sync_configs) + private def build_fdv1_fallback_builder(fdv1_fallback_config) builder = LaunchDarkly::DataSystem.fdv1_fallback_ds_builder - - # Use the first available polling config for the fallback base_uri - polling_config = sync_configs.lazy.map { |c| c[:polling] }.detect { |p| p } - if polling_config - builder.base_uri(polling_config[:baseUri]) if polling_config[:baseUri] - builder.poll_interval(polling_config[:pollIntervalMs] / 1_000.0) if polling_config[:pollIntervalMs] - end - + builder.base_uri(fdv1_fallback_config[:baseUri]) if fdv1_fallback_config[:baseUri] + builder.poll_interval(fdv1_fallback_config[:pollIntervalMs] / 1_000.0) if fdv1_fallback_config[:pollIntervalMs] builder end diff --git a/contract-tests/service.rb b/contract-tests/service.rb index 4d083696..31dd8d71 100644 --- a/contract-tests/service.rb +++ b/contract-tests/service.rb @@ -53,6 +53,7 @@ 'persistent-data-store-redis', 'flag-change-listeners', 'flag-value-change-listeners', + 'fdv1-fallback', ], }.to_json end From f8fdea681954f12f7887294d0c4efbeee7e50fb5 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Thu, 30 Apr 2026 11:55:33 -0400 Subject: [PATCH 3/4] fix: Honor FDv1 Fallback Directive across header casings and streaming Three contract-test failures surfaced by sdk-test-harness v3.0.0-alpha.6 shared a single underlying defect: the directive was honored in the unit tests but silently dropped in the integration paths. - Polling-initializer signal lost. HTTPPollingRequester#fetch downcases response header keys before handing them to the data system, but the fallback-detection helper was a case-sensitive Hash lookup against the canonical mixed-case constant. Replace the lookup with a helper that handles both plain Hashes and case-insensitive header containers (e.g. HTTP::Headers from the http gem). Apply the same helper to the env-id lookups so neither signal disappears against a downcased map. - Streaming-success directive dropped the payload. The on_connect handler yielded OFF and stopped the stream the moment the fallback header arrived, so the SSE handshake never produced a payload to apply (Requirement 1.6.2). Record the directive as a pending flag and let event processing continue; ride the signal on the next Valid update so the consumer applies the ChangeSet first, then transition to FDv1 and close the FDv2 stream. - "No FDv1 fallback configured" branch didn't halt. When the synchronizer loop received SyncResult::FDV1 and no FDv1 fallback builder was wired, it incremented current_index past the end of the builder list, where the existing wrap-around immediately reset it to zero -- producing an infinite reconnect loop against the same FDv2 endpoint that just delivered the directive. Transition the data source status to OFF and break the loop instead, mirroring Requirement 1.6.3(4). Adds case-insensitivity unit tests so a future refactor cannot resilently re-break the polling path, and includes coverage for the HTTP::Headers-style case-insensitive container shape that the streaming code paths actually receive. --- lib/ldclient-rb/impl/data_system/fdv2.rb | 13 ++- lib/ldclient-rb/impl/data_system/polling.rb | 58 +++++++++++-- lib/ldclient-rb/impl/data_system/streaming.rb | 52 ++++++++---- .../data_system/polling_initializer_spec.rb | 84 +++++++++++++++++++ 4 files changed, 185 insertions(+), 22 deletions(-) diff --git a/lib/ldclient-rb/impl/data_system/fdv2.rb b/lib/ldclient-rb/impl/data_system/fdv2.rb index c65cd951..4d4cb287 100644 --- a/lib/ldclient-rb/impl/data_system/fdv2.rb +++ b/lib/ldclient-rb/impl/data_system/fdv2.rb @@ -365,12 +365,21 @@ def synchronizer_loop case sync_result when SyncResult::FDV1 if @fdv1_fallback_synchronizer_builder + @logger.warn { "[LDClient] Falling back to FDv1 protocol" } @synchronizer_builders = [@fdv1_fallback_synchronizer_builder] current_index = 0 next end - # No FDv1 fallback configured, treat as regular fallback - current_index += 1 + # No FDv1 fallback configured: per spec section 1.6.3(4) the + # data system must HALT rather than fall through to the next + # FDv2 synchronizer. Continuing to retry would reopen the + # connection that just delivered the directive. + @logger.warn { "[LDClient] Synchronizer requested FDv1 fallback but none configured; halting data system" } + @data_source_status_provider.update_status( + LaunchDarkly::Interfaces::DataSource::Status::OFF, + @data_source_status_provider.status.last_error + ) + break when SyncResult::RECOVER @logger.info { "[LDClient] Recovery condition met, returning to primary synchronizer" } current_index = 0 diff --git a/lib/ldclient-rb/impl/data_system/polling.rb b/lib/ldclient-rb/impl/data_system/polling.rb index 0957dcbb..25672598 100644 --- a/lib/ldclient-rb/impl/data_system/polling.rb +++ b/lib/ldclient-rb/impl/data_system/polling.rb @@ -25,12 +25,58 @@ module DataSystem # Reports whether the response headers signal that the SDK should fall # back to the FDv1 protocol. # - # @param headers [Hash, nil] + # The header lookup is case-insensitive: response header maps in this + # codebase arrive in three flavours -- the http gem's case-insensitive + # `HTTP::Headers`, plain Ruby Hashes whose keys we have downcased + # (e.g. {HTTPPollingRequester#fetch}), and Hashes with the canonical + # mixed-case key. Lookups in only one of those casings would silently + # drop the directive against a perfectly valid response. + # + # @param headers [#[], Hash, nil] # @return [Boolean] # def self.fdv1_fallback_requested?(headers) - return false unless headers - headers[LD_FD_FALLBACK_HEADER] == 'true' + return false if headers.nil? + value = lookup_header(headers, LD_FD_FALLBACK_HEADER) + # http gem returns arrays for repeated headers; normalize to a string. + value = value.first if value.is_a?(Array) + value == 'true' + end + + # + # Performs a case-insensitive header lookup that works with both + # case-insensitive header containers (e.g. `HTTP::Headers`) and plain + # Ruby hashes -- including hashes whose keys we have downcased + # ourselves before reaching this code path. + # + # @param headers [#[], Hash] + # @param name [String] + # @return [String, Array, nil] + # + def self.lookup_header(headers, name) + return nil if headers.nil? + + if headers.is_a?(Hash) + # Plain hash: try canonical case, then exact lowercase, then a + # case-insensitive scan as a final fallback. + value = headers[name] + return value unless value.nil? + + downcased = name.downcase + value = headers[downcased] + return value unless value.nil? + + headers.each_pair do |key, val| + return val if key.to_s.downcase == downcased + end + return nil + end + + # Non-hash container (e.g. HTTP::Headers). Lookup via [] is + # already case-insensitive on those types. + return headers[name] if headers.respond_to?(:[]) + + nil end # @@ -88,7 +134,7 @@ def sync(ss) if !result.success? fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(result.headers) - envid = result.headers ? result.headers[LD_ENVID_HEADER] : nil + envid = result.headers ? LaunchDarkly::Impl::DataSystem.lookup_header(result.headers, LD_ENVID_HEADER) : nil if result.exception.is_a?(LaunchDarkly::Impl::DataSource::UnexpectedResponseError) error_info = LaunchDarkly::Interfaces::DataSource::ErrorInfo.new( @@ -162,7 +208,7 @@ def sync(ss) yield LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, change_set: change_set, - environment_id: headers[LD_ENVID_HEADER], + environment_id: LaunchDarkly::Impl::DataSystem.lookup_header(headers, LD_ENVID_HEADER), fallback_to_fdv1: fallback ) end @@ -223,7 +269,7 @@ def stop change_set, headers = result.value - env_id = headers[LD_ENVID_HEADER] + env_id = LaunchDarkly::Impl::DataSystem.lookup_header(headers, LD_ENVID_HEADER) env_id = nil unless env_id.is_a?(String) basis = LaunchDarkly::Interfaces::DataSystem::Basis.new( diff --git a/lib/ldclient-rb/impl/data_system/streaming.rb b/lib/ldclient-rb/impl/data_system/streaming.rb index ad00a129..8814ccf2 100644 --- a/lib/ldclient-rb/impl/data_system/streaming.rb +++ b/lib/ldclient-rb/impl/data_system/streaming.rb @@ -47,6 +47,11 @@ def initialize(sdk_key, http_config, initial_reconnect_delay, config) @stopped = Concurrent::Event.new @diagnostic_accumulator = nil @connection_attempt_start_time = 0 + # Set when the SSE connect handshake carries the X-LD-FD-Fallback + # directive. We finish applying the current payload before signalling + # fallback to the consumer, so evaluations can serve the streaming + # data while FDv1 takes over. + @fdv1_fallback_pending = false end # @@ -85,20 +90,19 @@ def sync(ss) @sse = SSE::Client.new(base_uri, **opts) do |client| client.on_connect do |headers| - # Extract environment ID and check for fallback on successful connection + # Extract environment ID and check for fallback on successful connection. if headers - envid = headers[LD_ENVID_HEADER] || envid - - # Check for fallback header on connection - if LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers) - log_connection_result(true) - yield LaunchDarkly::Interfaces::DataSystem::Update.new( - state: LaunchDarkly::Interfaces::DataSource::Status::OFF, - fallback_to_fdv1: true, - environment_id: envid - ) - stop - end + envid = LaunchDarkly::Impl::DataSystem.lookup_header(headers, LD_ENVID_HEADER) || envid + + # When the server signals the FDv1 Fallback Directive on a 200 + # SSE handshake the spec requires us to apply any payload that + # arrives on this stream BEFORE handing off to FDv1 + # (Requirement 1.6.2). We therefore record the directive here + # but stay subscribed; when the next payload-transferred event + # produces a Valid update, that update carries + # `fallback_to_fdv1: true` and the consumer transitions only + # after the payload has been applied. + @fdv1_fallback_pending = true if LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers) end end @@ -108,6 +112,26 @@ def sync(ss) if update log_connection_result(true) @connection_attempt_start_time = 0 + + # If the connection carried the FDv1 Fallback Directive, ride + # it on this Valid update. The consumer applies the + # ChangeSet first, then transitions to FDv1 -- so we close + # the stream after yielding to honor 1.6.3(2) (Primary + # Synchronizer must be stopped once the directive engages). + if @fdv1_fallback_pending && update.state == LaunchDarkly::Interfaces::DataSource::Status::VALID + update = LaunchDarkly::Interfaces::DataSystem::Update.new( + state: update.state, + change_set: update.change_set, + error: update.error, + environment_id: update.environment_id || envid, + fallback_to_fdv1: true + ) + @fdv1_fallback_pending = false + yield update + stop + next + end + yield update end rescue JSON::ParserError => e @@ -149,7 +173,7 @@ def sync(ss) # Extract envid and fallback from error headers if available if error.respond_to?(:headers) && error.headers - envid = error.headers[LD_ENVID_HEADER] || envid + envid = LaunchDarkly::Impl::DataSystem.lookup_header(error.headers, LD_ENVID_HEADER) || envid fallback = true if LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(error.headers) end diff --git a/spec/impl/data_system/polling_initializer_spec.rb b/spec/impl/data_system/polling_initializer_spec.rb index 8a27dd97..98ba3a0d 100644 --- a/spec/impl/data_system/polling_initializer_spec.rb +++ b/spec/impl/data_system/polling_initializer_spec.rb @@ -7,6 +7,54 @@ module LaunchDarkly module Impl module DataSystem + RSpec.describe ".fdv1_fallback_requested?" do + it "matches the canonical mixed-case header" do + headers = { 'X-LD-FD-Fallback' => 'true' } + expect(LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers)).to be true + end + + it "matches the downcased header (HTTPPollingRequester#fetch normalizes casing)" do + headers = { 'x-ld-fd-fallback' => 'true' } + expect(LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers)).to be true + end + + it "matches arbitrary mixed-case header keys" do + headers = { 'X-Ld-Fd-Fallback' => 'true' } + expect(LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers)).to be true + end + + it "returns false when the header is absent" do + expect(LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?({})).to be false + end + + it "returns false when the header value is not 'true'" do + headers = { 'X-LD-FD-Fallback' => 'false' } + expect(LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers)).to be false + end + + it "returns false when the headers object is nil" do + expect(LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(nil)).to be false + end + + it "works against case-insensitive containers (HTTP::Headers shape)" do + # The ld-eventsource gem hands us an HTTP::Headers instance whose [] + # accessor is case-insensitive but which does not implement + # each_pair. Simulate that shape so the helper is exercised against + # exactly the API surface that broke contract tests on PR #381. + ci_container = Class.new do + def initialize(values) + @values = values + end + + def [](name) + @values[name.to_s.downcase] + end + end + headers = ci_container.new('x-ld-fd-fallback' => 'true') + expect(LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers)).to be true + end + end + RSpec.describe PollingDataSource do let(:logger) { double("Logger", info: nil, warn: nil, error: nil, debug: nil) } @@ -172,6 +220,42 @@ def selector expect(fetch_result.fallback_to_fdv1).to be true end + it "honors the fallback header regardless of case" do + # The HTTPPollingRequester downcases response header keys before + # handing them off, but other code paths (and other HTTP clients) + # may keep the canonical mixed case. Header lookup must be + # case-insensitive or the directive silently disappears against a + # perfectly valid response -- this is the bug that the contract + # tests caught against the initializer-phase fix. + change_set = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes + headers = { 'x-ld-fd-fallback' => 'true' } # downcased -- mirrors HTTPPollingRequester + mock_requester = MockPollingRequester.new( + LaunchDarkly::Result.success([change_set, headers]) + ) + ds = PollingDataSource.new(1.0, mock_requester, logger) + + fetch_result = ds.fetch(MockSelectorStore.new(LaunchDarkly::Interfaces::DataSystem::Selector.no_selector)) + + expect(fetch_result.success?).to be true + expect(fetch_result.fallback_to_fdv1).to be true + end + + it "honors the fallback header on error responses with downcased keys" do + headers_with_fallback = { 'x-ld-fd-fallback' => 'true' } + error_result = LaunchDarkly::Result.fail( + "failure message", + LaunchDarkly::Impl::DataSource::UnexpectedResponseError.new(500), + headers_with_fallback + ) + mock_requester = MockPollingRequester.new(error_result) + ds = PollingDataSource.new(1.0, mock_requester, logger) + + fetch_result = ds.fetch(MockSelectorStore.new(LaunchDarkly::Interfaces::DataSystem::Selector.no_selector)) + + expect(fetch_result.success?).to be false + expect(fetch_result.fallback_to_fdv1).to be true + end + it "reports fallback_to_fdv1 as false when the header is absent on error" do mock_requester = MockPollingRequester.new( LaunchDarkly::Result.fail( From 8a708b358c3841e0e601bad647881d6a52558db7 Mon Sep 17 00:00:00 2001 From: Matthew Keeler Date: Thu, 30 Apr 2026 15:54:52 -0400 Subject: [PATCH 4/4] refactor: simplify directive plumbing per code review - Unify response-header handling on Result. Result.success now accepts optional headers, HTTPPollingRequester / HTTPFDv1PollingRequester pass headers via Result.headers on both success and failure, and PollingDataSource consumes them through a single result.headers lookup. The dual "headers ride on Result.headers on failure / on the value tuple on success" shape is gone. - Drop the FetchResult/Result legacy guard in FDv2.run_initializers. All in-tree initializers produce a FetchResult; the data system can just call .result and .fallback_to_fdv1 directly. - Streaming: move @fdv1_fallback_pending out of instance state into a local closed over by on_connect and on_event. The directive arrives in the connect handshake but on_event has no access to those headers, so some state has to bridge the two callbacks; a local scoped to a single #sync invocation expresses that lifecycle correctly without leaking across reconnects. process_message is now the only place that constructs Update -- it accepts an optional fdv1_fallback_pending: kwarg and stamps fallback_to_fdv1 on the Update it produces for events that complete a payload transfer (TRANSFER_NONE or PAYLOAD_TRANSFERRED), so on_event can simply yield the Update and stop the stream when the directive rode along. Flip envid lookups to ||= so we skip the case-insensitive scan once envid is known (it never changes server-side). - Drop the deprecated revert_to_fdv1 alias on Update. FDv2 is in EAP and we don't need to preserve compatibility with the old name. - Comment cleanup: drop remaining spec-section/requirement citations while preserving the behavioral descriptions, and tighten the return fallback ? true : false to return fallback. --- .../polling_data_source_builder.rb | 12 ++- lib/ldclient-rb/impl/data_system.rb | 4 - lib/ldclient-rb/impl/data_system/fdv2.rb | 30 +++--- lib/ldclient-rb/impl/data_system/polling.rb | 48 +++------- lib/ldclient-rb/impl/data_system/streaming.rb | 94 +++++++++---------- lib/ldclient-rb/interfaces/data_system.rb | 6 -- lib/ldclient-rb/util.rb | 5 +- .../data_system/polling_initializer_spec.rb | 16 ++-- .../data_system/polling_synchronizer_spec.rb | 26 ++--- 9 files changed, 105 insertions(+), 136 deletions(-) diff --git a/lib/ldclient-rb/data_system/polling_data_source_builder.rb b/lib/ldclient-rb/data_system/polling_data_source_builder.rb index e68a88db..e8bf4815 100644 --- a/lib/ldclient-rb/data_system/polling_data_source_builder.rb +++ b/lib/ldclient-rb/data_system/polling_data_source_builder.rb @@ -21,9 +21,9 @@ module DataSystem # include LaunchDarkly::DataSystem::Requester # # def fetch(selector) - # # Fetch data and return a Result containing [ChangeSet, headers] - # # ... - # LaunchDarkly::Result.success([change_set, {}]) + # # Fetch data and return a Result whose value is a ChangeSet and + # # whose headers carry any response metadata (e.g. directives). + # LaunchDarkly::Result.success(change_set, response_headers) # end # # def stop @@ -43,8 +43,10 @@ module Requester # @param selector [LaunchDarkly::Interfaces::DataSystem::Selector, nil] # The selector describing what data to fetch. May be nil if no # selector is available (e.g., on the first request). - # @return [LaunchDarkly::Result] A Result containing a tuple of - # [ChangeSet, headers] on success, or an error message on failure. + # @return [LaunchDarkly::Result] A Result whose `value` is a + # {LaunchDarkly::Interfaces::DataSystem::ChangeSet} on success and + # whose `headers` carry response headers in either case (so callers + # can inspect directives such as `X-LD-FD-Fallback`). # def fetch(selector) raise NotImplementedError diff --git a/lib/ldclient-rb/impl/data_system.rb b/lib/ldclient-rb/impl/data_system.rb index 40100731..62ee272c 100644 --- a/lib/ldclient-rb/impl/data_system.rb +++ b/lib/ldclient-rb/impl/data_system.rb @@ -268,10 +268,6 @@ def initialize(state:, change_set: nil, error: nil, fallback_to_fdv1: false, env @fallback_to_fdv1 = fallback_to_fdv1 @environment_id = environment_id end - - # Deprecated alias retained for backwards compatibility while FDv2 is in early access. - # @deprecated Prefer {#fallback_to_fdv1}. - alias_method :revert_to_fdv1, :fallback_to_fdv1 end # diff --git a/lib/ldclient-rb/impl/data_system/fdv2.rb b/lib/ldclient-rb/impl/data_system/fdv2.rb index 4d4cb287..edd016f0 100644 --- a/lib/ldclient-rb/impl/data_system/fdv2.rb +++ b/lib/ldclient-rb/impl/data_system/fdv2.rb @@ -264,27 +264,22 @@ def run_initializers @logger.info { "[LDClient] Attempting to initialize via #{initializer.name}" } fetch_result = initializer.fetch(@store) - fallback = fetch_result.respond_to?(:fallback_to_fdv1) && fetch_result.fallback_to_fdv1 - # Support legacy implementations that return a bare Result. - basis_result = fetch_result.respond_to?(:result) ? fetch_result.result : fetch_result + fallback = fetch_result.fallback_to_fdv1 + basis_result = fetch_result.result if basis_result.success? basis = basis_result.value @logger.info { "[LDClient] Initialized via #{initializer.name}" } - # Apply the basis to the store regardless of whether - # fallback was signalled -- if the server returned a valid - # payload alongside the directive we still want evaluations - # to serve that data. + # Apply the basis to the store regardless of whether fallback was signalled. + # If the server returned a valid payload alongside the directive we still want + # evaluations to serve that data while the FDv1 synchronizer spins up. @store.apply(basis.change_set, basis.persist) # Set ready event if and only if a selector is defined for the changeset. - # Even when fallback is requested, the payload that arrived with the directive - # has been applied to the store, so evaluations can serve it while the FDv1 - # synchronizer spins up. if basis.change_set.selector && basis.change_set.selector.defined? @ready_event.set - return fallback ? true : false + return fallback end else @logger.warn { "[LDClient] Initializer #{initializer.name} failed: #{basis_result.error}" } @@ -303,9 +298,8 @@ def run_initializers end end - # Honor the FDv1 Fallback Directive even on an error or undefined-selector path: - # the directive takes precedence over the regular failover algorithm, so we must - # not fall through to the next initializer. + # The fallback directive takes precedence over the regular failover algorithm, + # so do not fall through to the next initializer when it is set. return true if fallback rescue => e @logger.error { "[LDClient] Initializer failed with exception: #{e.message}" } @@ -370,10 +364,10 @@ def synchronizer_loop current_index = 0 next end - # No FDv1 fallback configured: per spec section 1.6.3(4) the - # data system must HALT rather than fall through to the next - # FDv2 synchronizer. Continuing to retry would reopen the - # connection that just delivered the directive. + # No FDv1 fallback configured: the data system must HALT + # rather than fall through to the next FDv2 synchronizer. + # Continuing to retry would reopen the connection that just + # delivered the directive. @logger.warn { "[LDClient] Synchronizer requested FDv1 fallback but none configured; halting data system" } @data_source_status_provider.update_status( LaunchDarkly::Interfaces::DataSource::Status::OFF, diff --git a/lib/ldclient-rb/impl/data_system/polling.rb b/lib/ldclient-rb/impl/data_system/polling.rb index 25672598..769cc3d7 100644 --- a/lib/ldclient-rb/impl/data_system/polling.rb +++ b/lib/ldclient-rb/impl/data_system/polling.rb @@ -23,14 +23,10 @@ module DataSystem # # Reports whether the response headers signal that the SDK should fall - # back to the FDv1 protocol. - # - # The header lookup is case-insensitive: response header maps in this - # codebase arrive in three flavours -- the http gem's case-insensitive - # `HTTP::Headers`, plain Ruby Hashes whose keys we have downcased - # (e.g. {HTTPPollingRequester#fetch}), and Hashes with the canonical - # mixed-case key. Lookups in only one of those casings would silently - # drop the directive against a perfectly valid response. + # back to the FDv1 protocol. Lookup is case-insensitive so callers do + # not need to know whether the header map preserves canonical casing + # (e.g. {HTTP::Headers}) or has been normalized to lowercase (e.g. + # {HTTPPollingRequester#fetch}). # # @param headers [#[], Hash, nil] # @return [Boolean] @@ -131,11 +127,10 @@ def sync(ss) until @stop.set? result = @requester.fetch(ss.selector) + fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(result.headers) + envid = LaunchDarkly::Impl::DataSystem.lookup_header(result.headers, LD_ENVID_HEADER) if !result.success? - fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(result.headers) - envid = result.headers ? LaunchDarkly::Impl::DataSystem.lookup_header(result.headers, LD_ENVID_HEADER) : nil - if result.exception.is_a?(LaunchDarkly::Impl::DataSource::UnexpectedResponseError) error_info = LaunchDarkly::Interfaces::DataSource::ErrorInfo.new( LaunchDarkly::Interfaces::DataSource::ErrorInfo::ERROR_RESPONSE, @@ -203,12 +198,10 @@ def sync(ss) fallback_to_fdv1: false ) else - change_set, headers = result.value - fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers) yield LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, - change_set: change_set, - environment_id: LaunchDarkly::Impl::DataSystem.lookup_header(headers, LD_ENVID_HEADER), + change_set: result.value, + environment_id: envid, fallback_to_fdv1: fallback ) end @@ -236,17 +229,7 @@ def stop # private def poll(ss) result = @requester.fetch(ss.selector) - - # On success, the requester returns headers as the second element of the value tuple; - # on failure, headers ride on Result.headers. Check both so the fallback signal is - # surfaced regardless of outcome. - response_headers = nil - if result.success? - _, response_headers = result.value - else - response_headers = result.headers - end - fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(response_headers) + fallback = LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(result.headers) unless result.success? if result.exception.is_a?(LaunchDarkly::Impl::DataSource::UnexpectedResponseError) @@ -267,9 +250,8 @@ def stop ) end - change_set, headers = result.value - - env_id = LaunchDarkly::Impl::DataSystem.lookup_header(headers, LD_ENVID_HEADER) + change_set = result.value + env_id = LaunchDarkly::Impl::DataSystem.lookup_header(result.headers, LD_ENVID_HEADER) env_id = nil unless env_id.is_a?(String) basis = LaunchDarkly::Interfaces::DataSystem::Basis.new( @@ -351,7 +333,7 @@ def fetch(selector) end if status == 304 - return LaunchDarkly::Result.success([LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes, response_headers]) + return LaunchDarkly::Result.success(LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes, response_headers) end body = response.to_s @@ -363,7 +345,7 @@ def fetch(selector) changeset_result = LaunchDarkly::Impl::DataSystem.polling_payload_to_changeset(data) if changeset_result.success? - LaunchDarkly::Result.success([changeset_result.value, response_headers]) + LaunchDarkly::Result.success(changeset_result.value, response_headers) else LaunchDarkly::Result.fail(changeset_result.error, changeset_result.exception, response_headers) end @@ -439,7 +421,7 @@ def fetch(selector) end if status == 304 - return LaunchDarkly::Result.success([LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes, response_headers]) + return LaunchDarkly::Result.success(LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes, response_headers) end body = response.to_s @@ -451,7 +433,7 @@ def fetch(selector) changeset_result = LaunchDarkly::Impl::DataSystem.fdv1_polling_payload_to_changeset(data) if changeset_result.success? - LaunchDarkly::Result.success([changeset_result.value, response_headers]) + LaunchDarkly::Result.success(changeset_result.value, response_headers) else LaunchDarkly::Result.fail(changeset_result.error, changeset_result.exception, response_headers) end diff --git a/lib/ldclient-rb/impl/data_system/streaming.rb b/lib/ldclient-rb/impl/data_system/streaming.rb index 8814ccf2..4c3aa62e 100644 --- a/lib/ldclient-rb/impl/data_system/streaming.rb +++ b/lib/ldclient-rb/impl/data_system/streaming.rb @@ -47,11 +47,6 @@ def initialize(sdk_key, http_config, initial_reconnect_delay, config) @stopped = Concurrent::Event.new @diagnostic_accumulator = nil @connection_attempt_start_time = 0 - # Set when the SSE connect handshake carries the X-LD-FD-Fallback - # directive. We finish applying the current payload before signalling - # fallback to the consumer, so evaluations can serve the streaming - # data while FDv1 takes over. - @fdv1_fallback_pending = false end # @@ -77,6 +72,21 @@ def sync(ss) change_set_builder = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.new envid = nil + # The directive arrives in the connect-time response headers (handled + # by on_connect) but the SDK must apply the next full payload before + # transitioning. on_event has no access to the connect headers, so + # this local closes over both callbacks to bridge the lifecycle of a + # single sync invocation. A local (rather than an instance variable) + # is the right shape for two reasons: + # 1. Lifecycle -- it is scoped to one sync invocation and cannot + # leak state across reconnects or to a future sync call. + # 2. Thread safety -- ld-eventsource guarantees on_connect, + # on_event, and on_error all dispatch on the same SSE worker + # thread, so reads and writes here are single-threaded by + # construction. No atomic / mutex needed even on JRuby. An + # instance variable would be vulnerable to cross-thread reads + # if some future caller queried state from another thread. + fdv1_fallback_pending = false base_uri = @http_config.base_uri + FDV2_STREAMING_ENDPOINT headers = Impl::Util.default_http_headers(@sdk_key, @config) @@ -90,49 +100,32 @@ def sync(ss) @sse = SSE::Client.new(base_uri, **opts) do |client| client.on_connect do |headers| - # Extract environment ID and check for fallback on successful connection. if headers - envid = LaunchDarkly::Impl::DataSystem.lookup_header(headers, LD_ENVID_HEADER) || envid - - # When the server signals the FDv1 Fallback Directive on a 200 - # SSE handshake the spec requires us to apply any payload that - # arrives on this stream BEFORE handing off to FDv1 - # (Requirement 1.6.2). We therefore record the directive here - # but stay subscribed; when the next payload-transferred event - # produces a Valid update, that update carries - # `fallback_to_fdv1: true` and the consumer transitions only - # after the payload has been applied. - @fdv1_fallback_pending = true if LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers) + # Per-environment identifier: server sends it on every connect, + # but it never changes once known so only assign once. + envid ||= LaunchDarkly::Impl::DataSystem.lookup_header(headers, LD_ENVID_HEADER) + fdv1_fallback_pending = true if LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(headers) end end client.on_event do |event| begin - update = process_message(event, change_set_builder, envid) - if update - log_connection_result(true) - @connection_attempt_start_time = 0 - - # If the connection carried the FDv1 Fallback Directive, ride - # it on this Valid update. The consumer applies the - # ChangeSet first, then transitions to FDv1 -- so we close - # the stream after yielding to honor 1.6.3(2) (Primary - # Synchronizer must be stopped once the directive engages). - if @fdv1_fallback_pending && update.state == LaunchDarkly::Interfaces::DataSource::Status::VALID - update = LaunchDarkly::Interfaces::DataSystem::Update.new( - state: update.state, - change_set: update.change_set, - error: update.error, - environment_id: update.environment_id || envid, - fallback_to_fdv1: true - ) - @fdv1_fallback_pending = false - yield update - stop - next - end - - yield update + update = process_message(event, change_set_builder, envid, fdv1_fallback_pending: fdv1_fallback_pending) + next unless update + + log_connection_result(true) + @connection_attempt_start_time = 0 + + yield update + + # When the FDv1 Fallback Directive rode along on a Valid update, close + # the stream so the primary synchronizer is stopped once the directive + # engages. process_message marks the Update with fallback_to_fdv1 only + # on payloads that complete a transfer, so the consumer has already + # applied the ChangeSet by the time we get here. + if update.fallback_to_fdv1 + fdv1_fallback_pending = false + stop end rescue JSON::ParserError => e @logger.info { "[LDClient] Error parsing stream event; will restart stream: #{e}" } @@ -171,9 +164,8 @@ def sync(ss) log_connection_result(false) fallback = false - # Extract envid and fallback from error headers if available if error.respond_to?(:headers) && error.headers - envid = LaunchDarkly::Impl::DataSystem.lookup_header(error.headers, LD_ENVID_HEADER) || envid + envid ||= LaunchDarkly::Impl::DataSystem.lookup_header(error.headers, LD_ENVID_HEADER) fallback = true if LaunchDarkly::Impl::DataSystem.fdv1_fallback_requested?(error.headers) end @@ -214,9 +206,15 @@ def stop # @param message [SSE::StreamEvent] # @param change_set_builder [LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder] # @param envid [String, nil] + # @param fdv1_fallback_pending [Boolean] true when the connect-time + # response headers carried the FDv1 Fallback Directive. When set, + # the next Update that completes a payload transfer (TRANSFER_NONE + # or PAYLOAD_TRANSFERRED) is marked with fallback_to_fdv1: true so + # the consumer can engage the FDv1 Fallback Synchronizer after + # applying the in-flight ChangeSet. # @return [LaunchDarkly::Interfaces::DataSystem::Update, nil] # - private def process_message(message, change_set_builder, envid) + private def process_message(message, change_set_builder, envid, fdv1_fallback_pending: false) event_type = message.type # Handle heartbeat @@ -235,7 +233,8 @@ def stop change_set_builder.expect_changes return LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, - environment_id: envid + environment_id: envid, + fallback_to_fdv1: fdv1_fallback_pending ) end nil @@ -272,7 +271,8 @@ def stop LaunchDarkly::Interfaces::DataSystem::Update.new( state: LaunchDarkly::Interfaces::DataSource::Status::VALID, change_set: change_set, - environment_id: envid + environment_id: envid, + fallback_to_fdv1: fdv1_fallback_pending ) else diff --git a/lib/ldclient-rb/interfaces/data_system.rb b/lib/ldclient-rb/interfaces/data_system.rb index 5fa3337a..72e52e56 100644 --- a/lib/ldclient-rb/interfaces/data_system.rb +++ b/lib/ldclient-rb/interfaces/data_system.rb @@ -580,12 +580,6 @@ def initialize(state:, change_set: nil, error: nil, fallback_to_fdv1: false, env @fallback_to_fdv1 = fallback_to_fdv1 @environment_id = environment_id end - - # Deprecated alias retained so that existing callers continue to work - # while the FDv2 data system is in early access. Prefer - # {#fallback_to_fdv1}. - # @deprecated - alias_method :revert_to_fdv1, :fallback_to_fdv1 end # diff --git a/lib/ldclient-rb/util.rb b/lib/ldclient-rb/util.rb index 206e4788..c596f8d7 100644 --- a/lib/ldclient-rb/util.rb +++ b/lib/ldclient-rb/util.rb @@ -16,10 +16,11 @@ class Result # Create a successful result with the provided value. # # @param value [Object, nil] + # @param headers [Hash, nil] Optional headers associated with the result # @return [Result] # - def self.success(value) - Result.new(value) + def self.success(value, headers = nil) + Result.new(value, nil, nil, headers) end # diff --git a/spec/impl/data_system/polling_initializer_spec.rb b/spec/impl/data_system/polling_initializer_spec.rb index 98ba3a0d..d21a0ed4 100644 --- a/spec/impl/data_system/polling_initializer_spec.rb +++ b/spec/impl/data_system/polling_initializer_spec.rb @@ -141,7 +141,7 @@ def selector it "handles transfer none" do mock_requester = MockPollingRequester.new( - LaunchDarkly::Result.success([LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes, {}]) + LaunchDarkly::Result.success(LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes, {}) ) ds = PollingDataSource.new(1.0, mock_requester, logger) @@ -170,7 +170,7 @@ def selector change_set_result = LaunchDarkly::Impl::DataSystem.polling_payload_to_changeset(JSON.parse(payload_str, symbolize_names: true)) expect(change_set_result.success?).to eq(true) - mock_requester = MockPollingRequester.new(LaunchDarkly::Result.success([change_set_result.value, {}])) + mock_requester = MockPollingRequester.new(LaunchDarkly::Result.success(change_set_result.value, {})) ds = PollingDataSource.new(1.0, mock_requester, logger) result = ds.fetch(MockSelectorStore.new(LaunchDarkly::Interfaces::DataSystem::Selector.no_selector)) @@ -183,13 +183,13 @@ def selector end it "surfaces fallback_to_fdv1 on a successful response with the fallback header" do - # Server-directed FDv1 Fallback Directive may ride along on a 200 response that also - # carries a valid payload. The SDK must apply the payload AND surface the fallback - # signal so the data system can transition to the FDv1 Fallback Synchronizer. + # The fallback directive may ride along on a 200 response that also carries a valid + # payload. The SDK must apply the payload AND surface the fallback signal so the + # data system can transition to the FDv1 Fallback Synchronizer. change_set = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes headers = { LD_FD_FALLBACK_HEADER => 'true' } mock_requester = MockPollingRequester.new( - LaunchDarkly::Result.success([change_set, headers]) + LaunchDarkly::Result.success(change_set, headers) ) ds = PollingDataSource.new(1.0, mock_requester, logger) @@ -230,7 +230,7 @@ def selector change_set = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes headers = { 'x-ld-fd-fallback' => 'true' } # downcased -- mirrors HTTPPollingRequester mock_requester = MockPollingRequester.new( - LaunchDarkly::Result.success([change_set, headers]) + LaunchDarkly::Result.success(change_set, headers) ) ds = PollingDataSource.new(1.0, mock_requester, logger) @@ -276,7 +276,7 @@ def selector change_set_result = LaunchDarkly::Impl::DataSystem.polling_payload_to_changeset(JSON.parse(payload_str, symbolize_names: true)) expect(change_set_result.success?).to eq(true) - mock_requester = MockPollingRequester.new(LaunchDarkly::Result.success([change_set_result.value, {}])) + mock_requester = MockPollingRequester.new(LaunchDarkly::Result.success(change_set_result.value, {})) ds = PollingDataSource.new(1.0, mock_requester, logger) result = ds.fetch(MockSelectorStore.new(LaunchDarkly::Interfaces::DataSystem::Selector.no_selector)) diff --git a/spec/impl/data_system/polling_synchronizer_spec.rb b/spec/impl/data_system/polling_synchronizer_spec.rb index 19edc130..b2538843 100644 --- a/spec/impl/data_system/polling_synchronizer_spec.rb +++ b/spec/impl/data_system/polling_synchronizer_spec.rb @@ -59,7 +59,7 @@ def selector it "handles no changes" do change_set = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes headers = {} - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) synchronizer = PollingDataSource.new(0.01, ListBasedRequester.new([polling_result]), logger) updates = [] @@ -91,7 +91,7 @@ def selector builder.start(LaunchDarkly::Interfaces::DataSystem::IntentCode::TRANSFER_FULL) change_set = builder.finish(LaunchDarkly::Interfaces::DataSystem::Selector.new(state: "p:SOMETHING:300", version: 300)) headers = {} - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) synchronizer = PollingDataSource.new(0.01, ListBasedRequester.new([polling_result]), logger) updates = [] @@ -132,7 +132,7 @@ def selector ) change_set = builder.finish(LaunchDarkly::Interfaces::DataSystem::Selector.new(state: "p:SOMETHING:300", version: 300)) headers = {} - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) synchronizer = PollingDataSource.new(0.01, ListBasedRequester.new([polling_result]), logger) updates = [] @@ -173,7 +173,7 @@ def selector builder.add_delete(LaunchDarkly::Interfaces::DataSystem::ObjectKind::FLAG, :flagkey, 101) change_set = builder.finish(LaunchDarkly::Interfaces::DataSystem::Selector.new(state: "p:SOMETHING:300", version: 300)) headers = {} - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) synchronizer = PollingDataSource.new(0.01, ListBasedRequester.new([polling_result]), logger) updates = [] @@ -213,7 +213,7 @@ def selector builder.add_delete(LaunchDarkly::Interfaces::DataSystem::ObjectKind::FLAG, "flagkey", 101) change_set = builder.finish(LaunchDarkly::Interfaces::DataSystem::Selector.new(state: "p:SOMETHING:300", version: 300)) headers = {} - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) synchronizer = PollingDataSource.new( 0.01, @@ -259,7 +259,7 @@ def selector builder.add_delete(LaunchDarkly::Interfaces::DataSystem::ObjectKind::FLAG, "flagkey", 101) change_set = builder.finish(LaunchDarkly::Interfaces::DataSystem::Selector.new(state: "p:SOMETHING:300", version: 300)) headers = {} - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) failure = LaunchDarkly::Result.fail( "error for test", @@ -341,7 +341,7 @@ def selector it "captures envid from success headers" do change_set = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes headers = { LD_ENVID_HEADER => 'test-env-polling-123' } - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) synchronizer = PollingDataSource.new(0.01, ListBasedRequester.new([polling_result]), logger) updates = [] @@ -379,7 +379,7 @@ def selector LD_ENVID_HEADER => 'test-env-456', LD_FD_FALLBACK_HEADER => 'true', } - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) synchronizer = PollingDataSource.new(0.01, ListBasedRequester.new([polling_result]), logger) updates = [] @@ -410,7 +410,7 @@ def selector builder.add_delete(LaunchDarkly::Interfaces::DataSystem::ObjectKind::FLAG, "flagkey", 101) change_set = builder.finish(LaunchDarkly::Interfaces::DataSystem::Selector.new(state: "p:SOMETHING:300", version: 300)) headers_success = { LD_ENVID_HEADER => 'test-env-success' } - polling_result = LaunchDarkly::Result.success([change_set, headers_success]) + polling_result = LaunchDarkly::Result.success(change_set, headers_success) headers_error = { LD_ENVID_HEADER => 'test-env-408' } failure = LaunchDarkly::Result.fail( @@ -523,7 +523,7 @@ def selector builder.start(LaunchDarkly::Interfaces::DataSystem::IntentCode::TRANSFER_FULL) change_set = builder.finish(LaunchDarkly::Interfaces::DataSystem::Selector.new(state: "p:SOMETHING:300", version: 300)) headers_success = {} - polling_result = LaunchDarkly::Result.success([change_set, headers_success]) + polling_result = LaunchDarkly::Result.success(change_set, headers_success) headers_error = { LD_ENVID_HEADER => 'test-env-generic' } failure = LaunchDarkly::Result.fail("generic error for test", nil, headers_error) @@ -650,7 +650,7 @@ def selector } # Server sends successful response with valid data but also signals fallback - success_result = LaunchDarkly::Result.success([change_set, headers_with_fallback]) + success_result = LaunchDarkly::Result.success(change_set, headers_with_fallback) synchronizer = PollingDataSource.new( 0.01, @@ -683,7 +683,7 @@ def selector it "closes requester when sync exits" do change_set = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes headers = {} - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) requester = RequesterWithCleanup.new([polling_result]) synchronizer = PollingDataSource.new(0.01, requester, logger) @@ -703,7 +703,7 @@ def selector it "closes requester when fetch is called" do change_set = LaunchDarkly::Interfaces::DataSystem::ChangeSetBuilder.no_changes headers = {} - polling_result = LaunchDarkly::Result.success([change_set, headers]) + polling_result = LaunchDarkly::Result.success(change_set, headers) requester = RequesterWithCleanup.new([polling_result]) synchronizer = PollingDataSource.new(0.01, requester, logger)