Skip to content
This repository has been archived by the owner on Dec 4, 2024. It is now read-only.

feat: adds init async with timeout and deprecated non-timeout init functions #95

Merged
merged 4 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/LaunchDarkly.ClientSdk/ILdClientExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ public static EvaluationDetail<T> EnumVariationDetail<T>(this ILdClient client,
/// to be logged and no event will be sent</param>
/// <param name="maxWaitTime">the maximum time to wait for the new flag values</param>
/// <returns>true if new flag values were obtained</returns>
/// <seealso cref="ILdClient.Identify(Context, TimeSpan)"/>
[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);

Expand All @@ -123,7 +123,7 @@ public static bool Identify(this ILdClient client, User user, TimeSpan maxWaitTi
/// <param name="user">the user; should not be null (a null reference will cause an error
/// to be logged and no event will be sent</param>
/// <returns>a task that yields true if new flag values were obtained</returns>
/// <seealso cref="ILdClient.Identify(Context, TimeSpan)"/>
[Obsolete("User has been superseded by Context. See ILdClient.Identify(Context, TimeSpan)")]
public static Task<bool> IdentifyAsync(this ILdClient client, User user) =>
client.IdentifyAsync(Context.FromUser(user));
}
Expand Down
2 changes: 0 additions & 2 deletions src/LaunchDarkly.ClientSdk/Interfaces/ILdClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,6 @@ public interface ILdClient : IDisposable
/// <param name="maxWaitTime">the maximum time to wait for the new flag values</param>
/// <returns>true if new flag values were obtained</returns>
/// <seealso cref="IdentifyAsync(Context)"/>
/// <seealso cref="ILdClientExtensions.Identify(ILdClient, User, TimeSpan)"/>
bool Identify(Context context, TimeSpan maxWaitTime);

/// <summary>
Expand All @@ -349,7 +348,6 @@ public interface ILdClient : IDisposable
/// about setting the context and optionally requesting a unique key for it</param>
/// <returns>a task that yields true if new flag values were obtained</returns>
/// <seealso cref="Identify(Context, TimeSpan)"/>
/// <seealso cref="ILdClientExtensions.IdentifyAsync(ILdClient, User)"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: User is deprecated

Task<bool> IdentifyAsync(Context context);

/// <summary>
Expand Down
174 changes: 151 additions & 23 deletions src/LaunchDarkly.ClientSdk/LdClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
}

/// <summary>
/// Starts the client and waits up to the wait time to initialize feature flags. If offline,
cwaldren-ld marked this conversation as resolved.
Show resolved Hide resolved
/// returns immediately.
/// </summary>
/// <param name="maxWaitTime">the maximum length of time to wait for the client to initialize</param>
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));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this != logic work? (just haven't seen it before with tasks)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The whenAny returns the task that completed and if it isn't the start task then it must be the delay task.

if (completedTask != startTask)
{
_log.Warn(DidNotInitializeTimelyWarning, maxWaitTime.TotalMilliseconds);
}
}

/// <summary>
/// Starts the client and waits to initialize feature flags. If offline, returns immediately.
/// </summary>
async Task StartAsync()
{
await _connectionManager.Start();
Expand All @@ -244,16 +281,19 @@ async Task StartAsync()
/// </summary>
/// <remarks>
/// <para>
/// 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 <see cref="Initialized"/> property of <see langword="false"/>.
/// The constructor will return the <see cref="LdClient"/> 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 <see cref="Initialized"/> property of <see langword="false"/>, but the instance will continue
/// trying to get fresh feature flags.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: Documentation changes are just tidying up the contents and making each of the init docs more consistent with each other. They diverged in subtle ways over time.

/// </para>
/// <para>
/// If you would rather this happen asynchronously, use
/// <see cref="InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context)"/>. To
/// specify additional configuration options rather than just the mobile key, use
/// <see cref="Init(Configuration, Sdk.Context, TimeSpan)"/> or <see cref="InitAsync(Configuration, Sdk.Context)"/>.
/// To specify additional configuration options rather than just the mobile key, use
/// <see cref="Init(Configuration, Sdk.Context, TimeSpan)"/>.
/// </para>
/// <para>
/// If you would rather an asynchronous version of this method, use
/// <see cref="InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context, TimeSpan)"/>.
/// </para>
/// <para>
/// You must use one of these static factory methods to instantiate the single instance of LdClient
Expand Down Expand Up @@ -306,6 +346,7 @@ public static LdClient Init(string mobileKey, ConfigurationBuilder.AutoEnvAttrib
/// <seealso cref="Init(Configuration, User, TimeSpan)"/>
/// <seealso cref="Init(string, ConfigurationBuilder.AutoEnvAttributes, User, TimeSpan)"/>
/// <seealso cref="InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context)"/>
[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);
Expand All @@ -317,7 +358,53 @@ public static LdClient Init(string mobileKey, ConfigurationBuilder.AutoEnvAttrib
/// <remarks>
/// <para>
/// The returned task will yield the <see cref="LdClient"/> 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.
/// </para>
/// <para>
/// To specify additional configuration options rather than just the mobile key, you can use
/// <see cref="Init(Configuration, Sdk.Context, TimeSpan)"/> or <see cref="InitAsync(Configuration, Sdk.Context)"/>.
/// </para>
/// <para>
/// If you would rather a synchronous version of this method, use
/// <see cref="Init(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context, TimeSpan)"/>.
/// </para>
/// <para>
/// You must use one of these static factory methods to instantiate the single instance of LdClient
/// for the lifetime of your application.
/// </para>
/// </remarks>
/// <param name="mobileKey">the mobile key given to you by LaunchDarkly</param>
/// <param name="autoEnvAttributes">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
/// <a href="https://docs.launchdarkly.com/sdk/features/environment-attributes">our documentation</a> for
/// more details.</param>
/// <param name="initialContext">the initial evaluation context; see <see cref="LdClient"/> for more
/// about setting the context and optionally requesting a unique key for it</param>
/// <returns>a Task that resolves to the singleton LdClient instance</returns>
[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.")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For reviewers: The diff engine makes this look like I added a method and deprecated it immediately. That is not the case, just the diff shifting things around.

public static async Task<LdClient> InitAsync(string mobileKey,
ConfigurationBuilder.AutoEnvAttributes autoEnvAttributes, Context initialContext)
{
var config = Configuration.Default(mobileKey, autoEnvAttributes);
return await InitAsync(config, initialContext);
}

/// <summary>
/// Creates a new <see cref="LdClient"/> singleton instance and attempts to initialize feature flags
/// asynchronously.
/// </summary>
/// <remarks>
/// <para>
/// The returned task will yield the <see cref="LdClient"/> 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 <see cref="Initialized"/> property of <see langword="false"/>, and the instance will continue
/// trying to get fresh feature flags.
/// </para>
/// <para>
/// If you would rather this happen synchronously, use
Expand All @@ -340,13 +427,13 @@ public static LdClient Init(string mobileKey, ConfigurationBuilder.AutoEnvAttrib
/// more details.</param>
/// <param name="initialContext">the initial evaluation context; see <see cref="LdClient"/> for more
/// about setting the context and optionally requesting a unique key for it</param>
/// <param name="maxWaitTime">the maximum length of time to wait for the client to initialize</param>
/// <returns>a Task that resolves to the singleton LdClient instance</returns>
public static async Task<LdClient> 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);
}

/// <summary>
Expand All @@ -368,6 +455,7 @@ public static async Task<LdClient> InitAsync(string mobileKey,
/// more details.</param>
/// <param name="initialUser">the initial user attributes</param>
/// <returns>a Task that resolves to the singleton LdClient instance</returns>
[Obsolete("User has been superseded by Context, use Init(string, ConfigurationBuilder.AutoEnvAttributes, Context) instead.")]
public static Task<LdClient> InitAsync(string mobileKey,
ConfigurationBuilder.AutoEnvAttributes autoEnvAttributes, User initialUser) =>
InitAsync(mobileKey, autoEnvAttributes, Context.FromUser(initialUser));
Expand All @@ -378,16 +466,19 @@ public static Task<LdClient> InitAsync(string mobileKey,
/// </summary>
/// <remarks>
/// <para>
/// 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 <see cref="Initialized"/> property of <see langword="false"/>.
/// The constructor will return the <see cref="LdClient"/> 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 <see cref="Initialized"/> property of <see langword="false"/>, but the instance will continue
/// trying to get fresh feature flags.
/// </para>
/// <para>
/// If you would rather this happen asynchronously, use <see cref="InitAsync(Configuration, Sdk.Context)"/>.
/// If you do not need to specify configuration options other than the mobile key, you can use
/// <see cref="Init(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context, TimeSpan)"/> or
/// <see cref="InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context)"/>.
/// <see cref="Init(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context, TimeSpan)"/>.
/// </para>
/// <para>
/// If you would rather an asynchronous version of this method, use
/// <see cref="InitAsync(Configuration, Sdk.Context, TimeSpan)"/>.
/// </para>
/// <para>
/// You must use one of these static factory methods to instantiate the single instance of LdClient
Expand Down Expand Up @@ -433,6 +524,8 @@ public static LdClient Init(Configuration config, Context initialContext, TimeSp
/// <seealso cref="Init(Configuration, Sdk.Context, TimeSpan)"/>
/// <seealso cref="Init(string, ConfigurationBuilder.AutoEnvAttributes, User, TimeSpan)"/>
/// <seealso cref="InitAsync(Configuration, User)"/>
///
[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);

Expand All @@ -448,7 +541,6 @@ public static LdClient Init(Configuration config, User initialUser, TimeSpan max
/// <para>
/// If you would rather this happen synchronously, use <see cref="Init(Configuration, Sdk.Context, TimeSpan)"/>.
/// If you do not need to specify configuration options other than the mobile key, you can use
/// <see cref="Init(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context, TimeSpan)"/> or
/// <see cref="InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context)"/>.
/// </para>
/// <para>
Expand All @@ -463,13 +555,48 @@ public static LdClient Init(Configuration config, User initialUser, TimeSpan max
/// <seealso cref="InitAsync(Configuration, User)"/>
/// <seealso cref="InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, Sdk.Context)"/>
/// <seealso cref="Init(Configuration, Sdk.Context, TimeSpan)"/>
[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<LdClient> InitAsync(Configuration config, Context initialContext)
{
var c = CreateInstance(config, initialContext, TimeSpan.Zero);
await c.StartAsync();
return c;
}

/// <summary>
/// Creates a new <see cref="LdClient"/> singleton instance and attempts to initialize feature flags
cwaldren-ld marked this conversation as resolved.
Show resolved Hide resolved
/// asynchronously.
/// </summary>
/// <remarks>
/// <para>
/// The returned task will yield the <see cref="LdClient"/> 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 <see cref="Initialized"/> property of <see langword="false"/>, and the instance will continue
/// trying to get fresh feature flags.
/// </para>
/// <para>
/// If you would rather this happen synchronously, use <see cref="Init(Configuration, Sdk.Context, TimeSpan)"/>
/// </para>
/// <para>
/// You must use one of these static factory methods to instantiate the single instance of LdClient
/// for the lifetime of your application.
/// </para>
/// </remarks>
/// <param name="config">the client configuration</param>
/// <param name="initialContext">the initial evaluation context; see <see cref="LdClient"/> for more
/// about setting the context and optionally requesting a unique key for it</param>
/// <param name="maxWaitTime">the maximum length of time to wait for the client to initialize</param>
/// <returns>a Task that resolves to the singleton LdClient instance</returns>
public static async Task<LdClient> InitAsync(Configuration config, Context initialContext, TimeSpan maxWaitTime)
{
var c = CreateInstance(config, initialContext, TimeSpan.Zero);
await c.StartAsync(maxWaitTime);
return c;
}

/// <summary>
/// Creates a new <see cref="LdClient"/> singleton instance and attempts to initialize feature flags
/// asynchronously.
Expand All @@ -484,6 +611,7 @@ public static async Task<LdClient> InitAsync(Configuration config, Context initi
/// <seealso cref="InitAsync(Configuration, Sdk.Context)"/>
/// <seealso cref="InitAsync(string, ConfigurationBuilder.AutoEnvAttributes, User)"/>
/// <seealso cref="Init(Configuration, User, TimeSpan)"/>
[Obsolete("User has been superseded by Context, use InitAsync(Configuration, Context) instead.")]
public static Task<LdClient> InitAsync(Configuration config, User initialUser) =>
InitAsync(config, Context.FromUser(initialUser));

Expand Down
Loading