-
Notifications
You must be signed in to change notification settings - Fork 43
fix(flagd): Throw general error when targeting variant not found #553
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix(flagd): Throw general error when targeting variant not found #553
Conversation
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
Summary of ChangesHello @kylejuliandev, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request refines the error handling within the Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request updates the JsonEvaluator to throw a General error when a targeting rule resolves to a variant that is not defined, which aligns with the specified behavior. The changes include adding the exception logic and a corresponding unit test. Additionally, an existing test was improved to assert the specific error type and message.
My review focuses on improving the clarity of the new error message. I've suggested making it more descriptive by including the name of the missing variant and using string interpolation for consistency and readability. This will aid in debugging and improve overall code maintainability.
src/OpenFeature.Contrib.Providers.Flagd/Resolver/InProcess/JsonEvaluator.cs
Outdated
Show resolved
Hide resolved
Signed-off-by: Kyle Julian <38759683+kylejuliandev@users.noreply.github.com>
This PR
Updates the JsonEvaluator for the InProcess resolver to return a general error when the targeting variant is not found. This is defined behaviour in the targeting.feature. Also used this as an opportunity to add an extra check for the type of error and message being returned in the previous test case.
Related Issues
Notes
Follow-up Tasks
How to test