Conversation
We're seeing an error (https://sentry.sentry.io/issues/7087482631/) where we're trying to encode ints that are OOB of the `int64` max. In this edge case, let's cast to `str`. (Better than dropping the data and fixes the problem for now. If it's a problem later we can look into it again.)
8617602 to
8c900f1
Compare
| elif isinstance(value, int): | ||
| # int_value is a signed int64, so it has a range of valid values. | ||
| # if value doesn't fit into an int64, cast it to string. | ||
| if abs(value) >= (2**63): |
There was a problem hiding this comment.
Bug: The boundary check abs(value) >= (2**63) incorrectly converts the minimum valid int64 value, -2**63, to a string due to an off-by-one error.
Severity: HIGH | Confidence: High
🔍 Detailed Analysis
The boundary check at line 47, if abs(value) >= (2**63):, is intended to handle integers outside the valid int64 range. However, it contains an off-by-one error. The valid range for a signed 64-bit integer is [-2**63, 2**63 - 1]. When the input value is exactly -2**63 (the minimum valid value), abs(value) becomes 2**63, causing the condition to evaluate to True. This incorrectly triggers the conversion of a valid int64 value to a string, which can lead to data type mismatches and processing errors in downstream systems that expect an integer.
💡 Suggested Fix
Modify the boundary check to correctly handle the full int64 range. The condition should be if value < -(2**63) or value >= 2**63:. This ensures that the minimum valid value, -2**63, is correctly identified as an integer and not converted to a string.
🤖 Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: src/sentry/eventstream/item_helpers.py#L47
Potential issue: The boundary check at line 47, `if abs(value) >= (2**63):`, is intended
to handle integers outside the valid `int64` range. However, it contains an off-by-one
error. The valid range for a signed 64-bit integer is `[-2**63, 2**63 - 1]`. When the
input `value` is exactly `-2**63` (the minimum valid value), `abs(value)` becomes
`2**63`, causing the condition to evaluate to `True`. This incorrectly triggers the
conversion of a valid `int64` value to a string, which can lead to data type mismatches
and processing errors in downstream systems that expect an integer.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 7204861
There was a problem hiding this comment.
I decided that this doesn't matter.
| elif isinstance(value, int): | ||
| # int_value is a signed int64, so it has a range of valid values. | ||
| # if value doesn't fit into an int64, cast it to string. | ||
| if abs(value) >= (2**63): |
There was a problem hiding this comment.
Bug: Off-by-one error for minimum int64 value
The boundary check abs(value) >= (2**63) incorrectly converts the minimum valid int64 value -2**63 to a string. Since abs(-2**63) equals 2**63, the condition triggers for this valid int64 value. The signed int64 range is [-2**63, 2**63 - 1], so -2**63 is valid. The check could use abs(value) > 2**63 - 1 to handle both boundaries correctly, though this is a minor edge case that doesn't cause errors—it just unnecessarily stringifies one extra value.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #104787 +/- ##
===========================================
- Coverage 80.47% 80.47% -0.01%
===========================================
Files 9363 9363
Lines 401849 401851 +2
Branches 25833 25833
===========================================
+ Hits 323389 323390 +1
- Misses 78018 78019 +1
Partials 442 442 |
We're seeing an error (https://sentry.sentry.io/issues/7087482631/) where we're trying to encode ints that are OOB of the
int64max.In this edge case, let's cast to
str. (Better than dropping the data and fixes the problem for now. If it's a problem later we can look into it again.)