Skip to content

fix: provider exceptions crash the caller #97

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.

Sign up for GitHub

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

Draft
NeaguGeorgiana23 wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

fix: provider exceptions crash the caller #97

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions openfeature/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ cc_library(
":provider",
":resolution_details",
":value",
"@abseil-cpp//absl/status:statusor",
],
)

Expand Down Expand Up @@ -165,6 +166,7 @@ cc_library(
include_prefix = "openfeature",
deps = [
"@abseil-cpp//absl/status",
"@abseil-cpp//absl/status:statusor",
":evaluation_context",
":metadata",
":resolution_details",
Expand Down
9 changes: 8 additions & 1 deletion openfeature/client_api.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,14 @@ std::unique_ptr<ResolutionDetailsType> ClientAPI::EvaluateFlag(
}

EvaluationContext merged_context = MergeContexts(ctx);
return provider_call(provider, merged_context);
auto result = provider_call(provider, merged_context);

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);
Comment on lines +133 to +138
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot May 11, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

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);

}

} // namespace openfeature
Expand Down
29 changes: 18 additions & 11 deletions openfeature/memory_provider/in_memory_provider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,29 +50,36 @@
flags_.insert_or_assign(std::move(key), std::move(new_flag));
}

std::unique_ptr<BoolResolutionDetails> InMemoryProvider::GetBooleanEvaluation(
std::string_view key, bool default_value, const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<BoolResolutionDetails>>
InMemoryProvider::GetBooleanEvaluation(std::string_view key, bool default_value,
const EvaluationContext& ctx) {
return Evaluate<bool>(key, default_value, ctx);
}

std::unique_ptr<StringResolutionDetails> InMemoryProvider::GetStringEvaluation(
std::string_view key, std::string_view default_value,
const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<StringResolutionDetails>>
InMemoryProvider::GetStringEvaluation(std::string_view key,
std::string_view default_value,
const EvaluationContext& ctx) {
return Evaluate<std::string>(key, std::string(default_value), ctx);
}

std::unique_ptr<IntResolutionDetails> InMemoryProvider::GetIntegerEvaluation(
std::string_view key, int64_t default_value, const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<IntResolutionDetails>>
InMemoryProvider::GetIntegerEvaluation(std::string_view key,
int64_t default_value,
const EvaluationContext& ctx) {
return Evaluate<int64_t>(key, default_value, ctx);
}

std::unique_ptr<DoubleResolutionDetails> InMemoryProvider::GetDoubleEvaluation(
std::string_view key, double default_value, const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<DoubleResolutionDetails>>
InMemoryProvider::GetDoubleEvaluation(std::string_view key,
double default_value,
const EvaluationContext& ctx) {
return Evaluate<double>(key, default_value, ctx);
}

std::unique_ptr<ObjectResolutionDetails> InMemoryProvider::GetObjectEvaluation(
std::string_view key, Value default_value, const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<ObjectResolutionDetails>>
InMemoryProvider::GetObjectEvaluation(std::string_view key, Value default_value,
const EvaluationContext& ctx) {
return Evaluate<Value>(key, default_value, ctx);
}

Expand All @@ -98,7 +105,7 @@
}

std::string key_str{key};
auto it = flags_.find(key_str);

Check failure on line 108 in openfeature/memory_provider/in_memory_provider.cpp

View workflow job for this annotation

GitHub Actions / lint

openfeature/memory_provider/in_memory_provider.cpp:108:8 [readability-identifier-length] 

variable name 'it' is too short, expected at least 3 characters
if (it == flags_.end()) {
return std::make_unique<ResolutionDetails<T>>(
default_value, Reason::kError, std::nullopt, FlagMetadata{},
Expand Down
11 changes: 6 additions & 5 deletions openfeature/memory_provider/in_memory_provider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include <unordered_map>

#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "openfeature/evaluation_context.h"
#include "openfeature/metadata.h"
#include "openfeature/provider.h"
Expand All @@ -23,9 +24,9 @@
// evaluation based on the provided EvaluationContext.
class InMemoryProvider : public FeatureProvider {
public:
InMemoryProvider(std::unordered_map<std::string, std::any> flags);

Check failure on line 27 in openfeature/memory_provider/in_memory_provider.h

View workflow job for this annotation

GitHub Actions / lint

openfeature/memory_provider/in_memory_provider.h:27:3 [google-explicit-constructor] 

single-argument constructors must be marked explicit to avoid unintentional implicit conversions

~InMemoryProvider() = default;

Check failure on line 29 in openfeature/memory_provider/in_memory_provider.h

View workflow job for this annotation

GitHub Actions / lint

openfeature/memory_provider/in_memory_provider.h:29:3 [modernize-use-override] 

annotate this function with 'override' or (rarely) 'final'

Metadata GetMetadata() const override;

Expand All @@ -42,34 +43,34 @@
// will be added to the configuration.
void UpdateFlag(std::string key, std::any new_flag);

std::unique_ptr<BoolResolutionDetails> GetBooleanEvaluation(
absl::StatusOr<std::unique_ptr<BoolResolutionDetails>> GetBooleanEvaluation(
std::string_view key, bool default_value,
const EvaluationContext& ctx) override;

std::unique_ptr<StringResolutionDetails> GetStringEvaluation(
absl::StatusOr<std::unique_ptr<StringResolutionDetails>> GetStringEvaluation(
std::string_view key, std::string_view default_value,
const EvaluationContext& ctx) override;

std::unique_ptr<IntResolutionDetails> GetIntegerEvaluation(
absl::StatusOr<std::unique_ptr<IntResolutionDetails>> GetIntegerEvaluation(
std::string_view key, int64_t default_value,
const EvaluationContext& ctx) override;

std::unique_ptr<DoubleResolutionDetails> GetDoubleEvaluation(
absl::StatusOr<std::unique_ptr<DoubleResolutionDetails>> GetDoubleEvaluation(
std::string_view key, double default_value,
const EvaluationContext& ctx) override;

std::unique_ptr<ObjectResolutionDetails> GetObjectEvaluation(
absl::StatusOr<std::unique_ptr<ObjectResolutionDetails>> GetObjectEvaluation(
std::string_view key, Value default_value,
const EvaluationContext& ctx) override;

private:
template <typename T>
std::unique_ptr<ResolutionDetails<T>> Evaluate(std::string_view key,
T default_value,

Check failure on line 69 in openfeature/memory_provider/in_memory_provider.h

View workflow job for this annotation

GitHub Actions / lint

openfeature/memory_provider/in_memory_provider.h:69:52 [performance-unnecessary-value-param] 

the parameter 'default_value' is copied for each invocation but only used as a const reference; consider making it a const reference
const EvaluationContext& ctx);

std::unordered_map<std::string, std::any> flags_;
ProviderStatus status_;

Check failure on line 73 in openfeature/memory_provider/in_memory_provider.h

View workflow job for this annotation

GitHub Actions / lint

openfeature/memory_provider/in_memory_provider.h:73:18 [modernize-use-default-member-init] 

use default member initializer for 'status_'
mutable std::shared_mutex mutex_;
};

Expand Down
28 changes: 16 additions & 12 deletions openfeature/noop_provider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,38 +4,42 @@ namespace openfeature {

Metadata NoopProvider::GetMetadata() const { return Metadata{name_}; }

std::unique_ptr<BoolResolutionDetails> NoopProvider::GetBooleanEvaluation(
std::string_view flag, bool default_value, const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<BoolResolutionDetails>>
NoopProvider::GetBooleanEvaluation(std::string_view flag, bool default_value,
const EvaluationContext& ctx) {
return std::make_unique<BoolResolutionDetails>(
default_value, Reason::kDefault, "default-variant", FlagMetadata(),
std::nullopt, "");
}

std::unique_ptr<StringResolutionDetails> NoopProvider::GetStringEvaluation(
std::string_view flag, std::string_view default_value,
const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<StringResolutionDetails>>
NoopProvider::GetStringEvaluation(std::string_view flag,
std::string_view default_value,
const EvaluationContext& ctx) {
return std::make_unique<StringResolutionDetails>(
std::string(default_value), Reason::kDefault, "default-variant",
FlagMetadata(), std::nullopt, "");
}

std::unique_ptr<IntResolutionDetails> NoopProvider::GetIntegerEvaluation(
std::string_view flag, int64_t default_value,
const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<IntResolutionDetails>>
NoopProvider::GetIntegerEvaluation(std::string_view flag, int64_t default_value,
const EvaluationContext& ctx) {
return std::make_unique<IntResolutionDetails>(
default_value, Reason::kDefault, "default-variant", FlagMetadata(),
std::nullopt, "");
}

std::unique_ptr<DoubleResolutionDetails> NoopProvider::GetDoubleEvaluation(
std::string_view flag, double default_value, const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<DoubleResolutionDetails>>
NoopProvider::GetDoubleEvaluation(std::string_view flag, double default_value,
const EvaluationContext& ctx) {
return std::make_unique<DoubleResolutionDetails>(
default_value, Reason::kDefault, "default-variant", FlagMetadata(),
std::nullopt, "");
}

std::unique_ptr<ObjectResolutionDetails> NoopProvider::GetObjectEvaluation(
std::string_view flag, Value default_value, const EvaluationContext& ctx) {
absl::StatusOr<std::unique_ptr<ObjectResolutionDetails>>
NoopProvider::GetObjectEvaluation(std::string_view flag, Value default_value,
const EvaluationContext& ctx) {
return std::make_unique<ObjectResolutionDetails>(
default_value, Reason::kDefault, "default-variant", FlagMetadata(),
std::nullopt, "");
Expand Down
11 changes: 6 additions & 5 deletions openfeature/noop_provider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <string>
#include <string_view>

#include "absl/status/statusor.h"
#include "openfeature/evaluation_context.h"
#include "openfeature/metadata.h"
#include "openfeature/provider.h"
Expand All @@ -23,27 +24,27 @@ class NoopProvider : public FeatureProvider {
Metadata GetMetadata() const override;

// BooleanEvaluation returns a boolean flag.
std::unique_ptr<BoolResolutionDetails> GetBooleanEvaluation(
absl::StatusOr<std::unique_ptr<BoolResolutionDetails>> GetBooleanEvaluation(
std::string_view flag, bool default_value,
const EvaluationContext& ctx) override;

// StringResolutionDetails returns a string flag.
std::unique_ptr<StringResolutionDetails> GetStringEvaluation(
absl::StatusOr<std::unique_ptr<StringResolutionDetails>> GetStringEvaluation(
std::string_view flag, std::string_view default_value,
const EvaluationContext& ctx) override;

// IntResolutionDetails returns an integer flag.
std::unique_ptr<IntResolutionDetails> GetIntegerEvaluation(
absl::StatusOr<std::unique_ptr<IntResolutionDetails>> GetIntegerEvaluation(
std::string_view flag, int64_t default_value,
const EvaluationContext& ctx) override;

// DoubleResolutionDetails returns a double flag.
std::unique_ptr<DoubleResolutionDetails> GetDoubleEvaluation(
absl::StatusOr<std::unique_ptr<DoubleResolutionDetails>> GetDoubleEvaluation(
std::string_view flag, double default_value,
const EvaluationContext& ctx) override;

// ObjectResolutionDetails returns an object flag.
std::unique_ptr<ObjectResolutionDetails> GetObjectEvaluation(
absl::StatusOr<std::unique_ptr<ObjectResolutionDetails>> GetObjectEvaluation(
std::string_view flag, Value default_value,
const EvaluationContext& ctx) override;

Expand Down
34 changes: 19 additions & 15 deletions openfeature/provider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
#include <string_view>

#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "openfeature/evaluation_context.h"
#include "openfeature/metadata.h"
#include "openfeature/resolution_details.h"
Expand All @@ -15,26 +16,29 @@ namespace openfeature {
// FeatureProvider interface defines a set of functions that can be called
// in order to evaluate a flag. This should be implemented by flag management
// systems.
// Implementations must NOT throw C++ exceptions. All errors
// must be reported by returning an absl::Status inside the absl::StatusOr.
// Throwing an exception will cause abnormal termination of the application.
// https://openfeature.dev/specification/sections/providers#21-feature-provider-interface
class FeatureProvider {
public:
virtual ~FeatureProvider() = default;
virtual Metadata GetMetadata() const = 0;
virtual std::unique_ptr<BoolResolutionDetails> GetBooleanEvaluation(
std::string_view flag, bool default_value,
const EvaluationContext& ctx) = 0;
virtual std::unique_ptr<StringResolutionDetails> GetStringEvaluation(
std::string_view flag, std::string_view default_value,
const EvaluationContext& ctx) = 0;
virtual std::unique_ptr<IntResolutionDetails> GetIntegerEvaluation(
std::string_view flag, int64_t default_value,
const EvaluationContext& ctx) = 0;
virtual std::unique_ptr<DoubleResolutionDetails> GetDoubleEvaluation(
std::string_view flag, double default_value,
const EvaluationContext& ctx) = 0;
virtual std::unique_ptr<ObjectResolutionDetails> GetObjectEvaluation(
std::string_view flag, Value default_value,
const EvaluationContext& ctx) = 0;
virtual absl::StatusOr<std::unique_ptr<BoolResolutionDetails>>
GetBooleanEvaluation(std::string_view flag, bool default_value,
const EvaluationContext& ctx) = 0;
virtual absl::StatusOr<std::unique_ptr<StringResolutionDetails>>
GetStringEvaluation(std::string_view flag, std::string_view default_value,
const EvaluationContext& ctx) = 0;
virtual absl::StatusOr<std::unique_ptr<IntResolutionDetails>>
GetIntegerEvaluation(std::string_view flag, int64_t default_value,
const EvaluationContext& ctx) = 0;
virtual absl::StatusOr<std::unique_ptr<DoubleResolutionDetails>>
GetDoubleEvaluation(std::string_view flag, double default_value,
const EvaluationContext& ctx) = 0;
virtual absl::StatusOr<std::unique_ptr<ObjectResolutionDetails>>
GetObjectEvaluation(std::string_view flag, Value default_value,
const EvaluationContext& ctx) = 0;
virtual absl::Status Init(const EvaluationContext& ctx) {
return absl::OkStatus();
}
Expand Down
4 changes: 3 additions & 1 deletion test/e2e/BUILD
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ cc_library(
"//openfeature:reason",
"//openfeature:value",
"//openfeature:resolution_details",
"@googletest//:gtest",
"@googletest//:gtest",
"@abseil-cpp//absl/status",
"@abseil-cpp//absl/status:statusor",
],
)

Expand Down
10 changes: 5 additions & 5 deletions test/e2e/context_storing_provider.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ openfeature::Metadata ContextStoringProvider::GetMetadata() const {
return openfeature::Metadata{"ContextStoringProvider"};
}

std::unique_ptr<openfeature::BoolResolutionDetails>
absl::StatusOr<std::unique_ptr<openfeature::BoolResolutionDetails>>
ContextStoringProvider::GetBooleanEvaluation(
std::string_view key, bool default_value,
const openfeature::EvaluationContext& ctx) {
Expand All @@ -25,7 +25,7 @@ ContextStoringProvider::GetBooleanEvaluation(
openfeature::FlagMetadata{}, std::nullopt, "");
}

std::unique_ptr<openfeature::StringResolutionDetails>
absl::StatusOr<std::unique_ptr<openfeature::StringResolutionDetails>>
ContextStoringProvider::GetStringEvaluation(
std::string_view key, std::string_view default_value,
const openfeature::EvaluationContext& ctx) {
Expand All @@ -36,7 +36,7 @@ ContextStoringProvider::GetStringEvaluation(
openfeature::FlagMetadata{}, std::nullopt, "");
}

std::unique_ptr<openfeature::IntResolutionDetails>
absl::StatusOr<std::unique_ptr<openfeature::IntResolutionDetails>>
ContextStoringProvider::GetIntegerEvaluation(
std::string_view key, int64_t default_value,
const openfeature::EvaluationContext& ctx) {
Expand All @@ -47,7 +47,7 @@ ContextStoringProvider::GetIntegerEvaluation(
openfeature::FlagMetadata{}, std::nullopt, "");
}

std::unique_ptr<openfeature::DoubleResolutionDetails>
absl::StatusOr<std::unique_ptr<openfeature::DoubleResolutionDetails>>
ContextStoringProvider::GetDoubleEvaluation(
std::string_view key, double default_value,
const openfeature::EvaluationContext& ctx) {
Expand All @@ -58,7 +58,7 @@ ContextStoringProvider::GetDoubleEvaluation(
openfeature::FlagMetadata{}, std::nullopt, "");
}

std::unique_ptr<openfeature::ObjectResolutionDetails>
absl::StatusOr<std::unique_ptr<openfeature::ObjectResolutionDetails>>
ContextStoringProvider::GetObjectEvaluation(
std::string_view key, const openfeature::Value default_value,
const openfeature::EvaluationContext& ctx) {
Expand Down
32 changes: 17 additions & 15 deletions test/e2e/context_storing_provider.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
#include <string>
#include <string_view>

#include "absl/status/status.h"
#include "absl/status/statusor.h"
#include "openfeature/evaluation_context.h"
#include "openfeature/metadata.h"
#include "openfeature/provider.h"
Expand All @@ -22,25 +24,25 @@ class ContextStoringProvider : public openfeature::FeatureProvider {

openfeature::Metadata GetMetadata() const override;

std::unique_ptr<openfeature::BoolResolutionDetails> GetBooleanEvaluation(
std::string_view key, bool default_value,
const openfeature::EvaluationContext& ctx) override;
absl::StatusOr<std::unique_ptr<openfeature::BoolResolutionDetails>>
GetBooleanEvaluation(std::string_view key, bool default_value,
const openfeature::EvaluationContext& ctx) override;

std::unique_ptr<openfeature::StringResolutionDetails> GetStringEvaluation(
std::string_view key, std::string_view default_value,
const openfeature::EvaluationContext& ctx) override;
absl::StatusOr<std::unique_ptr<openfeature::StringResolutionDetails>>
GetStringEvaluation(std::string_view key, std::string_view default_value,
const openfeature::EvaluationContext& ctx) override;

std::unique_ptr<openfeature::IntResolutionDetails> GetIntegerEvaluation(
std::string_view key, int64_t default_value,
const openfeature::EvaluationContext& ctx) override;
absl::StatusOr<std::unique_ptr<openfeature::IntResolutionDetails>>
GetIntegerEvaluation(std::string_view key, int64_t default_value,
const openfeature::EvaluationContext& ctx) override;

std::unique_ptr<openfeature::DoubleResolutionDetails> GetDoubleEvaluation(
std::string_view key, double default_value,
const openfeature::EvaluationContext& ctx) override;
absl::StatusOr<std::unique_ptr<openfeature::DoubleResolutionDetails>>
GetDoubleEvaluation(std::string_view key, double default_value,
const openfeature::EvaluationContext& ctx) override;

std::unique_ptr<openfeature::ObjectResolutionDetails> GetObjectEvaluation(
std::string_view key, openfeature::Value default_value,
const openfeature::EvaluationContext& ctx) override;
absl::StatusOr<std::unique_ptr<openfeature::ObjectResolutionDetails>>
GetObjectEvaluation(std::string_view key, openfeature::Value default_value,
const openfeature::EvaluationContext& ctx) override;
};

} // namespace openfeature_e2e
Expand Down
Loading
Loading