Event proxy causes undefined method push errors when calling native object methods
hwangm opened this issue · 6 comments
Describe the bug
If LD is configured to send events on flag read, when application code calls native object methods such as hasOwnProperty
on the flags
data from the useFlags
hook, an error gets thrown and subsequent events do not get sent to LD.
Uncaught TypeError: Cannot read properties of undefined (reading 'push')
at e.getSummary (vendors~app.8706a9e4d44d6226c395.js:173:24361)
at c.flush (vendors~app.8706a9e4d44d6226c395.js:173:36278)
at e (vendors~app.8706a9e4d44d6226c395.js:173:36702)
at r (vendors~app.8706a9e4d44d6226c395.js:236:31507)
To reproduce
- Configure LD react options without the
sendEventsOnFlagRead
option (or set totrue
) usingreact-client-sdk@2.27.0
- In a component, use the
useFlags
hook API to get a list of feature flags - Call a native object method on the returned
flags
object e.g.hasOwnProperty('name')
orpropertyIsEnumerable('name')
- Observe when that application code is called, the above browser error occurs
Expected behavior
If native object methods are called on the flags
object, they should be excluded from the flag read event map and no error should be thrown.
Logs
The undefined error occurs when the events are flushed to LD via the EventSummarizer (https://github.com/launchdarkly/js-sdk-common/blob/main/src/EventSummarizer.js#L63). I installed local copies of the js-client-sdk
and js-sdk-common
packages to add debugging around these files to see what the flag contents were (ss below):
I was able to identify application code that used conditionals checking against the flag name to perform some behavior:
const allFlags = useFeatureFlags();
const flag = allFlags.hasOwnProperty(name) ? allFlags[name] : null;
{...}
where useFeatureFlags
is defined as:
import { useFlags } from 'launchdarkly-react-client-sdk';
const useFeatureFlags = useFlags as () => FeatureFlags;
To test whether this was the issue, I first deleted any references to hasOwnProperty
on the flags
object, saw that the error was gone, then added new lines calling allFlags.propertyIsEnumerable('name')
to check if the error returned, resulting in the SS above. I also verified that setting the config option sendEventsOnFlagRead=false
eliminates the error even if native Object methods are called.
It appears that the JS proxy introduced in 2.27.0 considers native Object methods as flag keys and will add them to the flag map for count tracking unless the config option sendEventsOnFlagRead
is explicitly set to false
.
SDK version
react-client-sdk@2.27.0
and higher. I verified that this bug is present in all released versions after 2.27.0.
Language version, developer tools
Node 16.18, TS 4.7.3, React 17.0.2
OS/platform
Mac OS 12.6, M1 chip
Additional context
This should be considered a breaking change if applications have code that calls native Object methods on the flags
object and do not explicitly set this new configuration to false
, as errors will immediately start happening after upgrade and events will stop being set to LD.
Can someone from the LD team clarify whether this feature of sending events on flag read was something that was being done by default before this change, or is a new feature as a result of introducing the JS Proxy?
Thank you for reporting this. We are investigating the issue. This has been filed internally as 178466.
@yusinto Can you clarify: if someone upgrades to 2.27.0 and sets sendEventsOnFlagRead: false
, will they lose behavior that was previously working in 2.26.0? Or is this a new feature that an existing user may not be using? We're trying to determine whether we should wait for a fix before we upgrade or if it's safe to disable the feature and move on.
@hwangm the sendEventsOnFlagRead
feature is a new feature introduced in 2.27. Prior to 2.27 of the React SDK send an evaluation event for every flag on load by default.
@subvertallchris you won't lose the behavior in 2.26.0 by setting sendEventsOnFlagRead: false
. You can safely upgrade to 2.27.0 and not use this new feature. Be aware though in 2.27 the sendEventsOnlyForVariation is now set to true by default, so depending on your usage, you may need to adjust your code to keep existing behavior.
Please refer to the docs for more information.
@hwangm I will investigate the error on invoking native object methods on the flags object.
We reproduced the issue and have found the cause and the corresponding fix. A new release will be published soon with the fix. Thank you all for your patience.