add validation for feature flag definition

[Eskibear]

Feature flags are provided by IFeatureFlagProvider.

https://github.com/microsoft/FeatureManagement-JavaScript/blob/b70bbd341107b4093632d7272418884b5138de16/src/featureProvider.ts#L7-L18

We have two impl of feature flag providers for map-based and object-based usage. Data source can be arbitrary data but none of them validate whether a feature flag is valid according to the schema, which is not as robust as expected.

Two follow-up actions:

• In our built-in IFeatureFlagProvider impl, add validation and throw errors when data source itself is invalid.
• As customers can impl their own provider (potentially provide invalid data), in our impl of public APIs like isEnabled and getVariant, we can add validation after getting a feature flag from the provider, ensure we don't encounter unexpected error later.

[zhenlan]

What's the behavior of FM libraries in other languages? Do they also validate against the schema or tolerate unknown properties?

[Eskibear]

For strong-typed languages like C#, the validation happens automatically during deserialization stage. For weak-typed languages like JS/python, the validation should be manually added.

I notice there's validation logic in FM-Python as well, checking the related properties instead of against the whole schema. /cc @mrm9084 to comment with more details.

For JS, I'm thinking of doing the similar thing and do validation of the JSON content before constructing corresponding data objects, and to put the validation logic in a single place instead of scatter all over in each class/type. The minor difference is: python lib defines _validate method in some types and you first construct the objects and then call _validate later. E.g.FeatureConditions._validate, FeatureFlag._validate, etc. Cannot say which one is better, but in python if _validate can be in some "post-constructor" and automatically called, that would make a little bit more sense to me.

BTW, given the schema.json well defined, it's actually possible to validate via tools after loading, without writing specific validation logic manually. See https://json-schema.org/implementations#validators for libs for different languages. Just an alternative, trade-off is to introduce 3rd-party dependencies probably.

[zhenlan]

I understand we will fail if an expected field in a feature flag is not in the format we expect, but I'm not sure how tolerable we are about fields that we don't recognize/care.

If a user made a bad change to a feature flag, after a refresh, will that bad change cause all feature flags fail to load or just the bad one fails to load? Will the user application fail completely in this case, or will it just use the default value for that bad feature flag?

[Eskibear]

It depends on how severe we consider the user error (invalid format) of a feature flag is, do we want customers to know the error and get it fixed immediately, or let the app run with the error if possible (fallback to default value from the schema), with only something like a warning message.

but I'm not sure how tolerable we are about fields that we don't recognize/care.

All fields well-defined in the schema.json should be what we care. I cannot think of a counter example for the moment.

will that bad change cause all feature flags fail to load or just the bad one fails to load?

It makes more sense to only fail the bad ones, where they are explicitly used. Currently, validation happens after we fetch a feature flag via IFeatureFlagProvider.getFeatureFlag(flagName), only validating feature flag of the given name. On calling isEnabled(flagName) or getVariant(flagName, context), we fail for the bad ones.

One problem is whether we validate on calling FeatureManager.listFeatureNames() where we return a list of FeatureFlag.id, and the id field can be missing for bad ones. Here I would rather skip the validation, and only return feature flags more likely to have correct format, i.e. those who has id field.