feat: implement waitForInitialization for browser sdk 4.x
#1028
Conversation
|
@launchdarkly/browser size report |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/js-client-sdk-common size report |
joker23
force-pushed
the
skz/sdk-1360/browser-4.x-waitforinit
branch
from
d324724 to
acf1ac7
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| if (res.status === 'completed') { | ||
| this._isInitialized = true; | ||
| this._initResolve?.({ status: 'complete' }); | ||
| } |
There was a problem hiding this comment.
Bug: Initialization failure returns timeout instead of failed status
The identifyResult method only calls _initResolve when res.status === 'completed'. If identify fails with 'error', 'timeout', or 'shed' status, _initResolve is never called. This means waitForInitialization will always return { status: 'timeout' } for failed identifications, even though LDWaitForInitializationFailed type with status: 'failed' exists. Users cannot distinguish between an actual timeout and a genuine initialization failure.
There was a problem hiding this comment.
I think this is fine... if there is an error on one of the identifies then we will timeout here. Identification errors can be handled in the identify().catch() in case developers would want to try something else on error that could ultimately yield a successful identify within the waitForInitialization timeout.
There was a problem hiding this comment.
We do need to make sure that the waitForInitialization method returns immediately if we aren't actually attempting to initialize anymore though, versus actually timing out.
| return Promise.resolve({ | ||
| status: 'complete', | ||
| }); | ||
| } |
There was a problem hiding this comment.
Bug: No failure state tracking for post-identify waitForInitialization calls
The code tracks successful initialization with _isInitialized but has no equivalent tracking for failed initialization. If identify fails before waitForInitialization is called, the failure state is lost because _initResolve doesn't exist yet and there's no _initFailed or _initError field to record the failure. When waitForInitialization is called afterward, it sees _isInitialized as false, creates a new promise, but since identify already completed, _initResolve will never be called. The user experiences a timeout instead of receiving the immediate failed status.
Additional Locations (1)
unit tests broke because we introduced retries on initial polls which timed out of initial wait tests (since those tests were using fake timers). This change would simply advance the fake timer so all retries would have been triggered.
Requirements
Related issues
waitForInitializeto browser 4.xDescribe the solution you've provided
Added
waitForInitializationfunction to browser 4.x implementation.Additional context
f({timeout: 5})instead off(5)rejectconditions... I can add those back in case we get an error status returned? At that point we might want to just wait for the timeout to throw.initializedevent that a successful identify will emit, I figured that would be a way for developers to know if LDClient initialized even if the initialization goes past timeout.Note
Introduce
waitForInitializationwith timeout/result types for the browser SDK, emit aninitializedevent on successful identify, and add tests for success/timeout/failure.waitForInitialization(options)API returningLDWaitForInitializationResult(complete/failed/timeout) with default 5s timeout; implemented via internal init promise andcancelableTimedPromise.identifyResult: set internal initialized state; resolve ascompleteon success orfailedwith error; expose viamakeClient."initialized"event: new inLDEmitter; emitted on successful identify inLDClientImpl.LDWaitForInitialization*option/result types inLDClient.waitForInitializationfrom/compatLDClientsurface.waitForInitializationsuccess, timeout, and failure behaviors.Written by Cursor Bugbot for commit 0d640bf. This will update automatically on new commits. Configure here.