Repository navigation
feat!: Add a start wait time option for initialization - #71
kinyoklion wants to merge 8 commits into
Conversation
Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
@cursor review |
|
Devin check this against the latest version of the spec. |
Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
Rechecked against the latest OFP spec (
|
`InitializeAsync` ignored its `CancellationToken`, so a caller that passed a token to `SetProviderAsync` could not stop waiting for the LaunchDarkly client. - Cancelling the token now completes `InitializeAsync` with an `OperationCanceledException` instead of waiting for the data source - Cancellation is per-call: cancelling one wait leaves the provider initializing and does not fail other waiters - Feature matrix Initialization row updated to describe the behavior **Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Related issues** None. <details> <summary>Implementation details</summary> The OpenFeature .NET SDK passes the token given to `Api.SetProviderAsync(provider, cancellationToken)` through `ProviderRepository` into `FeatureProvider.InitializeAsync`. The provider returned `_initCompletion.Task` directly, so the token had no effect. Waiting now goes through a helper which, when the token can be canceled, races `_initCompletion.Task` against a `TaskCompletionSource` canceled by a token registration. `_initCompletion` itself is untouched, so the provider still becomes `READY` (or errors) when the data source reports its state, regardless of a canceled wait. Shutdown is unchanged: it only removes listeners and disposes the client, so there is no wait to cancel. Tests: two cases covering cancellation of a first wait and of a wait on an already in-progress initialization. `dotnet test -f net8.0` passes (70 tests). Note: [#71](#71) also changes initialization waiting; whichever merges second needs a trivial rebase of this region. </details> Link to Devin session: https://app.devin.ai/sessions/a47abf28ecd44130917b9b5131287221 Open in Devin Desktop: https://app.devin.ai/desktop/session/a47abf28ecd44130917b9b5131287221?variant=devin Requested by: @kinyoklion [SDK-3255](https://launchdarkly.atlassian.net/browse/SDK-3255) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > **`InitializeAsync` now honors the `CancellationToken`** passed through OpenFeature (e.g. from `SetProviderAsync`), so callers can stop waiting for the LaunchDarkly data source without blocking until ready or permanent failure. > > Waiting goes through **`WaitForInitializationAsync`**, which races `_initCompletion` against token cancellation. Cancellation is **per call**: `_initCompletion` is unchanged, so the client can still finish initializing and other waiters are unaffected. A canceled wait completes with **`OperationCanceledException`**. > > **`StatusProvider`** gains **`StartupWaitCanceled`**: it clears the “first event” suppression and, if status is already known, emits the matching provider event so subscribers still see **READY** (or stale/error) after a canceled startup wait. Status messages are retained for that replay; event emission is centralized in **`EmitStatusEvent`**. > > The README **Initialization** row and **three unit tests** document and cover token cancel on first wait, cancel on a concurrent second wait, and **ProviderReady** after cancel when the data source later becomes valid. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit afef99f. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> [SDK-3255]: https://launchdarkly.atlassian.net/browse/SDK-3255?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Gives the OpenFeature providers the initialization waiting values the OpenFeature provider spec (OFP) requires, which the numeric-only `initTimeoutSeconds` could not express. - `initTimeoutSeconds: 'forever'` waits indefinitely for the LaunchDarkly client - `initTimeoutSeconds: 0` waits nowhere and fails initialization unless the client is already ready, so readiness is observed through provider events - A positive timeout behaves as before, and the default is still 10 seconds - Node provider README feature matrix row updated for the new values <details> <summary>Implementation details</summary> **Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Related issues** OFP requirement 4.3.4 requires a value distinct from any duration to request waiting indefinitely, and 4.3.6 requires a zero wait to wait nowhere and fail initialization. Sibling changes for the same requirement: launchdarkly/openfeature-java-server#66, launchdarkly/openfeature-python-server#64, launchdarkly/openfeature-dotnet-server#71, launchdarkly/openfeature-ruby-server#36. **Describe the solution you've provided** `BaseProviderConfig.initTimeoutSeconds` and the Node provider's constructor parameter are now `number | 'forever'`, and zero is no longer coalesced into the default, since `??` only replaces `undefined`. `waitForInitialization` treats a falsy timeout as no timeout, so zero cannot be passed through: initialization races the pending initialization promise against an immediately rejected one, which succeeds only when the client is already ready. The Cloudflare provider is unaffected because it evaluates from a KV namespace and has no data source to wait for. **Describe alternatives you've considered** `number | null` was used initially for the indefinite case; `'forever'` reads better at the call site and matches the repository's preference for string unions. Adding `initialized()` to the client contract would let the zero case be checked directly, but that is a breaking change to a published interface for behavior the existing promise already expresses. **Additional context** Verified with `yarn workspace @launchdarkly/openfeature-js-server-common test` (82 tests), `yarn workspace @launchdarkly/openfeature-node-server test` (29 tests), and lint, format and build checks for both packages. </details> Link to Devin session: https://app.devin.ai/sessions/1fd3fcbfe79f482e8b58be29e8df2ffb Open in Devin Desktop: https://app.devin.ai/desktop/session/1fd3fcbfe79f482e8b58be29e8df2ffb?variant=devin Requested by: @kinyoklion <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Extends **`initTimeoutSeconds`** on the shared **`BaseOpenFeatureProvider`** and the Node **`LaunchDarklyProvider`** to **`number | 'forever'`**, aligning initialization with the OpenFeature provider spec. > > **`'forever'`** calls **`waitForInitialization()`** with no timeout. **`0`** no longer falls through to the 10-second default (only **`undefined`** does); initialization races the client’s init promise against an immediate failure so it succeeds only when the client is already ready, otherwise it throws a clear error. Positive timeouts behave as before (default **10** seconds). > > The Node provider README initialization row documents the new values, and unit tests cover indefinite wait, zero-timeout failure, and zero-timeout success when already ready. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 87e9a23. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
…et-start-wait Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com> # Conflicts: # README.md # src/LaunchDarkly.OpenFeature.ServerProvider/Provider.StatusProvider.cs # src/LaunchDarkly.OpenFeature.ServerProvider/Provider.cs # test/LaunchDarkly.OpenFeature.ServerProvider.Tests/ProviderTests.cs
| /// <param name="config">A client configuration object</param> | ||
| /// <param name="startWait">How long to wait for the client to become ready, or null to wait indefinitely</param> | ||
| public Provider(Configuration config, TimeSpan? startWait) | ||
| : this(new LdClient(WrapConfig(config, startWait ?? TimeSpan.Zero)), startWait == null) |
There was a problem hiding this comment.
Why would we use null instead of TimeSpan.Infinite?
There was a problem hiding this comment.
Good call — switched to it. The parameter is now TimeSpan startWait and Timeout.InfiniteTimeSpan is the indefinite sentinel, which also makes the signature non-nullable and matches Task.Delay/CancellationTokenSource conventions. Internally the provider still passes TimeSpan.Zero to StartWaitTime in that case, since the LaunchDarkly client's start wait would reject/ignore a negative timespan and the indefinite wait has to happen in InitializeAsync, not in the constructor. README and tests updated; 81 tests pass.
There was a problem hiding this comment.
Correction on one detail above: the .NET SDK docs only define StartWaitTime for 0 and positive durations, and don't specify behavior for a negative timespan, so rather than rely on that the provider maps the infinite case to TimeSpan.Zero for client construction and does the indefinite wait in InitializeAsync.
…ting Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
…it' into devin/1789570678-dotnet-start-wait Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com> # Conflicts: # README.md
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5c072d5. Configure here.
Co-Authored-By: rlamb@launchdarkly.com <4955475+kinyoklion@users.noreply.github.com>
LaunchDarkly .NET SDK 8.17.0 made a rejected SDK key (401/403 and other 4xx) retryable, so the data source no longer reaches `Off` for it. The README matrix still said `InitializeAsync` waits until the client becomes ready "or fails permanently", which is no longer true for a rejected key. - Initialization row: now⚠️ . It notes the SDK 8.17.0 behavior: without a cancellation token, `InitializeAsync` can wait indefinitely. It links the provider-side tracking PR [#71](#71) (start wait). - Asynchronous Initialization: the SDK's default `StartWaitTime` is 10 seconds (`ConfigurationBuilder.DefaultStartWaitTime`). The README said 5. <details> <summary>Details</summary> **Requirements** - [ ] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [ ] I have validated my changes against all supported platform versions **Related issues** #71 **Describe the solution you've provided** The provider references `LaunchDarkly.ServerSdk` `[8.1.0,9.0)`, so new installs resolve to 8.17.x and get the retry behavior. 8.17.0 release notes: "`DataSourceState.Off` no longer indicates a rejected SDK key. `Off` is now reached only by explicit shutdown." `main` was merged in after #72 (cancellation token) landed. The row now keeps #72's cancellation wording and no longer links #72 as a gap. **Additional context** #71 also edits the Initialization row. Whichever merges second will need a small README conflict fix. Found in the weekly OpenFeature provider audit. Docs-only change. </details> Link to Devin session: https://app.devin.ai/sessions/2a67f9d291bc4417bb40f632c191b44f Open in Devin Desktop: https://app.devin.ai/desktop/session/2a67f9d291bc4417bb40f632c191b44f?variant=devin Requested by: @kinyoklion [SDK-3255](https://launchdarkly.atlassian.net/browse/SDK-3255) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Updates the README **Initialization** feature matrix row to **partial (⚠️ )** and revises how `InitializeAsync` behaves with LaunchDarkly .NET SDK **8.17.0+**: a rejected SDK key is retried instead of failing permanently, so without a `SetProviderAsync` cancellation token initialization can wait indefinitely. The row now says init waits until ready or shutdown (not permanent failure) and links provider follow-up **#71**. > > In **Asynchronous Initialization**, the documented default constructor blocking wait is corrected from **5** to **10** seconds to match `ConfigurationBuilder.DefaultStartWaitTime`. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 7920750. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> [SDK-3255]: https://launchdarkly.atlassian.net/browse/SDK-3255?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Co-Authored-By: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com>
**Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Related issues** Fixes #58. **Describe the solution you've provided** Adds `Provider(String sdkKey, LDConfig config, Duration startWait)` so an application can bound, or deliberately opt out of, waiting for initialization, matching the OpenFeature provider spec (OFP) requirements for start wait behavior: - a positive `Duration` is applied by the LaunchDarkly client constructor, and `initialize` then reports whether the client became ready without waiting a second time - `Duration.ZERO` waits nowhere; `initialize` fails unless the client is already ready, and the application learns when it becomes usable from provider events - `Provider.INFINITE_START_WAIT` requests indefinite waiting: nothing is waited for during construction and `initialize` blocks until the data source becomes valid or fails permanently The existing constructors keep honouring `LDConfig.Builder.startWait(...)` and no longer wait again in `initialize`, so a configured start wait is the only bound on initialization instead of being followed by an unbounded wait. The README feature matrix row and initialization section describe the new behavior. <details> <summary>Implementation details</summary> `initialize` still registers data source and flag change listeners and evaluates the current data source status. Only the indefinite mode leaves the completion future pending; every other mode completes it with the outcome that is already known, so `initialize` returns promptly. The OpenFeature SDK emits its own event for the transition that completes initialization, so the provider records that one state without emitting a duplicate, and emits every state change after it — including a permanent failure that arrives immediately after the data source became valid, and a recovery after a failed initialization. </details> **Describe alternatives you've considered** Keeping the existing indefinite wait and adding a timeout parameter interpreted only inside `initialize` would double-wait when `LDConfig.startWait` is also set, and would not match OFP, which treats the start wait as a bound over construction plus initialization. Java has no built-in infinite `Duration`, so indefinite waiting was initially expressed as `null`. It is now the explicit `Provider.INFINITE_START_WAIT` constant, matching `Timeout.InfiniteTimeSpan` in the .NET provider. **Additional context** Sibling changes for the same OFP requirement: launchdarkly/openfeature-python-server#64, launchdarkly/openfeature-ruby-server#36, launchdarkly/openfeature-dotnet-server#71, and launchdarkly/js-core#2024. Verified with `./gradlew test` (all tests pass, including new coverage for positive, zero, and indefinite start wait). Link to Devin session: https://app.devin.ai/sessions/1fd3fcbfe79f482e8b58be29e8df2ffb Open in Devin Desktop: https://app.devin.ai/desktop/session/1fd3fcbfe79f482e8b58be29e8df2ffb?variant=devin Requested by: @kinyoklion [SDK-3285](https://launchdarkly.atlassian.net/browse/SDK-3285) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Overview** > Fixes OpenFeature initialization behavior (**#58**) by adding **`Provider(String, LDConfig, Duration startWait)`** and **`Provider.INFINITE_START_WAIT`**, so startup waiting is bounded in one place instead of blocking again indefinitely inside **`initialize()`** after **`LDConfig.startWait`**. > > **Positive durations** apply during client construction; **`Duration.ZERO`** fails fast unless already ready; **indefinite** mode skips construction blocking and waits in **`initialize()`** until the data source is valid, fails, or the provider shuts down. Oversized durations are treated as indefinite to avoid SDK millis overflow. > > **`initialize()`** is reworked around a completion future and tighter state/event handling: failed init surfaces data-source errors, duplicate **STALE**/**READY** events are deduped, teardown **OFF** does not emit **PROVIDER_ERROR**, and **`shutdown()`** calls **`super.shutdown()`** to stop leaking the event executor. README documents cancellation of indefinite **`setProviderAndWait`** (direct **`provider.shutdown()`** vs API lock contention). > > Lifecycle tests cover bounded/zero/infinite waits, recovery, and classification boundaries. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 1c96e64. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --- ## tanderson-ld additional changes Review-driven hardening of the start wait time behavior, on top of the commits above. Summary of the delta: **Indefinite wait is now cancellable.** `shutdown()` completes the initialization future, and the interrupt flag is restored if the waiting thread is interrupted. The README and constructor javadoc document how to cancel an indefinite `setProviderAndWait`: call `provider.shutdown()` directly from another thread, or register with the non-blocking `setProvider`. `OpenFeatureAPI.shutdown()` **cannot** end such a wait — it blocks on the same API lock `setProviderAndWait` holds for the duration of initialization. This lock-contention deadlock is inherent to the OpenFeature Java SDK (it holds the API write lock across a synchronous `initialize()` and contends on it in `shutdown()`), so it is documented rather than worked around; an upstream issue will be filed against the OpenFeature Java SDK. **Start wait time classification / validation.** - Any start wait time too large to represent in milliseconds — including `Provider.INFINITE_START_WAIT` and any other "forever" duration — is classified as indefinite, so it saturates to the indefinite path instead of overflowing when the SDK converts it to milliseconds. (Separately, a too-large duration configured through `LDConfig.Builder.startWait` instead of the constructor still overflows inside the SDK client constructor and leaks the client; that is pre-existing SDK behavior and has been filed as a separate java-core ticket. The sentinel is documented as constructor-argument-only.) - A `null` start wait time falls through to the SDK's default start wait rather than throwing. **Status / event fidelity.** - A failed initialization carries the data source's last error into the thrown exception, so the OpenFeature error event reports the cause rather than a generic message. - The client's `isInitialized()` latch no longer reports `READY` when the data source is currently `OFF`. - Repeated interruptions emit one `PROVIDER_STALE` per state change (state-based dedupe, retained and documented), and stale events now carry the HTTP error detail. **Lifecycle cleanup.** `shutdown()` removes its flag-change and data-source status listeners and calls `super.shutdown()` to terminate the event emitter executor. **Tests.** Added lifecycle coverage for bounded / zero / indefinite waits, failed-init-then-recovery, interruption → stale → ready, single-stale-per-outage dedupe, cancellation of an indefinite `setProviderAndWait` via a direct `provider.shutdown()`, the `OFF`-while-initialized exclusion, listener removal on shutdown, and a direct unit test of the start-wait classification boundary. Full suite and checkstyle pass. [SDK-3285]: https://launchdarkly.atlassian.net/browse/SDK-3285?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> Co-authored-by: Todd Anderson <tanderson@launchdarkly.com>

Summary
Brings initialization in line with the OpenFeature provider spec (OFP) start wait requirements, so the wait for the client has a bound.
Provider(Configuration config, TimeSpan startWait)constructor.InitializeAsyncthen reports the outcome and does not wait again.TimeSpan.Zerowaits nowhere, and readiness comes through provider events.Timeout.InfiniteTimeSpanwaits until the data source becomes valid or fails permanently. Canceling the token given toSetProviderAsyncends the wait (fix: Honor the cancellation token when waiting for initialization #72).StartWaitTimehas elapsed,InitializeAsyncstops waiting.InitializeAsyncstill registers the flag and data source listeners, then checks the current data source status. Only the indefinite mode leaves the initialization task pending. Every other mode completes it with whatever outcome is known at that point, so initialization does not wait a second time after the LaunchDarkly client constructor already waited.Latest
mainmerge (resolved conflicts):Provider.cs: kept this PR's_completionLockblock and added theOFFhandling from fix: Include the last data source error in provider error and stale events #74, which usesDescribeLastError.README.md:Timeout.InfiniteTimeSpankeeps waiting while the key is rejected.Configuration.DefaultStartWaitTime.Sibling changes for the same OFP requirement: launchdarkly/openfeature-java-server#66, launchdarkly/openfeature-python-server#64, launchdarkly/openfeature-ruby-server#36.
Testing:
dotnet test -f net8.0passes after the merge.Link to Devin session: https://app.devin.ai/sessions/b215bd8a675f4cc39541f1741d4013b7
Open in Devin Desktop: https://app.devin.ai/desktop/session/b215bd8a675f4cc39541f1741d4013b7?variant=devin
Requested by: @kinyoklion
SDK-3285