Skip to content

Commit 7e18bc1

Browse files
thomaspoignantthomaspoignant
authored
fix(ofrep): handle 401, 403, 404 and 5xx errors explicitly and add tests (#346)
* fix(ofrep): handle 401, 403, 404 and 5xx errors explicitly and add tests Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org> * refactor(ofrep): extract _raise_for_http_status and _raise_for_error_code to satisfy ruff complexity Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org> * refactor(ofrep): use elif chain and _HTTP_AUTH_ERRORS dict in _raise_for_http_status Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org> * fix(ofrep): use data.get for errorCode fallback to GENERAL Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org> * fix(ofrep): handle invalid errorCode with try/except, fallback to GENERAL Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org> * Adding warn message Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org> --------- Signed-off-by: Thomas Poignant <thomas.poignant@gofeatureflag.org>
1 parent 0ecd1d4 commit 7e18bc1

File tree

2 files changed

+121
-11
lines changed

2 files changed

+121
-11
lines changed

‎providers/openfeature-provider-ofrep/src/openfeature/contrib/provider/ofrep/__init__.py‎

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import logging
12
import re
23
from collections.abc import Callable, Mapping, Sequence
34
from datetime import datetime, timedelta, timezone
@@ -30,12 +31,16 @@
3031

3132
__all__ = ["OFREPProvider"]
3233

34+
logger = logging.getLogger("openfeature.contrib.ofrep")
35+
3336

3437
TypeMap = dict[
3538
FlagType,
3639
type[bool] | type[int] | type[float] | type[str] | tuple[type[dict], type[list]],
3740
]
3841

42+
_HTTP_AUTH_ERRORS: dict[int, str] = {401: "unauthorized", 403: "forbidden"}
43+
3944

4045
class OFREPProvider(AbstractProvider):
4146
def __init__(
@@ -161,24 +166,22 @@ def _handle_error(self, exception: requests.RequestException) -> NoReturn:
161166
if response is None:
162167
raise GeneralError(str(exception)) from exception
163168

164-
if response.status_code == 429:
165-
retry_after = response.headers.get("Retry-After")
166-
self.retry_after = _parse_retry_after(retry_after)
167-
raise GeneralError(
168-
f"Rate limited, retry after: {retry_after}"
169-
) from exception
170-
169+
self._raise_for_http_status(response, exception)
170+
# Fallthrough: parse JSON and raise based on error code
171171
try:
172172
data = response.json()
173173
except JSONDecodeError:
174174
raise ParseError(str(exception)) from exception
175175

176-
error_code = ErrorCode(data["errorCode"])
176+
try:
177+
error_code = ErrorCode(data["errorCode"])
178+
except ValueError:
179+
logger.warning(
180+
"Invalid errorCode %r, falling back to GENERAL", data.get("errorCode")
181+
)
182+
error_code = ErrorCode.GENERAL
177183
error_details = data["errorDetails"]
178184

179-
if response.status_code == 404:
180-
raise FlagNotFoundError(error_details) from exception
181-
182185
if error_code == ErrorCode.PARSE_ERROR:
183186
raise ParseError(error_details) from exception
184187
if error_code == ErrorCode.TARGETING_KEY_MISSING:
@@ -190,6 +193,32 @@ def _handle_error(self, exception: requests.RequestException) -> NoReturn:
190193

191194
raise OpenFeatureError(error_code, error_details) from exception
192195

196+
def _raise_for_http_status(
197+
self,
198+
response: requests.Response,
199+
exception: requests.RequestException,
200+
) -> None:
201+
status = response.status_code
202+
203+
if status == 429:
204+
retry_after = response.headers.get("Retry-After")
205+
self.retry_after = _parse_retry_after(retry_after)
206+
raise GeneralError(
207+
f"Rate limited, retry after: {retry_after}"
208+
) from exception
209+
elif status in _HTTP_AUTH_ERRORS:
210+
raise OpenFeatureError(
211+
ErrorCode.GENERAL, _HTTP_AUTH_ERRORS[status]
212+
) from exception
213+
elif status == 404:
214+
try:
215+
error_details = response.json()["errorDetails"]
216+
except (JSONDecodeError, KeyError):
217+
error_details = response.text
218+
raise FlagNotFoundError(error_details) from exception
219+
elif status > 400:
220+
raise OpenFeatureError(ErrorCode.GENERAL, response.text) from exception
221+
193222

194223
def _build_request_data(
195224
evaluation_context: EvaluationContext | None,

‎providers/openfeature-provider-ofrep/tests/test_provider.py‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,15 @@
1+
import logging
2+
13
import pytest
24

35
from openfeature.contrib.provider.ofrep import OFREPProvider
46
from openfeature.evaluation_context import EvaluationContext
57
from openfeature.exception import (
8+
ErrorCode,
69
FlagNotFoundError,
710
GeneralError,
811
InvalidContextError,
12+
OpenFeatureError,
913
ParseError,
1014
TypeMismatchError,
1115
)
@@ -83,6 +87,61 @@ def test_provider_flag_not_found(ofrep_provider, requests_mock):
8387
ofrep_provider.resolve_boolean_details("flag_key", False)
8488

8589

90+
def test_provider_unauthorized(ofrep_provider, requests_mock):
91+
requests_mock.post(
92+
"http://localhost:8080/ofrep/v1/evaluate/flags/flag_key",
93+
status_code=401,
94+
text="unauthorized",
95+
)
96+
97+
with pytest.raises(OpenFeatureError) as exc_info:
98+
ofrep_provider.resolve_boolean_details("flag_key", False)
99+
100+
assert exc_info.value.error_code == ErrorCode.GENERAL
101+
assert exc_info.value.error_message == "unauthorized"
102+
103+
104+
def test_provider_forbidden(ofrep_provider, requests_mock):
105+
requests_mock.post(
106+
"http://localhost:8080/ofrep/v1/evaluate/flags/flag_key",
107+
status_code=403,
108+
text="forbidden",
109+
)
110+
111+
with pytest.raises(OpenFeatureError) as exc_info:
112+
ofrep_provider.resolve_boolean_details("flag_key", False)
113+
114+
assert exc_info.value.error_code == ErrorCode.GENERAL
115+
assert exc_info.value.error_message == "forbidden"
116+
117+
118+
def test_provider_flag_not_found_invalid_json(ofrep_provider, requests_mock):
119+
"""Test 404 with non-JSON response falls back to response text for error details"""
120+
requests_mock.post(
121+
"http://localhost:8080/ofrep/v1/evaluate/flags/flag_key",
122+
status_code=404,
123+
text="Flag not found - plain text response",
124+
)
125+
126+
with pytest.raises(FlagNotFoundError, match="Flag not found - plain text response"):
127+
ofrep_provider.resolve_boolean_details("flag_key", False)
128+
129+
130+
def test_provider_server_error(ofrep_provider, requests_mock):
131+
"""Test generic OpenFeatureError for status codes > 400 (e.g. 500, 502)"""
132+
requests_mock.post(
133+
"http://localhost:8080/ofrep/v1/evaluate/flags/flag_key",
134+
status_code=500,
135+
text="Internal Server Error",
136+
)
137+
138+
with pytest.raises(OpenFeatureError) as exc_info:
139+
ofrep_provider.resolve_boolean_details("flag_key", False)
140+
141+
assert exc_info.value.error_code == ErrorCode.GENERAL
142+
assert exc_info.value.error_message == "Internal Server Error"
143+
144+
86145
def test_provider_invalid_context(ofrep_provider, requests_mock):
87146
requests_mock.post(
88147
"http://localhost:8080/ofrep/v1/evaluate/flags/flag_key",
@@ -98,6 +157,28 @@ def test_provider_invalid_context(ofrep_provider, requests_mock):
98157
ofrep_provider.resolve_boolean_details("flag_key", False)
99158

100159

160+
def test_provider_invalid_error_code_logs_warning(
161+
ofrep_provider, requests_mock, caplog
162+
):
163+
requests_mock.post(
164+
"http://localhost:8080/ofrep/v1/evaluate/flags/flag_key",
165+
status_code=400,
166+
json={
167+
"key": "flag_key",
168+
"errorCode": "UNKNOWN_CODE",
169+
"errorDetails": "Something went wrong",
170+
},
171+
)
172+
173+
with (
174+
caplog.at_level(logging.WARNING, logger="openfeature.contrib.ofrep"),
175+
pytest.raises(GeneralError),
176+
):
177+
ofrep_provider.resolve_boolean_details("flag_key", False)
178+
179+
assert any("UNKNOWN_CODE" in record.message for record in caplog.records)
180+
181+
101182
def test_provider_invalid_response(ofrep_provider, requests_mock):
102183
requests_mock.post(
103184
"http://localhost:8080/ofrep/v1/evaluate/flags/flag_key", text="invalid"

0 commit comments

Comments
 (0)