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.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
- 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.
pkgs/sdk/client/src/LdClient.cs
Outdated
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.
There was a problem hiding this comment.
iOS
The whole of identify work is encapsulated in a work item. This guarantees beforeIdentify finishes before beginning to make changes to internal data store.
There was a problem hiding this comment.
Now it is more like iOS
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.
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.
| async Task StartAsync() | ||
| { | ||
| await _connectionManager.Start(); | ||
| await RecordIdentify(_context, TimeSpan.Zero); |
There was a problem hiding this comment.
I think this should be before the connection manager start because the connection manager does work with the identified context. In this order, it is a contradiction from the customer code's perspective. How can something do work with the identified context until after beforeIdentify has finished?
I think this comment applies in other spots.
There was a problem hiding this comment.
Sure and Interesting
2 participants
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 core
Identifyand initialization paths and adds new async hook execution around them, which could affect context-switch timing and event/connection behavior if misordered or if hooks are slow/faulty.Overview
Adds a new hook series for identify operations by extending
HookwithBeforeIdentify/AfterIdentifyand introducingIdentifySeriesContext(includes timeout) plusIdentifySeriesResult(Completed/Error).Plumbs this through the internal hook executor (
IHookExecutor,Executor,NoopExecutor) and updatesLdClientstartup andIdentify/IdentifyAsyncflows to run identify operations inside the hook series, ensuringAfterIdentifyruns on both success and exceptions while keeping exceptions propagated. Includes comprehensive tests validating ordering (LIFO), data passthrough, logging on hook failures, status reporting, and hook activation during init.Written by Cursor Bugbot for commit 11b9692. This will update automatically on new commits. Configure here.