In T401705#11335589 it's noted that the routine at experiments.js#L44 could be made to be less forgiving. Presently if the initial regex doesn't pass it returns an empty set, not throwing a MalformedHeaderError, whereas if any of the three components of an experiment record in the X-Experiments-Enrollment is actually malformed that does throw a MalformedHeaderError.
Suggested updated approach:
- If the header value is empty or consists of whitespace throw a MalformedHeaderError with a message of "x-experiment-enrollments is empty or whitespace only"
- If the initial regex fails otherwise, throw a MalformedHeaderError with a message of "x-experiment-enrollments header is malformed, not matching initial regular expression"
- If the initial regex succeeds but one or more of the specific components turns out to be problematic, specify which via the MalformedHeaderError:
"x-experiment-enrollments header is malformed due to: <experiment name:[experiment name], group name:[group name], subject ID:[length V, WX...YZ ]>" - where the subject ID V value is a number, and W, X, Y, and Z are the first two and last two characters if there are 4 characters and the rest is redacted (otherwise if the value is three or fewer characters, whatever is in the string should be logged to Logstash).
Furthermore, the routine at experiments.js#L44 should be modified to log in a more helpful way (still being careful to avoid unnecessary data exposure).
Unfortunately, if we hit a scenario where there are multiple concurrent experiments for which the user is enrolled and just one of them is proving to be a problem in the header records, that means we'll get an error affecting all of them, which is suboptimal for data collection.
We should for now probably continue to be strict in this case by throwing the exception; but just be mindful that we may later need to apply both stringency and leniency here (e.g., in addition to the upfront initial regex check, merely log the issue for any record for which an experiment name / group name / subject ID is problematic and discard that record, but allowing any good enrollments into the resultant array)...but, again, for now it's probably best to stay on the stringent side.
For this to be complete, there are two testing requirements:
- the unit tests should be updated
- the work should be tested by making a shell script which tests the classes of issues available by creating malformed headers on the eventgate instance