ci(node-server-sdk): put in contract test workaround #1085
Conversation
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/browser size report |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| adaptedConfigs.dataSystem.synchronizers = { | ||
| primary: options.configuration.dataSystem.synchronizers?.[0], | ||
| secondary: options.configuration.dataSystem.synchronizers?.[1], | ||
| } |
There was a problem hiding this comment.
Missing null check causes crash when dataSystem undefined
High Severity
The code accesses adaptedConfigs.dataSystem.synchronizers without checking if dataSystem exists first. Since dataSystem is an optional property in SdkConfigOptions, this throws a TypeError when options.configuration.dataSystem is undefined. The makeSdkConfig function properly checks if (options.dataSystem) before accessing it, but this workaround code doesn't perform the same validation, creating an inconsistency that causes runtime crashes.
| adaptedConfigs.dataSystem.synchronizers = { | ||
| primary: options.configuration.dataSystem.synchronizers?.[0], | ||
| secondary: options.configuration.dataSystem.synchronizers?.[1], | ||
| } |
There was a problem hiding this comment.
Shallow copy causes unintended input mutation
Medium Severity
The shallow copy of options.configuration means adaptedConfigs.dataSystem still references the original object. When the code assigns adaptedConfigs.dataSystem.synchronizers to a new object structure, it mutates the original options.configuration.dataSystem.synchronizers, changing it from an array to an object. This unexpected side effect modifies the caller's input data, violating the principle that functions shouldn't mutate their parameters unless explicitly intended.
|
Please ignore - I thought I could do a quick workaround... |
Note
Low Risk
Test-only configuration adaptation; low blast radius, with main risk being masking real config-shape issues if relied on beyond contract tests.
Overview
Contract tests client initialization now adapts
dataSystem.synchronizersby converting an incoming array into the expected{ primary, secondary }shape before building the SDK config.This is explicitly marked as a temporary workaround (SDK-1798) and only affects how
newSdkClientEntitypasses configuration intomakeSdkConfigduringld.init.Written by Cursor Bugbot for commit 41edc61. This will update automatically on new commits. Configure here.