docs(ADR): standardize Flag configuration parsing and error handling

[aepfli]

relates to: #1690

This is the starting point, i think we should talk about what we want to achieve and how. My simple mind did not manage to come to another option i could propose, but i am open to add more options. please let me know what you think of this.

[netlify]

✅ Deploy Preview for polite-licorice-3db33c ready!

Name	Link
🔨 Latest commit	a9b3444
🔍 Latest deploy log	https://app.netlify.com/projects/polite-licorice-3db33c/deploys/68d27c09afce1600089ee6eb
😎 Deploy Preview	https://deploy-preview-1692--polite-licorice-3db33c.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[toddbaert]

I think a JSON document is either invalid or valid, so we don't have unlimited flexibility here unless we want to do some fairly annoying manipulation of the payload, either validating only 1 flag at a time (this might require breaking single flags into a subschema).

It might be possible to identify the specific sub-entity of an invalid configuration with the validation library, and remove it, but I think that might not be worth it or possible everywhere.

[aepfli]

we can select subsets for validating entities within the json schema, it will be one validation per flag, but i think it is better than invalidating the whole configuration.

[toddbaert]

We should make this consistent, I think. It's pretty trivial to support YAML/JSON conversion in most cases, though there might be some edge cases to consider.

[toddbaert]

💯

[toddbaert]

I have to admit, I have a strong preference for failing to start up if the config is invalid in any way, and fully rejecting an update if it's invalid in any way. I think this is what most users expect from invalid configurations. I would be open to a switch that would ignore validation errors, in which case the behavior might not be well-defined (but it would allow us to test some of our edge cases).

I think removing invalid flags actually leads to more confusing behaviors, since flagd will start up but certain flags might just be missing due to a typo. I think it's better to simply not start up if any single flag is invalid. When an update is received, I think similarly - it's better if the entire update fails than a single flag is "silently" removed (only showing up in a log or something).

I also think implementing removal of only invalid flags will be tricky.

I'm really interested in other's opinions here though. Maybe I'm in the minority here.

[aepfli]

I would not remove them, I would persist them with a parse_error in the flag storage. this is now implementation specific for eg a hashmap:

• an existing key without a value is a parse_error reason
• an existing key with a flag, will trigger the resolution process
• a non existing key, is a flag not found

This way, we keep the user informed about possible flag issues, but ensure that updates for other flags are working.

[aepfli]

enhanced the proposal with flow diagrams for easier understanding of the proposed flow

[toddbaert]

@aepfli sorry, I started typing without selecting the textbox first and accidently closed the issue 😅

To summarize my alternative proposal:

• use https://flagd.dev/schema/v0/flags.json to validate the whole document, on each sync
• at startup, if anything is invalid, flagd fails to start (this is already partially implemented; if we can't parse the flags configuration, so we'd just follow that same path)
• on any sync update, if anything is invalid, the update is ignored and an error/warning message is logged louded (again, this is already partially implemented; if we can't parse the flags configuration, so we'd just follow that same path)

Advantages:

• very easy to implement, just add a validation step in the exact same code-location we already parse
• we can remove many of our "error-edge-cases" in our test suite, since they will never be allowed
• we rely fully on a schema for validation so we don't have to implement any detailed error handling code

Disadvantages:

• 1 bad flag will prevent a whole update - however, this is already true if a single flag cannot be parsed (invalid JSON or nonsense variant configuration, etc) so we can never fully remove this situation, so I think we should just embrace this flow

[aepfli]

@toddbaert I also added your proposal.

I think it would be time to start some kind of vote - or give people the chance to add their remark - @open-feature/flagd-triagers please take a look