-
Notifications
You must be signed in to change notification settings - Fork 20
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
feat: Add evaluation details to finally hook stage #335
base: main
Are you sure you want to change the base?
feat: Add evaluation details to finally hook stage #335
Conversation
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #335 +/- ##
==========================================
- Coverage 85.30% 85.27% -0.04%
==========================================
Files 36 36
Lines 1477 1480 +3
Branches 150 151 +1
==========================================
+ Hits 1260 1262 +2
Misses 187 187
- Partials 30 31 +1 ☔ View full report in Codecov by Sentry. |
src/OpenFeature/OpenFeatureClient.cs
Outdated
throw new ProviderNotReadyException("Provider has not yet completed initialization."); | ||
} | ||
else if (provider.Status == ProviderStatus.Fatal) | ||
{ | ||
evaluation = new FlagEvaluationDetails<T>(flagKey, defaultValue, ErrorType.ProviderFatal, Reason.Error, string.Empty, string.Empty); |
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.
Won't this be overridden in the catch block?
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.
You are right! Removed here: 5cedb62
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
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.
Could you please add a test to ensure that the evaluation details included in the final stage match what's returned to the user? Here's how requirement 4.3.8 was tested in JS.
Thank!
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Add evaluation details to the finally hook stage
This pull request includes several changes to improve the handling of flag evaluation details and error handling in the OpenFeature library. The most important changes include modifying hook methods to accept
FlagEvaluationDetails
as a parameter, updating theEvaluateFlagAsync
method to handle provider errors more gracefully, and adjusting the corresponding unit tests to reflect these changes.Related Issues
Fixes #328
Notes
Improvements to Hook Methods:
src/OpenFeature/Hook.cs
: Modified theFinallyAsync
methods to acceptFlagEvaluationDetails
as a parameter. [1]src/OpenFeature/Hook.cs
: Modified the hooks methods to split the parameters per line.[1] [2] [3]Enhancements to Flag Evaluation:
src/OpenFeature/OpenFeatureClient.cs
: Updated theEvaluateFlagAsync
method to initializeFlagEvaluationDetails
in case of provider errors and ensure theFinallyAsync
hook is always called with the correct evaluation details. [1] [2] [3]