wingify / vwo-node-sdk

VWO Node SDK for server-side testing
https://developers.vwo.com/docs/fullstack-overview
Apache License 2.0
24 stars 9 forks source link

fix: replace weight calculation with a rounded ceil #52

Open samc opened 11 months ago

samc commented 11 months ago

Looks like we stumbled across a bug in our production middleware that was resulting in some unwarranted internal service errors - looks to us like there's an overflow in the min / max value calculation for random mutually exclusive groups.

I'll use an example from our production code that was giving us trouble to explain the problem.

1) We have a mutually exclusive group (random assignment) with exactly 6 tenants 2) shortlistedCampaigns would then have their weights adjusted to 16 based on the following:

https://github.com/wingify/vwo-node-sdk/blob/16df6ebc5fac741b9eac091242b10149250745de/lib/utils/DecisionUtil.js#L1016 3) based on that weight, the following would adjust the lower and upper bounds of startVariationAllocation & endVariationAllocation respectively:

https://github.com/wingify/vwo-node-sdk/blob/16df6ebc5fac741b9eac091242b10149250745de/lib/utils/CampaignUtil.js#L41 4) this all results in a maximum upper bound of 9600 when it comes time to decide on a variation:

https://github.com/wingify/vwo-node-sdk/blob/16df6ebc5fac741b9eac091242b10149250745de/lib/core/BucketingService.js#L62-L64

5) which, looking at the algo from _generateBucketValue, looks to default to a max of 10000 based on Constants.MAX_TRAFFIC_PERCENT. This leaves a 4% chance that an assigned bucketed value falls outside the acceptable bounds of all active tests in a group and returns null - failing on this line:

https://github.com/wingify/vwo-node-sdk/blob/efee33f24955c3c869185ae87f5dc0514ca0a230/lib/utils/DecisionUtil.js#L1033

Here's a snapshot of the overflow in action on our production site:

image