SemanticVersion logs a bad warning and throws when user attribute is missing or null

Question

johncrenshawofferup opened this issue 3 years ago · 1 comments

There is a ton to unpack here:

Audience targeting where a user attribute is null or missing is not treated as an exceptional condition for any other attribute. Comparison can and should reasonably return null. This is technically the effective result (after the exception is caught in , but the way it arrives there is fragile and poorly behaved.
Warning is the wrong logging level for a null or unprovided user attribute. This is a debug issue only. If you believe that logging on this condition really is needed, the correct logging level to use is debug (this is the logging level used for the null version case in the Javascript wrapper). Logging at the wrong level creates a potentially severe production burden for services that use this SDK.
Logging should be done from a logger on the SemanticVersion class. Instead, because this is thrown as an exception and bounced around before logging, the logger that gives the warning is on the UserAttribute class. The UserAttribute class probably SHOULD log a warning in this case, but a null attribute should never go that far. As communicated earlier, all other comparators include a null check on the user attribute and return null in that case rather than throwing. The problem with logging this from the wrong class is that now our services can't silence just the message from the SemanticVersion class, but rather, we have to set our configuration to silence the entire UserAttribute class, including potentially more serious conditions that we would want to know about.

Answer 1 · 2022-03-17T21:38:16.000Z