fix(ACI): Migrate fallthroughType to fallthrough_type #103764
Conversation
github-actions
bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| def migrate_fallthrough_type(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: | ||
| Action = apps.get_model("workflow_engine", "Action") | ||
| for action in RangeQuerySetWrapper(Action.objects.all()): | ||
| if action.data.get("fallthroughType"): | ||
| new_data = action.data.copy() | ||
| del new_data["fallthroughType"] | ||
| new_data["fallthrough_type"] = action.data["fallthroughType"] | ||
| action.data = new_data | ||
| action.save() |
There was a problem hiding this comment.
Bug: Migration renames fallthroughType to fallthrough_type, but EmailDataBlob expects fallthroughType, causing a TypeError.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The database migration renames the action.data field fallthroughType to fallthrough_type. However, the EmailDataBlob dataclass, used in EmailIssueAlertHandler.get_additional_fields(), still defines the field as fallthroughType. After the migration, when EmailDataBlob(**action.data) is called, it will receive fallthrough_type as an unexpected keyword argument, leading to a TypeError. This will cause all email-based workflow actions to crash.
💡 Suggested Fix
Update the EmailDataBlob dataclass field from fallthroughType to fallthrough_type to align with the migrated data key.
🤖 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/workflow_engine/migrations/0103_action_data_fallthrough_type.py#L11-L19
Potential issue: The database migration renames the `action.data` field
`fallthroughType` to `fallthrough_type`. However, the `EmailDataBlob` dataclass, used in
`EmailIssueAlertHandler.get_additional_fields()`, still defines the field as
`fallthroughType`. After the migration, when `EmailDataBlob(**action.data)` is called,
it will receive `fallthrough_type` as an unexpected keyword argument, leading to a
`TypeError`. This will cause all email-based workflow actions to crash.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference_id: 2862609
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| migrations.RunPython( | ||
| code=migrate_fallthrough_type, | ||
| reverse_code=migrations.RunPython.noop, | ||
| hints={"tables": ["workflow_engine_workflowfirehistory"]}, |
There was a problem hiding this comment.
Bug: Migration hints specify wrong table name
The migration operates on the Action model but specifies workflow_engine_workflowfirehistory in the hints. This incorrect table name can cause database routing and failover issues in sharded/replicated environments. The hints should specify workflow_engine_action to match the table being modified.
| def migrate_fallthrough_type(apps: StateApps, schema_editor: BaseDatabaseSchemaEditor) -> None: | ||
| Action = apps.get_model("workflow_engine", "Action") | ||
| for action in RangeQuerySetWrapper(Action.objects.all()): | ||
| if action.data.get("fallthroughType"): |
There was a problem hiding this comment.
Bug: Migration condition skips falsy fallthroughType values
The migration checks if action.data.get("fallthroughType"): which will silently skip any action where fallthroughType exists but is falsy (e.g., None, 0, False, empty string). This could result in data loss. Should use if "fallthroughType" in action.data: to migrate all occurrences of the field regardless of its value.
|
This PR has a migration; here is the generated SQL for for --
-- Raw Python operation
--
-- THIS OPERATION CANNOT BE WRITTEN AS SQL |
Companion PR to #103694 to migrate
fallthroughTypetofallthrough_typeinAction.data. Other PR must be merged first.