fix: Refactor error handling and improve documentation#417
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>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
…lag and improve assertions Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #417 +/- ##
==========================================
- Coverage 85.71% 85.69% -0.02%
==========================================
Files 39 39
Lines 1603 1601 -2
Branches 171 171
==========================================
- Hits 1374 1372 -2
Misses 191 191
Partials 38 38 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
test/OpenFeature.Tests/OpenFeatureClientTests.cs:187
- [nitpick] Consider adding a comment in the test to clarify that error logging is intentionally disabled during flag evaluation to help future maintainers understand this behavior.
mockedLogger.Received(0).IsEnabled(LogLevel.Error);
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
There was a problem hiding this comment.
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/OpenFeature/Providers/Memory/InMemoryProvider.cs:115
- [nitpick] Consider returning a ResolutionDetails with an appropriate error type for type mismatches instead of throwing an exception, to align the error handling behavior with that of missing flags. If the current behavior is by design, please add documentation explaining the rationale.
throw new TypeMismatchException("flag " + flagKey + " is not of type " + typeof(T));
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
kriscoleman
left a comment
There was a problem hiding this comment.
changes look good to me
I agree with the change on the flag-not-found. I prefer not to use exceptions to control logic flow; that's an old anti-pattern, and those stack traces have a hidden cost.
Thanks!
![]()
Signed-off-by: André Silva 2493377+askpt@users.noreply.github.com
This PR
This pull request includes updates to the
README.md, changes to error handling inOpenFeatureClient, and improvements to theInMemoryProviderand its related tests.Documentation updates:
README.mdto improve clarity on logging, transaction context propagators, dependency injection, and custom providers. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11]Error handling improvements:
FlagEvaluationErrorandFlagEvaluationErrorWithDescriptioninOpenFeatureClient. [1] [2] [3]InMemoryProvider enhancements:
InMemoryProviderto returnResolutionDetailswith appropriate error types instead of throwing exceptions for missing flags or type mismatches. [1] [2]Test updates:
OpenFeatureClientTestsandInMemoryProviderTeststo reflect changes in error handling and ensure proper error types and reasons are returned. [1] [2] [3]Configuration changes:
global.jsonto update the SDK roll-forward policy fromlatestMajortolatestFeature.Related Issues
Fixes #416
Notes
After evaluating the
global.json, I noticed the rollForward was way too high for what we should be using. I reduced tolatestFeaturewhich is safer and would allow us to change the GitHub Actions to use the version fromglobal.jsoninstead of from declaring it in the action itself.