fix!: provider exceptions crash the caller

[NeaguGeorgiana23]

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:

• Updates the FeatureProvider evaluation interface to return absl::StatusOr.
• Updates the client evaluation logic to gracefully handle failure statuses. If a provider returns an error status, the SDK safely intercepts it and returns the default value populated with a general error code.
• Prevents abnormal termination

Related Issues

Fixes #69

Follow-up Tasks

• Address remaining minor bugs from the Specification Compliance tracking issue.

[gemini-code-assist]

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.

[toddbaert]

Mostly LGTM. A few things:

• No tests for the 3 new branches in EvaluateFlag; cheap to add with MockFeatureProvider + a small throwing mock.
• Gemini's null-check point seems right
• I think this breaks the FeatureProvider (correct me if I'm wrong, I'm not that strong at C++). That's totally fine since we are not 1.0, but we should use a fix!: prefix.

I'll approve now, up to you to make these changes if you agree. ❤️

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 6cf860e5-23b1-4ca5-8334-65376303fcda

📥 Commits

Reviewing files that changed from the base of the PR and between 05d9241 and 3d200b5.

📒 Files selected for processing (1)

• test/client_api_test.cpp

🚧 Files skipped from review as they are similar to previous changes (1)

• test/client_api_test.cpp

---

📝 Walkthrough

Walkthrough

All FeatureProvider evaluation methods change their return type from std::unique_ptr<*ResolutionDetails> to absl::StatusOr<std::unique_ptr<*ResolutionDetails>>. ClientAPI::EvaluateFlag is updated to validate these results and catch exceptions. NoopProvider, InMemoryProvider, and test providers are updated to match. Tests are adapted to the new return type and four new ClientAPI error-handling tests are added.

Changes

StatusOr-based provider error propagation

Layer / File(s)	Summary
FeatureProvider interface and NoopProvider openfeature/provider.h, openfeature/noop_provider.h, openfeature/noop_provider.cpp, openfeature/BUILD	FeatureProvider gains absl/status/statusor.h, documents the no-throw contract, and changes all five Get*Evaluation virtual methods to return absl::StatusOr<std::unique_ptr<...>>. NoopProvider implements the same signature change. BUILD wires in the statusor dependency for both targets.
InMemoryProvider update openfeature/memory_provider/in_memory_provider.h, openfeature/memory_provider/in_memory_provider.cpp	Header adds statusor include, makes constructor explicit, moves status_ in-class initialization, clarifies UpdateFlags merge semantics, and changes all five override return types to absl::StatusOr. Evaluate template default_value parameter switches to const T&. Implementation removes the redundant status_ initializer and renames the flag-lookup iterator.
ClientAPI error handling openfeature/client_api.h	EvaluateFlag now wraps the provider call in try/catch, checks result.ok() and maps failures to ErrorCode::kGeneral, and converts both std::exception and unknown throws into safe fallback ResolutionDetailsType responses.
Mock and e2e test provider updates test/mocks/mock_feature_provider.h, test/e2e/context_storing_provider.h, test/e2e/context_storing_provider.cpp, test/e2e/BUILD	MockFeatureProvider MOCK_METHOD declarations and ContextStoringProvider method declarations and definitions are updated to return absl::StatusOr-wrapped results. e2e BUILD adds abseil status dependencies.
ClientAPI error handling tests test/client_api_test.cpp	Replaces namespace-wide using with explicit using declarations. Introduces unknown_exception_error constant and adds four new test cases verifying GetBooleanValue fallback when the provider fails via error status, null details, std::exception, and unknown-exception modes.
NoopProvider tests test/noop_provider_test.cpp	Updates all evaluation tests (boolean, string, integer, double, object) to extract StatusOr results using ok() assertion and dereference pattern. Introduces constexpr constants for double and object defaults.
InMemoryProvider tests test/memory_provider/in_memory_provider_test.cpp	Comprehensively updates all evaluation tests to the StatusOr ok()+dereference pattern. Refactors context-aware tests into a ContextAwareProviderTest fixture. Early error-path tests, update tests, and success tests for string/integer/double/object adopt constexpr constants.

Sequence Diagram

sequenceDiagram
  participant App
  participant ClientAPI
  participant FeatureProvider

  App->>ClientAPI: GetBooleanValue(flag, default)
  ClientAPI->>FeatureProvider: GetBooleanEvaluation(flag, default, ctx)

  alt provider returns error status
    FeatureProvider-->>ClientAPI: absl::StatusOr with !ok()
    ClientAPI-->>App: default value (ErrorCode::kGeneral)
  else provider returns null details
    FeatureProvider-->>ClientAPI: absl::StatusOr with nullptr
    ClientAPI-->>App: default value (ErrorCode::kGeneral)
  else provider throws std::exception
    FeatureProvider-->>ClientAPI: throws std::runtime_error
    ClientAPI-->>App: default value (ErrorCode::kGeneral + e.what())
  else provider throws unknown
    FeatureProvider-->>ClientAPI: throws non-std::exception
    ClientAPI-->>App: default value (ErrorCode::kGeneral)
  else success
    FeatureProvider-->>ClientAPI: absl::StatusOr with valid ResolutionDetails
    ClientAPI-->>App: resolved flag value
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~28 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 15.94% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'fix!: provider exceptions crash the caller' clearly and directly summarizes the main change—preventing provider exceptions from crashing the caller.
Description check	✅ Passed	The PR description clearly relates to the changeset by explaining how the refactoring addresses provider exceptions crashing the caller and compliance with Requirement 1.4.10.
Linked Issues check	✅ Passed	The PR fulfills the primary objective from issue #69 by refactoring the provider interface to use absl::StatusOr and adding exception handling in client evaluation logic to prevent exceptions from propagating and crashing the caller.
Out of Scope Changes check	✅ Passed	All changes are directly related to the stated objective of preventing provider exceptions from crashing the caller by refactoring the evaluation interface and error handling.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

test/memory_provider/in_memory_provider_test.cpp (1)

347-368: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Fail fixture setup fast when provider initialization fails.

Line 367 uses EXPECT_TRUE inside SetUp(). If init fails, tests continue with an invalid fixture and produce cascading failures. Use ASSERT_TRUE here.

🔧 Suggested change

-    EXPECT_TRUE(provider_.Init(empty_ctx_).ok());
+    ASSERT_TRUE(provider_.Init(empty_ctx_).ok());

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/memory_provider/in_memory_provider_test.cpp` around lines 347 - 368, In
the SetUp() method, replace the EXPECT_TRUE call that checks
provider_.Init(empty_ctx_).ok() with ASSERT_TRUE instead. EXPECT_TRUE is a soft
assertion that allows tests to continue even when initialization fails, leading
to cascading failures with invalid fixtures. Using ASSERT_TRUE ensures the test
fixture setup fails fast and stops immediately if provider initialization fails,
preventing invalid state from affecting subsequent tests.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@test/memory_provider/in_memory_provider_test.cpp`:
- Around line 274-302: The test assertions for flag1 remaining after UpdateFlags
is called conflict with the documented API contract in in_memory_provider.h
which states that UpdateFlags replaces existing flags with the new
configuration. Either remove the flag1 assertion block (lines checking
flag1_res->GetValue, flag1_res->GetReason, and flag1_res->GetVariant) to align
the test with the documented behavior that UpdateFlags replaces all flags, or
update the documentation in in_memory_provider.h to clarify that UpdateFlags
merges/retains existing flags not in the updated map. Ensure the implementation,
documentation, and test assertions are consistent with a single contract.

---

Nitpick comments:
In `@test/memory_provider/in_memory_provider_test.cpp`:
- Around line 347-368: In the SetUp() method, replace the EXPECT_TRUE call that
checks provider_.Init(empty_ctx_).ok() with ASSERT_TRUE instead. EXPECT_TRUE is
a soft assertion that allows tests to continue even when initialization fails,
leading to cascading failures with invalid fixtures. Using ASSERT_TRUE ensures
the test fixture setup fails fast and stops immediately if provider
initialization fails, preventing invalid state from affecting subsequent tests.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 551d67b7-b256-4a30-b33b-740c70450fe6

📥 Commits

Reviewing files that changed from the base of the PR and between aa27439 and f85c99f.

📒 Files selected for processing (14)

• openfeature/BUILD
• openfeature/client_api.h
• openfeature/memory_provider/in_memory_provider.cpp
• openfeature/memory_provider/in_memory_provider.h
• openfeature/noop_provider.cpp
• openfeature/noop_provider.h
• openfeature/provider.h
• test/client_api_test.cpp
• test/e2e/BUILD
• test/e2e/context_storing_provider.cpp
• test/e2e/context_storing_provider.h
• test/memory_provider/in_memory_provider_test.cpp
• test/mocks/mock_feature_provider.h
• test/noop_provider_test.cpp