fix: align NoOpProvider reason and ProviderInitializationError default error code with spec#271
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the default error code for ProviderInitializationError from PROVIDER_FATAL to GENERAL and updates the NoOpProvider to use Reason::DEFAULT. Corresponding tests have been updated to reflect these changes. Feedback was provided regarding a test in logging_hook_spec.rb that has become less effective because it now relies on the default error code; it is recommended to use a non-default error code to properly verify that the hook extracts the code from the exception.
There was a problem hiding this comment.
Pull request overview
Aligns the Ruby SDK’s default evaluation reason and provider initialization error-code defaults with the OpenFeature specification, improving spec compliance and consistency of logged/error metadata.
Changes:
- Update
NoOpProviderto returnProvider::Reason::DEFAULTinstead of a custom"No-op"reason string. - Change
ProviderInitializationErrordefaulterror_codefromPROVIDER_FATALtoGENERAL. - Update specs expecting the default error code and logged error-code fields accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| spec/open_feature/sdk/provider_initialization_error_spec.rb | Updates expectations to match GENERAL as the default error code. |
| spec/open_feature/sdk/hooks/logging_hook_spec.rb | Updates logged error_code expectation from PROVIDER_FATAL to GENERAL. |
| lib/open_feature/sdk/provider_initialization_error.rb | Changes initializer default error_code to Provider::ErrorCode::GENERAL and updates doc comment. |
| lib/open_feature/sdk/provider/no_op_provider.rb | Changes NoOpProvider fallback evaluation reason to Reason::DEFAULT. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…t error code with spec Signed-off-by: Cristian Marta <cristian@caffe-lento.com>
359e75a to
eb39a3a
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
"No-op"as the evaluation reason. Per spec Requirement 2.2.5, providers SHOULD use"DEFAULT"when returning the default/fallback value — changed toProvider::Reason::DEFAULT.PROVIDER_FATALerror code. Per the spec,PROVIDER_FATALis reserved for irrecoverable failures that permanently prevent flag evaluation.GENERALis the appropriate default for bare instantiation where no specific error code is known.