Conversation
…ailable before registration
…dk-identify * andrey/clientsdk-plugins-and-hooks: address feadback remove files remove identify # Conflicts: # pkgs/sdk/client/src/Hooks/IdentifySeriesResult.cs
* main: chore(main): release LaunchDarkly.ServerSdk.Ai 0.9.3 (#233) fix: Make defaultValue optional with a disabled default (#232) chore(main): release LaunchDarkly.ClientSdk 5.6.0 (#231) feat: Add plugin support to Client SDK (#229) # Conflicts: # pkgs/sdk/client/src/Internal/Hooks/Executor/Executor.cs
- Modified the ILdClient and ILdClientExtensions interfaces to add a maxWaitTime parameter to IdentifyAsync. - Updated related documentation and references across multiple files to reflect the new method signature. - Enhanced the Identify method implementation to utilize the updated IdentifyAsync method with the maxWaitTime parameter. - Introduced IdentifySeries method in the hook executor to manage identify operations with hooks, including error handling and execution order.
…ementation - Updated the IdentifyAsync method to remove the maxWaitTime parameter from the public interface. - Adjusted related documentation to reflect the new method signature. - Ensured internal implementation retains the maxWaitTime parameter for flexibility in asynchronous identification operations.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes 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.
- Updated IdentifySeriesContext to accept TimeSpan instead of int for timeout, improving clarity and flexibility. - Adjusted IdentifySeries method in Executor to align with the new IdentifySeriesContext signature. - Modified IdentifySeriesTest to utilize TimeSpan.Zero for timeout, ensuring consistency in test cases.
| Context | ||
| oldContext = | ||
| newContext; // this initialization is overwritten below, it's only here to satisfy the compiler | ||
|
|
There was a problem hiding this comment.
The logic of changing _context and initing with cached data is also part of identify.
The data in the data store is what drives evaluation. An invocation of identify can't switch which context is being used for evaluation until after beforeIdentify hooks are finished.
Have you considered having the hook executor just use the internal identify for simplicity sake?
There was a problem hiding this comment.
I used logic from iOS and Android
There was a problem hiding this comment.
I gotta look again. Will do tomorrow.
There was a problem hiding this comment.
The identify series is also supposed to be invoked in the init case I believe. Double check me here with the spec.
Requirements
Adds BeforeIdentify / AfterIdentify hook stages to the existing hooks infrastructure, mirroring the evaluation series pattern. Hooks execute in forward order for BeforeIdentify and reverse (LIFO) order for AfterIdentify.
Wraps LdClient.IdentifyAsync with the identify hook series so that configured hooks receive callbacks before and after each identify operation, with IdentifySeriesResult.Completed on success and IdentifySeriesResult.Error on failure.
Threads maxWaitTime from Identify / IdentifyAsync into IdentifySeriesContext.Timeout so hooks have visibility into the caller's timeout configuration.
Note
Medium Risk
Touches the client’s
Identifypath and introduces new async hook execution and exception-handling behavior, which could affect context switching or timing if misordered or if hooks misbehave.Overview
Adds an Identify hook series alongside the existing evaluation hooks by introducing
Hook.BeforeIdentify/Hook.AfterIdentify, plus newIdentifySeriesContext(includes the identifiedContextand timeout) andIdentifySeriesResult(Completed/Error).Extends the internal hook executor (
IHookExecutor,Executor,NoopExecutor) with an asyncIdentifySeriespipeline, including per-hook error logging and LIFO execution for the after stage, and wrapsLdClient.IdentifyAsyncso every identify operation runs through this series while preserving the original return value/exception behavior. Includes new unit tests covering ordering, data passthrough, result status, and failure isolation/logging.Written by Cursor Bugbot for commit 283e9d2. This will update automatically on new commits. Configure here.