From a883902677fab43f3fc78666c05e87fff56e4e51 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Thu, 2 May 2024 18:25:34 -0500 Subject: [PATCH 1/4] feat: adds init async with timeout, deprecates init functions without timeouts and functions taking in User instances. --- src/LaunchDarkly.ClientSdk/LdClient.cs | 174 +++++++++++++++++++++---- 1 file changed, 151 insertions(+), 23 deletions(-) diff --git a/src/LaunchDarkly.ClientSdk/LdClient.cs b/src/LaunchDarkly.ClientSdk/LdClient.cs index 7c5c8eb5..7093812b 100644 --- a/src/LaunchDarkly.ClientSdk/LdClient.cs +++ b/src/LaunchDarkly.ClientSdk/LdClient.cs @@ -47,6 +47,13 @@ public sealed class LdClient : ILdClient static readonly object _createInstanceLock = new object(); static volatile LdClient _instance; + private readonly TimeSpan ExcessiveInitWaitTime = TimeSpan.FromSeconds(15); + + private const String ExcessiveInitWaitTimeWarning = + "LDClient.Init called with max wait time parameter of {0} seconds. We recommend a timeout of less than {1} seconds."; + + private const String DidNotInitializeTimelyWarning = "Client did not initialize within {0} milliseconds."; + // Immutable client state readonly Configuration _config; readonly LdClientContext _clientContext; @@ -130,7 +137,8 @@ public sealed class LdClient : ILdClient _log = _clientContext.BaseLogger; _taskExecutor = _clientContext.TaskExecutor; - _log.Info("Starting LaunchDarkly Client {0} built with target framework {1}", Version, SdkPackage.DotNetTargetFramework); + _log.Info("Starting LaunchDarkly Client {0} built with target framework {1}", Version, + SdkPackage.DotNetTargetFramework); var persistenceConfiguration = (_config.PersistenceConfigurationBuilder ?? Components.Persistence()) .Build(_clientContext); @@ -226,14 +234,43 @@ public sealed class LdClient : ILdClient void Start(TimeSpan maxWaitTime) { + if (maxWaitTime >= ExcessiveInitWaitTime) + { + _log.Warn(ExcessiveInitWaitTimeWarning, maxWaitTime, ExcessiveInitWaitTime); + } + var success = AsyncUtils.WaitSafely(() => _connectionManager.Start(), maxWaitTime); if (!success) { - _log.Warn("Client did not successfully initialize within {0} milliseconds.", - maxWaitTime.TotalMilliseconds); + _log.Warn(DidNotInitializeTimelyWarning, maxWaitTime.TotalMilliseconds); } } + /// + /// Starts the client and waits up to the wait time to initialize feature flags. If offline, + /// returns immediately. + /// + /// the maximum length of time to wait for the client to initialize + async Task StartAsync(TimeSpan maxWaitTime) + { + if (maxWaitTime >= ExcessiveInitWaitTime) + { + _log.Warn( + ExcessiveInitWaitTimeWarning, + maxWaitTime, ExcessiveInitWaitTime); + } + + var startTask = _connectionManager.Start(); + var completedTask = await Task.WhenAny(startTask, Task.Delay(maxWaitTime)); + if (completedTask != startTask) + { + _log.Warn(DidNotInitializeTimelyWarning, maxWaitTime.TotalMilliseconds); + } + } + + /// + /// Starts the client and waits to initialize feature flags. If offline, returns immediately. + /// async Task StartAsync() { await _connectionManager.Start(); @@ -244,16 +281,19 @@ async Task StartAsync() /// /// /// - /// In offline mode, this constructor will return immediately. Otherwise, it will wait and block on - /// the current thread until initialization and the first response from the LaunchDarkly service is - /// returned, up to the specified timeout. If the timeout elapses, the returned instance will have - /// an property of . + /// The constructor will return the instance once the first response from + /// the LaunchDarkly service is returned, or immediately if offline, or when the the specified + /// wait time elapses. If the max wait time elapses, the returned instance will have + /// an property of , but the instance will continue + /// trying to get fresh feature flags. /// /// - /// If you would rather this happen asynchronously, use - /// . To - /// specify additional configuration options rather than just the mobile key, use - /// or . + /// To specify additional configuration options rather than just the mobile key, use + /// . + /// + /// + /// If you would rather an asynchronous version of this method, use + /// . /// /// /// You must use one of these static factory methods to instantiate the single instance of LdClient @@ -306,6 +346,7 @@ public static LdClient Init(string mobileKey, ConfigurationBuilder.AutoEnvAttrib /// /// /// + [Obsolete("User has been superseded by Context, use Init(string, ConfigurationBuilder.AutoEnvAttributes, Context, TimeSpan) instead.")] public static LdClient Init(string mobileKey, ConfigurationBuilder.AutoEnvAttributes autoEnvAttributes, User initialUser, TimeSpan maxWaitTime) => Init(mobileKey, autoEnvAttributes, Context.FromUser(initialUser), maxWaitTime); @@ -317,7 +358,53 @@ public static LdClient Init(string mobileKey, ConfigurationBuilder.AutoEnvAttrib /// /// /// The returned task will yield the instance once the first response from - /// the LaunchDarkly service is returned (or immediately if it is in offline mode). + /// the LaunchDarkly service is returned or immediately if it is offline. + /// + /// + /// To specify additional configuration options rather than just the mobile key, you can use + /// or . + /// + /// + /// If you would rather a synchronous version of this method, use + /// . + /// + /// + /// You must use one of these static factory methods to instantiate the single instance of LdClient + /// for the lifetime of your application. + /// + /// + /// the mobile key given to you by LaunchDarkly + /// Enable / disable Auto Environment Attributes functionality. When enabled, + /// the SDK will automatically provide data about the environment where the application is running. + /// This data makes it simpler to target your mobile customers based on application name or version, or on + /// device characteristics including manufacturer, model, operating system, locale, and so on. We recommend + /// enabling this when you configure the SDK. See + /// our documentation for + /// more details. + /// the initial evaluation context; see for more + /// about setting the context and optionally requesting a unique key for it + /// a Task that resolves to the singleton LdClient instance + [Obsolete("Initializing the LDClient without a timeout is no longer permitted to help prevent" + + "consumers from blocking their application execution by mistake when connectivity is poor. Please" + + "use InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, Context, TimeSpan) and specify a max wait time.")] + public static async Task InitAsync(string mobileKey, + ConfigurationBuilder.AutoEnvAttributes autoEnvAttributes, Context initialContext) + { + var config = Configuration.Default(mobileKey, autoEnvAttributes); + return await InitAsync(config, initialContext); + } + + /// + /// Creates a new singleton instance and attempts to initialize feature flags + /// asynchronously. + /// + /// + /// + /// The returned task will yield the instance once the first response from + /// the LaunchDarkly service is returned, or immediately if offline, or when the the specified + /// wait time elapses. If the max wait time elapses, the returned instance will have + /// an property of , and the instance will continue + /// trying to get fresh feature flags. /// /// /// If you would rather this happen synchronously, use @@ -340,13 +427,13 @@ public static LdClient Init(string mobileKey, ConfigurationBuilder.AutoEnvAttrib /// more details. /// the initial evaluation context; see for more /// about setting the context and optionally requesting a unique key for it + /// the maximum length of time to wait for the client to initialize /// a Task that resolves to the singleton LdClient instance public static async Task InitAsync(string mobileKey, - ConfigurationBuilder.AutoEnvAttributes autoEnvAttributes, Context initialContext) + ConfigurationBuilder.AutoEnvAttributes autoEnvAttributes, Context initialContext, TimeSpan maxWaitTime) { var config = Configuration.Default(mobileKey, autoEnvAttributes); - - return await InitAsync(config, initialContext); + return await InitAsync(config, initialContext, maxWaitTime); } /// @@ -368,6 +455,7 @@ public static async Task InitAsync(string mobileKey, /// more details. /// the initial user attributes /// a Task that resolves to the singleton LdClient instance + [Obsolete("User has been superseded by Context, use Init(string, ConfigurationBuilder.AutoEnvAttributes, Context) instead.")] public static Task InitAsync(string mobileKey, ConfigurationBuilder.AutoEnvAttributes autoEnvAttributes, User initialUser) => InitAsync(mobileKey, autoEnvAttributes, Context.FromUser(initialUser)); @@ -378,16 +466,19 @@ public static Task InitAsync(string mobileKey, /// /// /// - /// In offline mode, this constructor will return immediately. Otherwise, it will wait and block on - /// the current thread until initialization and the first response from the LaunchDarkly service is - /// returned, up to the specified timeout. If the timeout elapses, the returned instance will have - /// an property of . + /// The constructor will return the instance once the first response from + /// the LaunchDarkly service is returned, or immediately if offline, or when the the specified + /// wait time elapses. If the max wait time elapses, the returned instance will have + /// an property of , but the instance will continue + /// trying to get fresh feature flags. /// /// - /// If you would rather this happen asynchronously, use . /// If you do not need to specify configuration options other than the mobile key, you can use - /// or - /// . + /// . + /// + /// + /// If you would rather an asynchronous version of this method, use + /// . /// /// /// You must use one of these static factory methods to instantiate the single instance of LdClient @@ -433,6 +524,8 @@ public static LdClient Init(Configuration config, Context initialContext, TimeSp /// /// /// + /// + [Obsolete("User has been superseded by Context, use Init(Configuration, Context, TimeSpan) instead.")] public static LdClient Init(Configuration config, User initialUser, TimeSpan maxWaitTime) => Init(config, Context.FromUser(initialUser), maxWaitTime); @@ -448,7 +541,6 @@ public static LdClient Init(Configuration config, User initialUser, TimeSpan max /// /// If you would rather this happen synchronously, use . /// If you do not need to specify configuration options other than the mobile key, you can use - /// or /// . /// /// @@ -463,6 +555,9 @@ public static LdClient Init(Configuration config, User initialUser, TimeSpan max /// /// /// + [Obsolete("Initializing the LDClient without a timeout is no longer permitted to help prevent" + + "consumers from blocking their application execution by mistake when connectivity is poor. Please" + + "use InitAsync(Configuration, Context, TimeSpan) and specify a max wait time.")] public static async Task InitAsync(Configuration config, Context initialContext) { var c = CreateInstance(config, initialContext, TimeSpan.Zero); @@ -470,6 +565,38 @@ public static async Task InitAsync(Configuration config, Context initi return c; } + /// + /// Creates a new singleton instance and attempts to initialize feature flags + /// asynchronously. + /// + /// + /// + /// The returned task will yield the instance once the first response from + /// the LaunchDarkly service is returned, or immediately if offline, or when the the specified + /// wait time elapses. If the max wait time elapses, the returned instance will have + /// an property of , and the instance will continue + /// trying to get fresh feature flags. + /// + /// + /// If you would rather this happen synchronously, use + /// + /// + /// You must use one of these static factory methods to instantiate the single instance of LdClient + /// for the lifetime of your application. + /// + /// + /// the client configuration + /// the initial evaluation context; see for more + /// about setting the context and optionally requesting a unique key for it + /// the maximum length of time to wait for the client to initialize + /// a Task that resolves to the singleton LdClient instance + public static async Task InitAsync(Configuration config, Context initialContext, TimeSpan maxWaitTime) + { + var c = CreateInstance(config, initialContext, TimeSpan.Zero); + await c.StartAsync(maxWaitTime); + return c; + } + /// /// Creates a new singleton instance and attempts to initialize feature flags /// asynchronously. @@ -484,6 +611,7 @@ public static async Task InitAsync(Configuration config, Context initi /// /// /// + [Obsolete("User has been superseded by Context, use InitAsync(Configuration, Context) instead.")] public static Task InitAsync(Configuration config, User initialUser) => InitAsync(config, Context.FromUser(initialUser)); From 1af06fce8509591bfd45748c92866eb937179f50 Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 3 May 2024 09:11:53 -0500 Subject: [PATCH 2/4] deprecating some additional user related functions --- src/LaunchDarkly.ClientSdk/ILdClientExtensions.cs | 4 ++-- src/LaunchDarkly.ClientSdk/Interfaces/ILdClient.cs | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/LaunchDarkly.ClientSdk/ILdClientExtensions.cs b/src/LaunchDarkly.ClientSdk/ILdClientExtensions.cs index 2bc8ff37..549adaf7 100644 --- a/src/LaunchDarkly.ClientSdk/ILdClientExtensions.cs +++ b/src/LaunchDarkly.ClientSdk/ILdClientExtensions.cs @@ -106,7 +106,7 @@ public static EvaluationDetail EnumVariationDetail(this ILdClient client, /// to be logged and no event will be sent /// the maximum time to wait for the new flag values /// true if new flag values were obtained - /// + [Obsolete("User has been superseded by Context. See ILdClient.Identify(Context, TimeSpan)")] public static bool Identify(this ILdClient client, User user, TimeSpan maxWaitTime) => client.Identify(Context.FromUser(user), maxWaitTime); @@ -123,7 +123,7 @@ public static bool Identify(this ILdClient client, User user, TimeSpan maxWaitTi /// the user; should not be null (a null reference will cause an error /// to be logged and no event will be sent /// a task that yields true if new flag values were obtained - /// + [Obsolete("User has been superseded by Context. See ILdClient.Identify(Context, TimeSpan)")] public static Task IdentifyAsync(this ILdClient client, User user) => client.IdentifyAsync(Context.FromUser(user)); } diff --git a/src/LaunchDarkly.ClientSdk/Interfaces/ILdClient.cs b/src/LaunchDarkly.ClientSdk/Interfaces/ILdClient.cs index de9b630b..5629eb8e 100644 --- a/src/LaunchDarkly.ClientSdk/Interfaces/ILdClient.cs +++ b/src/LaunchDarkly.ClientSdk/Interfaces/ILdClient.cs @@ -327,7 +327,6 @@ public interface ILdClient : IDisposable /// the maximum time to wait for the new flag values /// true if new flag values were obtained /// - /// bool Identify(Context context, TimeSpan maxWaitTime); /// @@ -349,7 +348,6 @@ public interface ILdClient : IDisposable /// about setting the context and optionally requesting a unique key for it /// a task that yields true if new flag values were obtained /// - /// Task IdentifyAsync(Context context); /// From 6e0caff5d927e50d46dd35754f9e5df76bbc8f3d Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 3 May 2024 11:02:02 -0500 Subject: [PATCH 3/4] fixing unit test --- tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs b/tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs index 18ee2bbc..88c27562 100644 --- a/tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs +++ b/tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs @@ -37,7 +37,7 @@ public class LdClientEndToEndTests : BaseTest { new object[] { UpdateMode.Polling } }, { new object[] { UpdateMode.Streaming } } }; - + public LdClientEndToEndTests(ITestOutputHelper testOutput) : base(testOutput) { } [Theory] @@ -106,7 +106,7 @@ public void InitCanTimeOutSync() Assert.False(client.Initialized); Assert.Null(client.StringVariation(_flagData1.Items.First().Key, null)); Assert.True(logCapture.HasMessageWithText(Logging.LogLevel.Warn, - "Client did not successfully initialize within 200 milliseconds.")); + "Client did not initialize within 200 milliseconds.")); } } } From 81a913cd403f0fdc9c38ec44c49cd05560e3078a Mon Sep 17 00:00:00 2001 From: Todd Anderson Date: Fri, 3 May 2024 11:21:00 -0500 Subject: [PATCH 4/4] ci: adding unit test for async init with timeout --- src/LaunchDarkly.ClientSdk/LdClient.cs | 8 ++++---- .../LDClientEndToEndTests.cs | 19 +++++++++++++++++++ .../LaunchDarkly.ClientSdk.Tests/TestUtil.cs | 14 ++++++++++++++ 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/src/LaunchDarkly.ClientSdk/LdClient.cs b/src/LaunchDarkly.ClientSdk/LdClient.cs index 7093812b..5135657d 100644 --- a/src/LaunchDarkly.ClientSdk/LdClient.cs +++ b/src/LaunchDarkly.ClientSdk/LdClient.cs @@ -384,8 +384,8 @@ public static LdClient Init(string mobileKey, ConfigurationBuilder.AutoEnvAttrib /// the initial evaluation context; see for more /// about setting the context and optionally requesting a unique key for it /// a Task that resolves to the singleton LdClient instance - [Obsolete("Initializing the LDClient without a timeout is no longer permitted to help prevent" + - "consumers from blocking their application execution by mistake when connectivity is poor. Please" + + [Obsolete("Initializing the LDClient without a timeout is no longer permitted to help prevent " + + "consumers from blocking their application execution by mistake when connectivity is poor. Please " + "use InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, Context, TimeSpan) and specify a max wait time.")] public static async Task InitAsync(string mobileKey, ConfigurationBuilder.AutoEnvAttributes autoEnvAttributes, Context initialContext) @@ -555,8 +555,8 @@ public static LdClient Init(Configuration config, User initialUser, TimeSpan max /// /// /// - [Obsolete("Initializing the LDClient without a timeout is no longer permitted to help prevent" + - "consumers from blocking their application execution by mistake when connectivity is poor. Please" + + [Obsolete("Initializing the LDClient without a timeout is no longer permitted to help prevent " + + "consumers from blocking their application execution by mistake when connectivity is poor. Please " + "use InitAsync(Configuration, Context, TimeSpan) and specify a max wait time.")] public static async Task InitAsync(Configuration config, Context initialContext) { diff --git a/tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs b/tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs index 88c27562..15781c4d 100644 --- a/tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs +++ b/tests/LaunchDarkly.ClientSdk.Tests/LDClientEndToEndTests.cs @@ -111,6 +111,25 @@ public void InitCanTimeOutSync() } } + [Fact] + public async void InitCanTimeOutAsync() + { + var handler = Handlers.Delay(TimeSpan.FromSeconds(2)).Then(SetupResponse(_flagData1, UpdateMode.Polling)); + using (var server = HttpServer.Start(handler)) + { + var config = BaseConfig(builder => + builder.DataSource(Components.PollingDataSource()) + .ServiceEndpoints(Components.ServiceEndpoints().Polling(server.Uri))); + using (var client = await TestUtil.CreateClientAsync(config, _user, TimeSpan.FromMilliseconds(200))) + { + Assert.False(client.Initialized); + Assert.Null(client.StringVariation(_flagData1.Items.First().Key, null)); + Assert.True(logCapture.HasMessageWithText(Logging.LogLevel.Warn, + "Client did not initialize within 200 milliseconds.")); + } + } + } + [Theory] [MemberData(nameof(PollingAndStreaming))] public void InitFailsOn401Sync(UpdateMode mode) diff --git a/tests/LaunchDarkly.ClientSdk.Tests/TestUtil.cs b/tests/LaunchDarkly.ClientSdk.Tests/TestUtil.cs index de00e482..c4cb2c68 100644 --- a/tests/LaunchDarkly.ClientSdk.Tests/TestUtil.cs +++ b/tests/LaunchDarkly.ClientSdk.Tests/TestUtil.cs @@ -125,6 +125,20 @@ public static async Task CreateClientAsync(Configuration config, Conte }); } + // Calls LdClient.Init, but then sets LdClient.Instance to null so other tests can + // instantiate their own independent clients. Application code cannot do this because + // the LdClient.Instance setter has internal scope. + public static async Task CreateClientAsync(Configuration config, Context context, TimeSpan waitTime) + { + return await WithClientLockAsync(async () => + { + ClearClient(); + LdClient client = await LdClient.InitAsync(config, context, waitTime); + client.DetachInstance(); + return client; + }); + } + public static void ClearClient() { WithClientLock(() => { LdClient.Instance?.Dispose(); });