Fix/forced variations double format exception#152
Conversation
madhuchavva
left a comment
There was a problem hiding this comment.
Inline review comment on forced variation parsing semantics.
| int? forcedVariationIndex; | ||
| if (rawValue is int) { | ||
| forcedVariationIndex = rawValue; | ||
| } else if (rawValue is double) { |
There was a problem hiding this comment.
Good catch on int.parse(value.toString()); that crash is worth fixing. I think this parsing should be stricter before merge: double.toInt() silently turns values like 1.9 into variation 1, and string parsing via double.tryParse(rawValue)?.toInt() has the same issue. Forced variation IDs should be exact finite integer indexes, so we should only accept integer-valued numbers/strings (1, 1.0, "1", "1.0") and skip/log fractional or non-finite values. It would also be useful to validate the parsed index against experiment.variations before returning the forced result.
There was a problem hiding this comment.
Done — updated the parsing to only accept integer-valued numbers for doubles, same for string-parsed doubles and added bounds validation against experiment.variations.length before applying the forced result. Fractional or out-of-bounds values are now logged and skipped. Tests added for all new cases.
madhuchavva
left a comment
There was a problem hiding this comment.
Requesting changes based on the inline review comments. The notes call out functional gaps or compatibility/documentation risks that should be addressed before merge.
fix: handle int, double, and string values in
forcedVariationsMapto preventFormatExceptionProblem
forcedVariationsMapis typed asMap<String, dynamic>, so when variation values are deserialized from JSON, numeric values can arrive asdouble(e.g.1.0) instead ofint. The previous implementation converted any value to a string and calledint.parse(), which throws aFormatExceptionon inputs like"1.0":This was a runtime crash for any integration passing numeric variation indices through JSON.
Solution
Replace the
int.parse(.toString())call inexperiment_evaluator.dartwith explicit type-aware parsing:int→ used directlydouble→ converted via.toInt()String→ tried withint.tryParse, thendouble.tryParse(...).toInt()as fallbackbool, objects) → log a warning and skip the forced variation gracefully instead of crashingTest Cases Added
1(int)variationID = 11.0(double)variationID = 1"1"(string)variationID = 1"1.0"(string)variationID = 1"abc"trueBackward Compatibility
Existing integrations passing integer values are unaffected — the
intbranch is the first check and returns immediately.