Skip to content

Comments

fix(ofrep): handle 401, 403, 404 and 5xx errors explicitly and add tests#346

Open
thomaspoignant wants to merge 5 commits intomainfrom
fix-ofrep-parse-error
Open

fix(ofrep): handle 401, 403, 404 and 5xx errors explicitly and add tests#346
thomaspoignant wants to merge 5 commits intomainfrom
fix-ofrep-parse-error

Conversation

@thomaspoignant
Copy link
Member

@thomaspoignant thomaspoignant commented Feb 20, 2026

Description

What was the problem?

  • The OFREP provider did not explicitly handle 401, 403, 404, or generic 5xx responses before attempting to parse JSON. This could cause parse errors when servers returned non-JSON error bodies.
  • For 404, responses with invalid JSON would fail instead of falling back to the response text.
  • We had parse errors when it was not a parsing issue.

How is it resolved?

  • Add explicit handling for 401 (unauthorized), 403 (forbidden), 404 (flag not found with JSON/text fallback), and status codes > 400 before the generic JSON parsing path.
  • These cases now raise appropriate OpenFeatureError or FlagNotFoundError without attempting JSON parse.

How can we test?

  • Run pytest tests/test_provider.py -v in providers/openfeature-provider-ofrep/.

Checklist

  • I have followed the CONTRIBUTING guidelines.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.

@gemini-code-assist
Copy link
Contributor

Summary of Changes

Hello @thomaspoignant, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request enhances the robustness of the OFREP provider by introducing specific error handling for common HTTP error codes. The changes ensure that the provider can correctly interpret and propagate errors like unauthorized access, forbidden requests, flag not found scenarios, and server-side issues, even when the server's error responses are not in JSON format. This improves the reliability and user experience by providing more accurate and actionable error messages.

Highlights

  • Explicit Error Handling: Added explicit handling for HTTP status codes 401 (unauthorized), 403 (forbidden), 404 (flag not found), and generic 5xx errors within the OFREP provider. This prevents JSON parsing errors when non-JSON error bodies are returned by the server.
  • Improved 404 Handling: The 404 (Flag Not Found) error handling now attempts to parse JSON for error details but gracefully falls back to using the raw response text if the response body is not valid JSON, ensuring robust error reporting.
  • New Test Cases: Comprehensive test cases have been added to validate the new error handling logic for 401, 403, 404 (with invalid JSON), and 5xx responses, ensuring the provider behaves as expected under various error conditions.
Changelog
  • providers/openfeature-provider-ofrep/src/openfeature/contrib/provider/ofrep/init.py
    • Introduced explicit checks for HTTP status codes 401, 403, 404, and generic 5xx errors before attempting JSON parsing.
    • Implemented specific error raising for 401 (unauthorized) and 403 (forbidden) as OpenFeatureError.
    • Modified 404 handling to attempt JSON parsing for errorDetails and fall back to raw response text if JSON parsing fails, raising FlagNotFoundError.
    • Added a general OpenFeatureError for status codes greater than 400 not explicitly handled.
    • Removed a redundant 404 check that was previously located after the initial JSON parsing attempt.
  • providers/openfeature-provider-ofrep/tests/test_provider.py
    • Imported ErrorCode and OpenFeatureError for use in new test cases.
    • Added test_provider_unauthorized to verify correct error handling for 401 responses.
    • Added test_provider_forbidden to verify correct error handling for 403 responses.
    • Added test_provider_flag_not_found_invalid_json to test 404 responses with non-JSON bodies, ensuring the fallback to response text.
    • Added test_provider_server_error to test generic 5xx responses and ensure they raise OpenFeatureError.
Activity
  • The author has provided a detailed description of the problem and resolution.
  • The author has outlined how to test the changes by running pytest tests/test_provider.py -v.
  • The author has included a checklist, indicating an intention to follow contribution guidelines and add tests, though the checklist items are currently unchecked.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@codecov
Copy link

codecov bot commented Feb 20, 2026

Codecov Report

❌ Patch coverage is 90.90909% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.44%. Comparing base (80da6b9) to head (4af0a11).

Files with missing lines Patch % Lines
...src/openfeature/contrib/provider/ofrep/__init__.py 90.90% 2 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main     #346       +/-   ##
===========================================
- Coverage   96.55%   86.44%   -10.12%     
===========================================
  Files           5        1        -4     
  Lines         174      118       -56     
===========================================
- Hits          168      102       -66     
- Misses          6       16       +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request effectively addresses an issue where non-JSON error responses from the OFREP provider could cause parsing errors. By explicitly handling 401, 403, 404, and 5xx status codes, the provider is now more robust. The added tests are thorough and cover the new error handling paths well. I have one suggestion to improve the structure and consistency of the error handling logic.

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
…code to satisfy ruff complexity

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
…for_http_status

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
…ERAL

Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants