fix(workflow-engine): Handle missing ActionTarget in combined-rules serializer#119075
fix(workflow-engine): Handle missing ActionTarget in combined-rules serializer#119075sentry[bot] wants to merge 11 commits into
Conversation
| if target_type is not None | ||
| else None, | ||
| "targetIdentifier": get_identifier_from_action( | ||
| type_value, str(target_identifier), target_display |
There was a problem hiding this comment.
Bug: Calling str(target_identifier) when target_identifier is None leads to a ValueError when int() is called on the resulting string "None" for certain action types.
Severity: HIGH
Suggested Fix
Conditionally handle the case where target_identifier is None. Before converting it to a string, check if it is None and pass None or a sensible default to get_identifier_from_action. The get_identifier_from_action function should also be updated to handle a None or default value gracefully instead of assuming a valid integer 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/incidents/endpoints/serializers/workflow_engine_action.py#L79-L82
Potential issue: For PagerDuty or Sentry App actions, if the `target_identifier` is
missing from the action's configuration, it defaults to `None`. The code then
unconditionally converts this value to a string, resulting in `"None"`. This string is
passed to the `get_identifier_from_action` function, which attempts to cast it to an
integer using `int("None")`. This operation fails and raises a `ValueError`, causing a
server error during the serialization of the action.
Did we get this right? 👍 / 👎 to inform future reviews.
|
@sentry add logging for when this happens |
|
@sentry actually nvm just remove the tests |
|
@sentry can you remove logging |
|
@sentry actually no add logging |
|
@sentry actually remove logging again |
This PR addresses issue SENTRY-5QVV, where a
ValueError: None is not a valid ActionTargetwas occurring during the serialization of workflow engine actions.The root cause was that some
Actionmodel records, particularly those created via certain migration paths or for specific action types (like PagerDuty/OpsGenie), did not have atarget_typekey in theirconfigJSON. When theWorkflowEngineActionSerializerattempted to convert thisNonevalue to anActionTargetenum, it resulted in aValueError.The fix involves:
src/sentry/incidents/endpoints/serializers/workflow_engine_action.pyto ensure thatActionTarget(target_type)is only called whentarget_typeis notNone. Iftarget_typeisNone, the serializedtargetTypewill also beNone.logger.warningwhentarget_typeisNoneto provide visibility into these cases, including theaction_idandaction_typefor debugging.Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.
Fixes SENTRY-5QVV
Comment
@sentry <feedback>on this PR to have Autofix iterate on the changes.