Use specification values for error and reason enum

[alxckn]

The enum values should perhaps match the specification:

• reason
• error codes

Existing open feature compliant providers such as flagd return these exact codes, I believe it could be beneficial tohomogenize for matching purposes and in case a provider proxy implementation needs to generate errors as well

[maxveldink]

From my look just now, it looks like reason completely matches the spec, and we're missing the PROVIDER_FATAL error code on error codes. Would adding that be sufficient here, or am I missing something?

[alxckn]

I may be biased by working on the flagd provider but my thinking behind this issue is that any open feature compliant provider may respond with spec compatible reason or error code.
It'd be great to just be able to forward the codes sent by the remote providers, but with the current implementation I don't think we can and it will require mappings for each provider implementation even if open feature compliant.

For instance flagd may return "TARGETING_MATCH" as reason but "TARGETING_MATCH" != OpenFeature::SDK::Provider::Reason::TARGETING_MATCH

[maxveldink]

but with the current implementation I don't think we can and it will require mappings for each provider implementation even if open feature compliant.

I'd think the provider's job is to provide the mapping from the provider's error domain into the standard OF error domain (with the ability to add any additional context in reason, as that field has pre-defined types in the OF spec, but is not limited to those).

That said, I might still be missing what you're saying. Could you provide an example limitation in the flagd implementation that is requiring extra work to be compatible with the current SDK?

[alxckn]

In the flagd provider implementation, here is how the result is built: https://github.com/open-feature/ruby-sdk-contrib/blob/main/providers/openfeature-flagd-provider/lib/openfeature/flagd/provider/client.rb#L85

As you can see in this test, the reason is STATIC (upper case).
I think the SDK could support additional enum fields for reason, error or other fields but perhaps there's no need to reimplement custom mappings for OF compliant providers?

[maxveldink]

For the providers we've implemented, we've had a mapper that goes from the domain of the provider-specific results/errors into an OF-based reason, as defined in here. The backing strings aren't upper-case, which was an oversight over the type definition for reason. I'd accept a PR to change that casing.

The reason is an optional, open-ended string field. If the provider returns something equivalent to one of the enumerated reasons, we should map to those for consistency for users. If it doesn't map or could be interpreted as two different reasons, I would just pass back either what the provider returned or a provider-defined reason string that biases toward explanation.