fix: provider exceptions crash the caller#97
Conversation
Signed-off-by: NeaguGeorgiana23 <neagugeorgiana@google.com>
There was a problem hiding this comment.
Code Review
This pull request updates the FeatureProvider interface and its implementations, including InMemoryProvider and NoopProvider, to return absl::StatusOr<std::unique_ptr<...>> for all flag evaluation methods. This change standardizes error reporting and explicitly discourages the use of C++ exceptions. Corresponding updates were made to the ClientAPI, test suites, and mocks to handle the new return types. Feedback suggests adding a null check for the unique_ptr within the absl::StatusOr in openfeature/client_api.h to prevent potential crashes if a provider returns an OK status with a null pointer.
| if (!result.ok()) { | ||
| return std::make_unique<ResolutionDetailsType>( | ||
| default_value, Reason::kError, std::nullopt, FlagMetadata(), | ||
| ErrorCode::kGeneral, std::string(result.status().message())); | ||
| } | ||
| return std::move(*result); |
There was a problem hiding this comment.
While checking result.ok() is necessary to handle explicit provider errors, it is also important to ensure that the provider did not return a nullptr inside the absl::StatusOr. If a provider implementation returns absl::OkStatus() but a null unique_ptr, the subsequent dereference in the caller (e.g., GetBooleanValue) will cause a crash. Given the PR's objective to prevent abnormal termination, adding a null check here provides an essential layer of safety.
if (!result.ok() || *result == nullptr) {
return std::make_unique<ResolutionDetailsType>(
default_value, Reason::kError, std::nullopt, FlagMetadata(),
ErrorCode::kGeneral,
result.ok() ? "Provider returned null resolution details"
: std::string(result.status().message()));
}
return std::move(*result);
This PR
Resolves the issue where feature provider errors could propagate and crash the caller during flag evaluation.
To satisfy Requirement 1.4.10 while strictly adhering to the Google C++ Style Guide's zero-exception rules, this PR refactors the evaluation boundary to use compile-time contractual safety:
FeatureProviderevaluation interface to returnabsl::StatusOr.Related Issues
Fixes #69
Follow-up Tasks