Skip to content
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
from collections.abc import Mapping
from typing import Any, Sequence

Expand All @@ -18,6 +19,8 @@
from sentry.users.services.user.model import RpcUser
from sentry.workflow_engine.models import Action, ActionAlertRuleTriggerAction

logger = logging.getLogger(__name__)


class WorkflowEngineActionSerializer(Serializer[dict[str, Any]]):
def get_attrs(
Expand Down Expand Up @@ -61,6 +64,12 @@ def serialize(
target_identifier: str | None = obj.config.get("target_identifier")
target_display: str | None = obj.config.get("target_display")

if target_type is None:
logger.warning(
"workflow_engine_action.missing_target_type",
extra={"action_id": obj.id, "action_type": obj.type},
)

sentry_app_id = None
sentry_app_config = None
if obj.type == Action.Type.SENTRY_APP.value:
Expand All @@ -75,7 +84,9 @@ def serialize(
),
"alertRuleTriggerId": str(alert_rule_trigger_id),
"type": obj.type,
"targetType": ACTION_TARGET_TYPE_TO_STRING[ActionTarget(target_type)],
"targetType": ACTION_TARGET_TYPE_TO_STRING[ActionTarget(target_type)]
if target_type is not None
else None,
"targetIdentifier": get_identifier_from_action(
type_value, str(target_identifier), target_display
Comment on lines +88 to 91

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

),
Expand Down
Loading