Skip to content
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions src/sentry/integrations/messaging/metrics.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ class MessagingInteractionType(StrEnum):
VIEW_SUBMISSION = "VIEW_SUBMISSION"

# Automatic behaviors
PROCESS_SHARED_LINK = "PROCESS_SHARED_LINK"
UNFURL_LINK = "UNFURL_LINK"
UNFURL_ISSUES = "UNFURL_ISSUES"
UNFURL_METRIC_ALERTS = "UNFURL_METRIC_ALERTS"
Expand Down
9 changes: 4 additions & 5 deletions src/sentry/integrations/slack/unfurl/discover.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
from typing import Any
from urllib.parse import urlparse

from django.http.request import HttpRequest, QueryDict
from django.http.request import QueryDict

from sentry import analytics, features
from sentry.api import client
Expand Down Expand Up @@ -118,15 +118,14 @@ def is_aggregate(field: str) -> bool:


def unfurl_discover(
request: HttpRequest,
integration: Integration | RpcIntegration,
links: list[UnfurlableUrl],
user: User | RpcUser | None = None,
) -> UnfurledUrl:
event = MessagingInteractionEvent(
with MessagingInteractionEvent(
MessagingInteractionType.UNFURL_DISCOVER, SlackMessagingSpec(), user=user
)
with event.capture():
).capture() as lifecycle:
lifecycle.add_extras({"integration_id": integration.id})
return _unfurl_discover(integration, links, user)


Expand Down
9 changes: 3 additions & 6 deletions src/sentry/integrations/slack/unfurl/issues.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

import re

from django.http.request import HttpRequest

from sentry.integrations.messaging.metrics import (
MessagingInteractionEvent,
MessagingInteractionType,
Expand Down Expand Up @@ -33,7 +31,6 @@


def unfurl_issues(
request: HttpRequest,
integration: Integration | RpcIntegration,
links: list[UnfurlableUrl],
user: User | RpcUser | None = None,
Expand All @@ -43,10 +40,10 @@ def unfurl_issues(
for a particular issue by the URL of the yet-unfurled links a user included
in their Slack message.
"""
event = MessagingInteractionEvent(
with MessagingInteractionEvent(
MessagingInteractionType.UNFURL_ISSUES, SlackMessagingSpec(), user=user
)
with event.capture():
).capture() as lifecycle:
lifecycle.add_extras({"integration_id": integration.id})
return _unfurl_issues(integration, links)


Expand Down
9 changes: 4 additions & 5 deletions src/sentry/integrations/slack/unfurl/metric_alerts.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

import sentry_sdk
from django.db.models import Q
from django.http.request import HttpRequest, QueryDict
from django.http.request import QueryDict

from sentry import features
from sentry.api.serializers import serialize
Expand Down Expand Up @@ -52,15 +52,14 @@


def unfurl_metric_alerts(
request: HttpRequest,
integration: Integration | RpcIntegration,
links: list[UnfurlableUrl],
user: User | RpcUser | None = None,
) -> UnfurledUrl:
event = MessagingInteractionEvent(
with MessagingInteractionEvent(
MessagingInteractionType.UNFURL_METRIC_ALERTS, SlackMessagingSpec(), user=user
)
with event.capture():
).capture() as lifecycle:
lifecycle.add_extras({"integration_id": integration.id})
return _unfurl_metric_alerts(integration, links, user)


Expand Down
3 changes: 0 additions & 3 deletions src/sentry/integrations/slack/unfurl/types.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
from re import Pattern
from typing import Any, NamedTuple, Optional, Protocol

from django.http.request import HttpRequest

from sentry.integrations.models.integration import Integration
from sentry.integrations.services.integration import RpcIntegration
from sentry.users.models.user import User
Expand All @@ -30,7 +28,6 @@ class UnfurlableUrl(NamedTuple):
class HandlerCallable(Protocol):
def __call__(
self,
request: HttpRequest,
integration: Integration | RpcIntegration,
links: list[UnfurlableUrl],
user: User | RpcUser | None = None,
Expand Down
178 changes: 103 additions & 75 deletions src/sentry/integrations/slack/webhooks/event.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

import logging
from collections import defaultdict
from collections.abc import Mapping, MutableMapping
from collections.abc import Mapping
from typing import Any

import orjson
Expand Down Expand Up @@ -31,6 +31,7 @@
from sentry.integrations.slack.unfurl.types import LinkType, UnfurlableUrl
from sentry.integrations.slack.views.link_identity import build_linking_url
from sentry.organizations.services.organization import organization_service
from sentry.organizations.services.organization.model import RpcOrganization

from .base import SlackDMEndpoint
from .command import LINK_FROM_CHANNEL_MESSAGE
Expand Down Expand Up @@ -159,108 +160,135 @@ def on_message(self, request: Request, slack_request: SlackDMRequest) -> Respons

return self.respond()

def on_link_shared(self, request: Request, slack_request: SlackDMRequest) -> bool:
"""Returns true on success"""
matches: MutableMapping[LinkType, list[UnfurlableUrl]] = defaultdict(list)
def _get_unfurlable_links(
self,
request: Request,
slack_request: SlackDMRequest,
data: dict[str, Any],
organization: RpcOrganization | None,
logger_params: dict[str, Any],
) -> dict[LinkType, list[UnfurlableUrl]]:
matches: dict[LinkType, list[UnfurlableUrl]] = defaultdict(list)
links_seen = set()

data = slack_request.data.get("event", {})

logger_params = {
"integration_id": slack_request.integration.id,
"team_id": slack_request.team_id,
"channel_id": slack_request.channel_id,
"user_id": slack_request.user_id,
"channel": slack_request.channel_id,
**data,
}

# An unfurl may have multiple links to unfurl
for item in data.get("links", []):
try:
url = item["url"]
except Exception:
_logger.exception("parse-link-error", extra={**logger_params, "url": url})
continue

link_type, args = match_link(url)

# Link can't be unfurled
if link_type is None or args is None:
continue

ois = integration_service.get_organization_integrations(
integration_id=slack_request.integration.id, limit=1
)
organization_id = ois[0].organization_id if len(ois) > 0 else None
organization_context = (
organization_service.get_organization_by_id(
id=organization_id, user_id=None, include_projects=False, include_teams=False
)
if organization_id
else None
)
organization = organization_context.organization if organization_context else None
logger_params["organization_id"] = organization_id

if (
organization
and link_type == LinkType.DISCOVER
and not slack_request.has_identity
and features.has("organizations:discover-basic", organization, actor=request.user)
):
with MessagingInteractionEvent(
interaction_type=MessagingInteractionType.PROCESS_SHARED_LINK,
spec=SlackMessagingSpec(),
).capture() as lifecycle:
try:
analytics.record(
SlackIntegrationChartUnfurl(
organization_id=organization.id,
unfurls_count=0,
)
url = item["url"]
except Exception:
lifecycle.record_failure("Failed to parse link", extra={**logger_params})
Copy link
Member

Choose a reason for hiding this comment

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

This try/except is sort of weird. Realistically, this should only ever be a key error, and we can probably handle it a bit more gracefully with a None check 🤔

continue

link_type, args = match_link(url)

# Link can't be unfurled
if link_type is None or args is None:
lifecycle.record_halt("Unfurlable link", extra={"url": url})
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Cannot unfurl: misleading halt message under None conditions

The halt message "Unfurlable link" is misleading. This message is recorded when link_type is None or args is None, which means the link CANNOT be unfurled. The message should say something like "Cannot unfurl link" or "Non-unfurlable link" to accurately reflect the condition being checked.

Fix in Cursor Fix in Web

Copy link
Member Author

Choose a reason for hiding this comment

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

bruh

continue

if (
organization
and link_type == LinkType.DISCOVER
and not slack_request.has_identity
and features.has(
"organizations:discover-basic", organization, actor=request.user
)
except Exception as e:
sentry_sdk.capture_exception(e)
):
try:
analytics.record(
SlackIntegrationChartUnfurl(
organization_id=organization.id,
unfurls_count=0,
)
)
except Exception as e:
sentry_sdk.capture_exception(e)

self.prompt_link(slack_request)
return True
self.prompt_link(slack_request)
lifecycle.record_halt("Discover link requires identity", extra={"url": url})
return {}

# Don't unfurl the same thing multiple times
seen_marker = hash(orjson.dumps((link_type, list(args))).decode())
if seen_marker in links_seen:
continue
# Don't unfurl the same thing multiple times
seen_marker = hash(orjson.dumps((link_type, list(args))).decode())
if seen_marker in links_seen:
continue

links_seen.add(seen_marker)
matches[link_type].append(UnfurlableUrl(url=url, args=args))
links_seen.add(seen_marker)
matches[link_type].append(UnfurlableUrl(url=url, args=args))
Copy link
Contributor

Choose a reason for hiding this comment

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

Bug: Missing return: unfurlable links None instead of dict

The method _get_unfurlable_links is missing a return statement at the end. When the loop completes normally, the function implicitly returns None instead of returning the matches dictionary. This will cause the caller at line 277 to incorrectly treat successful link parsing as a failure since if not matches will be True when matches is None. The method should end with return matches.

Fix in Cursor Fix in Web


if not matches:
return False
return matches

# Unfurl each link type
results: MutableMapping[str, Any] = {}
def _unfurl_links(
self, slack_request: SlackDMRequest, matches: dict[LinkType, list[UnfurlableUrl]]
) -> dict[str, Any]:
results: dict[str, Any] = {}
for link_type, unfurl_data in matches.items():
results.update(
link_handlers[link_type].fn(
request,
slack_request.integration,
unfurl_data,
slack_request.user,
slack_request.integration, unfurl_data, slack_request.user
)
)

if not results:
return False

# XXX(isabella): we use our message builders to create the blocks for each link to be
# unfurled, so the original result will include the fallback text string, however, the
# unfurl endpoint does not accept fallback text.
for link_info in results.values():
if "text" in link_info:
del link_info["text"]

payload = {"channel": data["channel"], "ts": data["message_ts"], "unfurls": results}
return results

def on_link_shared(self, request: Request, slack_request: SlackDMRequest) -> bool:
"""Returns true on success"""

data = slack_request.data.get("event", {})

ois = integration_service.get_organization_integrations(
integration_id=slack_request.integration.id, limit=1
)
organization_id = ois[0].organization_id if len(ois) > 0 else None
organization_context = (
organization_service.get_organization_by_id(
id=organization_id,
user_id=None,
include_projects=False,
include_teams=False,
)
if organization_id
else None
)
organization = organization_context.organization if organization_context else None

logger_params = {
"integration_id": slack_request.integration.id,
"team_id": slack_request.team_id,
"channel_id": slack_request.channel_id,
"user_id": slack_request.user_id,
"channel": slack_request.channel_id,
"organization_id": organization_id,
**data,
}

# An unfurl may have multiple links to unfurl
matches = self._get_unfurlable_links(
request, slack_request, data, organization, logger_params
)
if not matches:
return False

# Unfurl each link type
results = self._unfurl_links(slack_request, matches)
if not results:
return False

with MessagingInteractionEvent(
interaction_type=MessagingInteractionType.UNFURL_LINK,
spec=SlackMessagingSpec(),
).capture() as lifecycle:
payload = {"channel": data["channel"], "ts": data["message_ts"], "unfurls": results}
client = SlackSdkClient(integration_id=slack_request.integration.id)
try:
client.chat_unfurl(
Expand Down
Loading
Loading