nit(aci): handle NaN aggregation value in subscription processor #105704
Conversation
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
|
|
||
| @patch("sentry.incidents.subscription_processor.metrics") | ||
| def test_invalid_aggregation_value(self, mock_metrics: MagicMock) -> None: | ||
| self.send_update("nan") |
There was a problem hiding this comment.
I think you might want the value to be float("nan") to actually make it a nan rather than a string
There was a problem hiding this comment.
strings are by default not numbers so I think any string is okay (if we do the math.isnan check)
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| aggregation_value = self.get_aggregation_value(subscription_update, comparison_delta) | ||
|
|
||
| if aggregation_value is None: | ||
| if aggregation_value is None or math.isnan(aggregation_value): |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| aggregation_value = self.get_aggregation_value(subscription_update, comparison_delta) | ||
|
|
||
| if aggregation_value is None: | ||
| if aggregation_value is None or math.isnan(aggregation_value): |
There was a problem hiding this comment.
math.isnan() crashes on non-numeric aggregation values
High Severity
The math.isnan() function requires a numeric argument and raises TypeError for non-numeric types. When aggregation_value is a non-numeric, non-None value (like a string), the condition aggregation_value is None is False, causing math.isnan(aggregation_value) to be evaluated and crash. The test sends "not a number" string which would trigger this TypeError. A type check is needed before calling math.isnan(), or use a pattern like isinstance(aggregation_value, (int, float)) and math.isnan(aggregation_value).
Additional Locations (1)
There was a problem hiding this comment.
I don't think it's ever possible to not send a numeric value; fixed the test
Since we know before data condition evaluation that this is the case, skip processing and return early.