feat(workflow): Make OrganizationIncidentDetailsEndpoint.get support single-written workflows#111588
feat(workflow): Make OrganizationIncidentDetailsEndpoint.get support single-written workflows#111588
Conversation
tests/sentry/incidents/endpoints/test_organization_incident_details.py
Outdated
Show resolved
Hide resolved
ceorourke
left a comment
There was a problem hiding this comment.
lgtm, the bot complaining about put requests is fine for now
| try: | ||
| igop = IncidentGroupOpenPeriod.objects.select_related( | ||
| "group_open_period__project", | ||
| "group_open_period__group", | ||
| ).get( | ||
| incident_identifier=int_id, | ||
| group_open_period__project__organization=organization, | ||
| ) | ||
| gop = igop.group_open_period | ||
| except IncidentGroupOpenPeriod.DoesNotExist: |
There was a problem hiding this comment.
Bug: A query using a fake ID will cause an integer overflow because the incident_identifier field is a 32-bit IntegerField but the ID can exceed 10^10, leading to a DataError.
Severity: HIGH
Suggested Fix
Change the type of the incident_identifier field in the IncidentGroupOpenPeriod model from models.IntegerField to models.BigIntegerField. This will ensure the field can accommodate the large numerical values of the fake IDs without causing a database overflow error.
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/incidents/endpoints/organization_incident_details.py#L81-L90
Potential issue: The code generates a "fake ID" for an `IncidentGroupOpenPeriod` by
adding an offset of 10^10 to a `GroupOpenPeriod` object's ID. This large number is then
used to query the `incident_identifier` field in the database. However, the
`incident_identifier` field is defined as a 32-bit `IntegerField`, which has a maximum
value of approximately 2.1 billion. Since the fake ID will always be at least 10
billion, it exceeds the capacity of the field. This mismatch causes the database to
raise a `DataError` for an "integer out of range". The existing error handling does not
catch this specific exception, leading to an unhandled error and a 500 Internal Server
Error response.
There was a problem hiding this comment.
There's a passing test that hits this case, so I'm pretty sure postgres doesn't overflow when simply doing an comparison. Valid point, though: fake ids probably should be checked first as a matter of idiom.
Part of the project described in src/sentry/workflow_engine/docs/legacy_backport.md.
Fixes ISWF-2296.