fix: Ask the override layer for the key before the client has LaunchDarkly data

Before initialization the clients consult the override layer directly. A flag the layer does not hold gets the not-ready handling it had without an override source, for an invalid context, a store outage, and an uninitialized persistent store alike. The all-flags state is unavailable when the layer is empty. The sync client stops the components it started when the data system fails to start.

Author: kinyoklion

ldclient/async_client.py

@@ -505,36 +505,37 @@ def not_ready() -> Tuple[EvaluationDetail, Optional[FeatureFlag]]:
             self._send_event(event_factory.new_unknown_flag_event(key, context, default, reason))
             return EvaluationDetail(default, None, reason), None
 
-        no_launchdarkly_data = False
+        flag: Any = None
         availability = await self._data_system.data_availability()
         if availability != DataAvailability.REFRESHED:
             if availability == DataAvailability.CACHED:
                 if not self._eval_cached_data_warned:
                     self._eval_cached_data_warned = True
                     log.warning("Feature Flag evaluation attempted before client has initialized - using last known values from feature store for feature key: " + key + ". This message is logged once.")
-            elif self._data_system.override_source_configured:
-                # No data from LaunchDarkly is available. The store read below still finds an
-                # entry that the override layer holds, and the SDK serves it. A miss returns the
-                # not-ready default.
-                no_launchdarkly_data = True
             else:
-                return not_ready()
+                # No data from LaunchDarkly is available. A flag that the override layer holds
+                # is served anyway. A flag the layer does not hold gets the not-ready default,
+                # as it does without an override source.
+                layer = self._data_system.override_layer
+                if layer is not None:
+                    flag = layer.get(FEATURES, key)
+                if flag is None:
+                    return not_ready()
 
         if not context.valid:
             log.warning("Context was invalid for flag evaluation (%s); returning default value" % context.error)
             return EvaluationDetail(default, None, error_reason('USER_NOT_SPECIFIED')), None
 
-        try:
-            flag = await self._data_system.store.get(FEATURES, key)
-        except Exception as e:
-            log.error("Unexpected error while retrieving feature flag \"%s\": %s" % (key, repr(e)))
-            log.debug(traceback.format_exc())
-            reason = error_reason('EXCEPTION')
-            self._send_event(event_factory.new_unknown_flag_event(key, context, default, reason))
-            return EvaluationDetail(default, None, reason), None
+        if flag is None:
+            try:
+                flag = await self._data_system.store.get(FEATURES, key)
+            except Exception as e:
+                log.error("Unexpected error while retrieving feature flag \"%s\": %s" % (key, repr(e)))
+                log.debug(traceback.format_exc())
+                reason = error_reason('EXCEPTION')
+                self._send_event(event_factory.new_unknown_flag_event(key, context, default, reason))
+                return EvaluationDetail(default, None, reason), None
         if not flag:
-            if no_launchdarkly_data:
-                return not_ready()
             reason = error_reason('FLAG_NOT_FOUND')
             self._send_event(event_factory.new_unknown_flag_event(key, context, default, reason))
             return EvaluationDetail(default, None, reason), None
@@ -591,13 +592,15 @@ async def all_flags_state(self, context: Context, **kwargs) -> FeatureFlagsState
                 if not self._all_flags_cached_data_warned:
                     self._all_flags_cached_data_warned = True
                     log.warning("all_flags_state() called before client has finished initializing! Using last known values from feature store. This message is logged once.")
-            elif self._data_system.override_source_configured:
-                # No data from LaunchDarkly is available. The store read below returns only the
-                # entries that the override layer holds. The result decides the state.
-                overrides_only = True
             else:
-                log.warning("all_flags_state() called before client has finished initializing! Feature store unavailable - returning empty state")
-                return FeatureFlagsState(False)
+                # No data from LaunchDarkly is available. When the override layer holds entries,
+                # the state is built from them alone. Otherwise the state is unavailable, as it
+                # is without an override source.
+                layer = self._data_system.override_layer
+                if layer is None or layer.is_empty:
+                    log.warning("all_flags_state() called before client has finished initializing! Feature store unavailable - returning empty state")
+                    return FeatureFlagsState(False)
+                overrides_only = True
 
         if not context.valid:
             log.warning("Context was invalid for all_flags_state (%s); returning default value" % context.error)

ldclient/client.py

@@ -212,7 +212,13 @@ def __start_up(self, start_wait: float):
         self.__register_plugins(environment_metadata)
 
         update_processor_ready = threading.Event()
-        self._data_system.start(update_processor_ready)
+        try:
+            self._data_system.start(update_processor_ready)
+        except BaseException:
+            # A data system that fails to start fails the constructor. The components that
+            # are already running stop here, so the failure leaves no threads behind.
+            self._close_components()
+            raise
 
         if not self._config.offline and not self._config.use_ldd:
             if start_wait > 60:
@@ -282,6 +288,15 @@ def close(self):
         self._data_system.stop()
         self.__big_segment_store_manager.stop()
 
+    def _close_components(self):
+        """
+        Stops the SDK components after a failed start-up. The event processor is last because
+        it may still be sending events the other components generated.
+        """
+        self._data_system.stop()
+        self.__big_segment_store_manager.stop()
+        self._event_processor.stop()
+
     # These magic methods allow a client object to be automatically cleaned up by the "with" scope operator
     def __enter__(self):
         return self
@@ -474,7 +489,7 @@ def not_ready() -> Tuple[EvaluationDetail, Optional[FeatureFlag]]:
             self._send_event(event_factory.new_unknown_flag_event(key, context, default, reason))
             return EvaluationDetail(default, None, reason), None
 
-        no_launchdarkly_data = False
+        flag: Any = None
         availability = self._data_system.data_availability
         if availability != DataAvailability.REFRESHED:
             if availability == DataAvailability.CACHED:
@@ -483,29 +498,30 @@ def not_ready() -> Tuple[EvaluationDetail, Optional[FeatureFlag]]:
                         if not self._eval_cached_data_warned:
                             self._eval_cached_data_warned = True
                             log.warning("Feature Flag evaluation attempted before client has initialized - using last known values from feature store for feature key: " + key + ". This message is logged once.")
-            elif self._data_system.override_source_configured:
-                # No data from LaunchDarkly is available. The store read below still finds an
-                # entry that the override layer holds, and the SDK serves it. A miss returns the
-                # not-ready default.
-                no_launchdarkly_data = True
             else:
-                return not_ready()
+                # No data from LaunchDarkly is available. A flag that the override layer holds
+                # is served anyway. A flag the layer does not hold gets the not-ready default,
+                # as it does without an override source.
+                layer = self._data_system.override_layer
+                if layer is not None:
+                    flag = layer.get(FEATURES, key)
+                if flag is None:
+                    return not_ready()
 
         if not context.valid:
             log.warning("Context was invalid for flag evaluation (%s); returning default value" % context.error)
             return EvaluationDetail(default, None, error_reason('USER_NOT_SPECIFIED')), None
 
-        try:
-            flag = self._data_system.store.get(FEATURES, key)
-        except Exception as e:
-            log.error("Unexpected error while retrieving feature flag \"%s\": %s" % (key, repr(e)))
-            log.debug(traceback.format_exc())
-            reason = error_reason('EXCEPTION')
-            self._send_event(event_factory.new_unknown_flag_event(key, context, default, reason))
-            return EvaluationDetail(default, None, reason), None
+        if flag is None:
+            try:
+                flag = self._data_system.store.get(FEATURES, key)
+            except Exception as e:
+                log.error("Unexpected error while retrieving feature flag \"%s\": %s" % (key, repr(e)))
+                log.debug(traceback.format_exc())
+                reason = error_reason('EXCEPTION')
+                self._send_event(event_factory.new_unknown_flag_event(key, context, default, reason))
+                return EvaluationDetail(default, None, reason), None
         if not flag:
-            if no_launchdarkly_data:
-                return not_ready()
             reason = error_reason('FLAG_NOT_FOUND')
             self._send_event(event_factory.new_unknown_flag_event(key, context, default, reason))
             return EvaluationDetail(default, None, reason), None
@@ -566,13 +582,15 @@ def all_flags_state(self, context: Context, **kwargs) -> FeatureFlagsState:
                         if not self._all_flags_cached_data_warned:
                             self._all_flags_cached_data_warned = True
                             log.warning("all_flags_state() called before client has finished initializing! Using last known values from feature store. This message is logged once.")
-            elif self._data_system.override_source_configured:
-                # No data from LaunchDarkly is available. The store read below returns only the
-                # entries that the override layer holds. The result decides the state.
-                overrides_only = True
             else:
-                log.warning("all_flags_state() called before client has finished initializing! Feature store unavailable - returning empty state")
-                return FeatureFlagsState(False)
+                # No data from LaunchDarkly is available. When the override layer holds entries,
+                # the state is built from them alone. Otherwise the state is unavailable, as it
+                # is without an override source.
+                layer = self._data_system.override_layer
+                if layer is None or layer.is_empty:
+                    log.warning("all_flags_state() called before client has finished initializing! Feature store unavailable - returning empty state")
+                    return FeatureFlagsState(False)
+                overrides_only = True
 
         if not context.valid:
             log.warning("Context was invalid for all_flags_state (%s); returning default value" % context.error)

ldclient/impl/datasystem/__init__.py

@@ -10,6 +10,7 @@
 
 if TYPE_CHECKING:
     from ldclient.impl.aio.concurrency import AsyncEvent
+    from ldclient.impl.overrides.layer import OverrideLayer
 
 from ldclient.impl.listeners import Listeners
 from ldclient.interfaces import (
@@ -160,12 +161,12 @@ def environment_id(self) -> Optional[str]:
 
     @property
     @abstractmethod
-    def override_source_configured(self) -> bool:
+    def override_layer(self) -> Optional["OverrideLayer"]:
         """
-        Reports whether the data system was built with an override source. The value is
-        fixed at construction. When true, the store served by :attr:`store` overlays the
-        override layer, and the client serves an overridden flag before it has LaunchDarkly
-        data.
+        Returns the override layer when the data system was built with an override source,
+        and None otherwise. The store served by :attr:`store` overlays this layer. The client
+        consults the layer directly before it has LaunchDarkly data, so that a flag the layer
+        holds is served and a flag it does not hold gets the not-ready handling.
         """
         raise NotImplementedError
 
@@ -245,10 +246,10 @@ def store(self) -> AsyncReadOnlyStore:
 
     @property
     @abstractmethod
-    def override_source_configured(self) -> bool:
+    def override_layer(self) -> Optional["OverrideLayer"]:
         """
-        Reports whether the data system was built with an override source. The value is
-        fixed at construction.
+        Returns the override layer when the data system was built with an override source,
+        and None otherwise. See :attr:`DataSystem.override_layer`.
         """
         raise NotImplementedError
 

ldclient/impl/datasystem/async_fdv1.py

@@ -125,9 +125,9 @@ def store(self) -> AsyncReadOnlyStore:
         return self._store_view
 
     @property
-    def override_source_configured(self) -> bool:
+    def override_layer(self) -> None:
         # Overrides are an option of the FDv2 data system only.
-        return False
+        return None
 
     def set_diagnostic_accumulator(self, diagnostic_accumulator: DiagnosticAccumulator):
         """

ldclient/impl/datasystem/async_fdv2.py

@@ -677,8 +677,8 @@ def store(self) -> AsyncReadOnlyStore:
         return self._store_view
 
     @property
-    def override_source_configured(self) -> bool:
-        return self._override_source is not None
+    def override_layer(self) -> Optional[OverrideLayer]:
+        return self._override_layer
 
     async def data_availability(self) -> DataAvailability:  # type: ignore[override]
         """Reports what form of data is currently available, awaiting the store's

ldclient/impl/datasystem/fdv1.py

@@ -234,9 +234,9 @@ def store(self) -> ReadOnlyStore:
         return self._store_view
 
     @property
-    def override_source_configured(self) -> bool:
+    def override_layer(self) -> None:
         # Overrides are an option of the FDv2 data system only.
-        return False
+        return None
 
     @property
     def environment_id(self) -> Optional[str]:

ldclient/impl/datasystem/fdv2.py

@@ -680,8 +680,8 @@ def store(self) -> ReadOnlyStore:
         return self._store_view
 
     @property
-    def override_source_configured(self) -> bool:
-        return self._override_source is not None
+    def override_layer(self) -> Optional[OverrideLayer]:
+        return self._override_layer
 
 
 __all__ = [

ldclient/testing/mock_components.py

@@ -69,15 +69,21 @@ class MockOverrideSource:
     on every later call to set_overrides, and records its lifecycle calls.
     """
 
-    def __init__(self, flags: Optional[Dict[str, Any]] = None, segments: Optional[Dict[str, Any]] = None):
+    def __init__(self, flags: Optional[Dict[str, Any]] = None, segments: Optional[Dict[str, Any]] = None, start_error: Optional[Exception] = None):
+        """
+        :param start_error: when given, start raises it instead of supplying the contents
+        """
         self._flags: Dict[str, Any] = dict(flags or {})
         self._segments: Dict[str, Any] = dict(segments or {})
+        self._start_error = start_error
         self._sink: Optional[OverrideSink] = None
         self.start_count = 0
         self.close_count = 0
 
     def start(self, sink: OverrideSink) -> None:
         self.start_count += 1
+        if self._start_error is not None:
+            raise self._start_error
         self._sink = sink
         sink.set_overrides(self._flags, self._segments)