chore(node-client): Implement NodeClient and createClient entry point#1416
chore(node-client): Implement NodeClient and createClient entry point#1416joker23 wants to merge 3 commits into
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@cursor review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit ac79085. Configure here.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| // preventing a stale reject from firing if setConnectionMode runs after resolution. | ||
| const wrappedResolve = identifyResolve | ||
| ? () => { | ||
| this._pendingIdentifyReject = undefined; |
There was a problem hiding this comment.
I am going to leave this in as a way to handle the edge case described in the comments... I do think that we can get rid of this handling and just let the previous identify timeout or be shedded.
put in the for ordering
fixing up the race conditions that could happen when connection mode is changed during identify
joker23
force-pushed
the
skz/sdk-2195/node-client-sdk-next-port-client-client-impl
branch
from
b3127f2 to
ad986dd
Compare
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 ad986dd. Configure here.
| this._setupConnection(context); | ||
| } else { | ||
| this._setupConnection(context, identifyResolve, identifyReject); | ||
| } |
There was a problem hiding this comment.
Close misses cache-load identify
Medium Severity
After await this.flagManager.loadCached(context), identify never checks this.closed and does not register identifyReject for close() until _setupConnection runs. If close() happens while the cache read is in flight, the identify promise can still resolve successfully or call _setupConnection and start a processor on a closed client, instead of failing fast like other in-flight identify paths.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit ad986dd. Configure here.
|
|
||
| // Serializes connection-mode transitions so concurrent calls cannot leave event-sending | ||
| // state out of sync with the active connection mode. | ||
| private _connectionModeQueue: Promise<void> = Promise.resolve(); |
There was a problem hiding this comment.
This feels like it would be inside the data manager.
2 participants
This PR will:
Note
Medium Risk
New client lifecycle and connection/identify logic affect flag loading and analytics delivery; behavior is heavily tested but this is core SDK surface area.
Overview
Replaces the node-client package placeholder with a working
createCliententry point and aNodeClient/NodeDataManagerstack wired tojs-client-sdk-common(FDv1 endpoints,requiresStart, client-side ID).NodeDataManagerowns streaming/polling/offline: bootstrap resolves identify immediately and skips cache; when not offline it still opens a live connection without forwarding identify callbacks. In-flight identifies are rejected onclose(),setConnectionMode, or mode races after cache load (instead of hanging on timeouts).NodeClientserializessetConnectionMode, flushes before going offline, and keeps event sending aligned with the effective mode;makeClientexposes the publicLDClientfacade (includingidentify→LDIdentifyResult).Adds broad Jest coverage (bootstrap, data sources, events, mocks) and drops
src/index.tsfrom release-pleaseextra-filesfor this package.Reviewed by Cursor Bugbot for commit ad986dd. Bugbot is set up for automated code reviews on this repo. Configure here.