Skip to content

Commit 95094f9

Browse files
committed
fix(feature_flags): keep exception handlers reachable for missing context keys
The missing-key guard returned False before the comparator ran, so a comparator that raises for a None context value (ANY_IN_VALUE, ALL_IN_VALUE, NONE_IN_VALUE) never reached a registered validation_exception_handler. That was a regression from develop, where the handler was called and its result used. Run the comparator first and only then apply the missing-key rule. Any exception still flows through the existing handler lookup unchanged; the only difference from develop is that a comparator returning True for an absent key now yields False.
1 parent 80e50af commit 95094f9

2 files changed

Lines changed: 92 additions & 13 deletions

File tree

‎aws_lambda_powertools/utilities/feature_flags/feature_flags.py‎

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,26 @@ def __init__(self, store: StoreProvider, logger: logging.Logger | Logger | None
8282
self.logger = logger or logging.getLogger(__name__)
8383
self._exception_handlers: dict[Exception, Callable] = {}
8484

85-
def _match_by_action(self, action: str, condition_value: Any, context_value: Any) -> bool:
85+
def _match_by_action(
86+
self,
87+
action: str,
88+
condition_value: Any,
89+
context_value: Any,
90+
context_key_present: bool = True,
91+
) -> bool:
8692
try:
8793
func = RULE_ACTION_MAPPING.get(action, lambda a, b: False)
88-
return func(context_value, condition_value)
94+
matched = func(context_value, condition_value)
95+
96+
if not context_key_present:
97+
# A key absent from the context never satisfies a condition. Without this, `None` would be
98+
# compared as a regular value and negative actions (NOT_EQUALS, NOT_IN, ...) would match.
99+
# We still run the comparator first so that any exception it raises (e.g. ANY_IN_VALUE on a
100+
# non-list) reaches registered validation exception handlers exactly as it did before.
101+
self.logger.debug(f"context key not present, condition does not match: action={action}")
102+
return False
103+
104+
return matched
89105
except Exception as exc:
90106
self.logger.debug(f"caught exception while matching action: action={action}, exception={str(exc)}")
91107

@@ -118,6 +134,7 @@ def _evaluate_conditions(
118134
cond_key = condition.get(schema.CONDITION_KEY, "")
119135
cond_action = condition.get(schema.CONDITION_ACTION, "")
120136
cond_value = condition.get(schema.CONDITION_VALUE)
137+
context_key_present = True
121138

122139
# time based rule actions have no user context. the context is the condition key
123140
if cond_action in (
@@ -126,18 +143,16 @@ def _evaluate_conditions(
126143
schema.RuleAction.SCHEDULE_BETWEEN_DAYS_OF_WEEK.value,
127144
):
128145
context_value = cond_key # e.g., CURRENT_TIME
129-
elif cond_key not in context:
130-
# A key absent from the context never satisfies a condition. Without this guard, `None`
131-
# would be compared as a regular value and negative actions (NOT_EQUALS, NOT_IN, ...) would match.
132-
self.logger.debug(
133-
f"rule did not match, context key not found, rule_name={rule_name}, "
134-
f"rule_value={rule_match_value}, name={feature_name}, key={cond_key}",
135-
)
136-
return False
137146
else:
138-
context_value = context[cond_key]
139-
140-
if not self._match_by_action(action=cond_action, condition_value=cond_value, context_value=context_value):
147+
context_key_present = cond_key in context
148+
context_value = context.get(cond_key)
149+
150+
if not self._match_by_action(
151+
action=cond_action,
152+
condition_value=cond_value,
153+
context_value=context_value,
154+
context_key_present=context_key_present,
155+
):
141156
self.logger.debug(
142157
f"rule did not match action, rule_name={rule_name}, rule_value={rule_match_value}, "
143158
f"name={feature_name}, context_value={str(context_value)} ",

‎tests/functional/feature_flags/_boto3/test_feature_flags.py‎

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1767,3 +1767,67 @@ def catch_exception(exc):
17671767
context={"tenant_id": "not a list value"},
17681768
default=False,
17691769
)
1770+
1771+
1772+
def test_flags_missing_context_key_still_invokes_validation_exception_handler(mocker, config):
1773+
# GIVEN an ANY_IN_VALUE rule and a handler registered for the ValueError the comparator raises
1774+
# when the context value is not a list
1775+
mocked_app_config_schema = {
1776+
"my_feature": {
1777+
"default": False,
1778+
"rules": {
1779+
"tenant_id is in allowed list": {
1780+
"when_match": True,
1781+
"conditions": [
1782+
{
1783+
"action": RuleAction.ANY_IN_VALUE.value,
1784+
"key": "tenant_id",
1785+
"value": ["Akua", "John", "Maria", "Pat"],
1786+
},
1787+
],
1788+
},
1789+
},
1790+
},
1791+
}
1792+
feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
1793+
handled = []
1794+
1795+
@feature_flags.validation_exception_handler(ValueError)
1796+
def handle_invalid_context(exc):
1797+
handled.append(exc)
1798+
return True
1799+
1800+
# WHEN the context does not carry the key at all
1801+
toggle = feature_flags.evaluate(name="my_feature", context={}, default=False)
1802+
1803+
# THEN the handler is still called and its result is honoured, as it was before missing keys were guarded
1804+
assert len(handled) == 1
1805+
assert toggle is True
1806+
1807+
1808+
def test_flags_missing_context_key_no_match_without_handler_for_raising_action(mocker, config):
1809+
# GIVEN an ANY_IN_VALUE rule and no exception handler registered
1810+
mocked_app_config_schema = {
1811+
"my_feature": {
1812+
"default": False,
1813+
"rules": {
1814+
"tenant_id is in allowed list": {
1815+
"when_match": True,
1816+
"conditions": [
1817+
{
1818+
"action": RuleAction.ANY_IN_VALUE.value,
1819+
"key": "tenant_id",
1820+
"value": ["Akua", "John", "Maria", "Pat"],
1821+
},
1822+
],
1823+
},
1824+
},
1825+
},
1826+
}
1827+
feature_flags = init_feature_flags(mocker, mocked_app_config_schema, config)
1828+
1829+
# WHEN the context does not carry the key at all
1830+
toggle = feature_flags.evaluate(name="my_feature", context={}, default=False)
1831+
1832+
# THEN the rule does not match
1833+
assert toggle is False

0 commit comments

Comments
 (0)