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
27 changes: 5 additions & 22 deletions src/sentry/seer/code_review/webhooks/task.py
Original file line number Diff line number Diff line change
Expand Up @@ -110,23 +110,20 @@ def schedule_task(
def process_github_webhook_event(
*,
enqueued_at_str: str,
# Later to be removed when we have migrated all callers to use seer_path
seer_path: str | None = None,
github_event: str | None = None,
seer_path: str,
event_payload: Mapping[str, Any],
tags: Mapping[str, Any] | None = None,
**kwargs: Any,
) -> None:
"""
Process GitHub webhook event by forwarding to Seer if applicable.
Forward a validated code-review payload to Seer.

Args:
enqueued_at_str: The timestamp when the task was enqueued
github_event: The GitHub webhook event type from X-GitHub-Event header (e.g., "check_run", "pull_request")
seer_path: The path to the Seer API endpoint to call
event_payload: The payload of the webhook event (already validated before scheduling)
event_payload: The payload (already validated before scheduling)
tags: Sentry SDK tags to set on this task's scope for error correlation
**kwargs: Parameters to pass to webhook handler functions
**kwargs: Absorbs legacy serialized task arguments from in-flight work
"""
status = "success"
should_record_latency = True
Expand All @@ -137,21 +134,7 @@ def process_github_webhook_event(
if tags and (org_id := tags.get("sentry_organization_id")):
viewer_context = SeerViewerContext(organization_id=int(org_id))

# Temporary check for backwards compatibility (pre-seer_path tasks).
# event_payload is the Seer-shaped body {external_owner_id, data}; it never
# includes GitHub's "action". Old tasks used request_type "pr-closed" instead.
if seer_path is None and github_event is not None:
assert isinstance(github_event, str)
action = event_payload.get("action")
if action is None and event_payload.get("request_type") == "pr-closed":
action = "closed"
if action is None and tags:
action = tags.get("github_event_action")
path = get_seer_path_for_request(github_event, action)
else:
assert isinstance(seer_path, str)
path = seer_path
make_seer_request(path=path, payload=event_payload, viewer_context=viewer_context)
make_seer_request(path=seer_path, payload=event_payload, viewer_context=viewer_context)
except Exception as e:
status = e.__class__.__name__
# Retryable errors are automatically retried by taskworker.
Expand Down
99 changes: 15 additions & 84 deletions tests/sentry/seer/code_review/test_webhooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ def test_server_error_response_raises_for_retry(

with pytest.raises(HTTPError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand All @@ -113,7 +113,7 @@ def test_service_unavailable_response_raises_for_retry(

with pytest.raises(HTTPError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand All @@ -140,7 +140,7 @@ def test_rate_limit_response_raises_for_retry(

with pytest.raises(HTTPError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand All @@ -165,7 +165,7 @@ def test_client_error_response_not_retried(

with pytest.raises(ClientError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand Down Expand Up @@ -195,7 +195,7 @@ def test_network_error_raises_for_retry(

with pytest.raises(MaxRetryError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand Down Expand Up @@ -224,7 +224,7 @@ def test_timeout_error_raises_for_retry(

with pytest.raises(TimeoutError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand Down Expand Up @@ -253,7 +253,7 @@ def test_ssl_error_raises_for_retry(

with pytest.raises(SSLError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand Down Expand Up @@ -282,7 +282,7 @@ def test_new_connection_error_raises_for_retry(

with pytest.raises(NewConnectionError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand Down Expand Up @@ -311,7 +311,7 @@ def test_network_error_not_logged_on_early_retry(

with pytest.raises(TimeoutError):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand Down Expand Up @@ -341,7 +341,7 @@ def test_unexpected_error_logged_and_tracked(
# Unexpected exceptions are not retried
with pytest.raises(ValueError, match="Invalid JSON format"):
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand All @@ -367,7 +367,7 @@ def test_latency_tracking_on_first_attempt(
mock_request.return_value = self._mock_response(200, b"{}")

process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand Down Expand Up @@ -405,7 +405,7 @@ def test_latency_tracking_on_max_retries_with_failures(

try:
process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=enqueued_at_str,
tags={},
Expand All @@ -432,11 +432,11 @@ def test_latency_tracking_on_max_retries_with_failures(

@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_check_run_and_pr_events_processed_separately(self, mock_request: MagicMock) -> None:
"""Test that CHECK_RUN uses rerun endpoint; PR events use review-request endpoint."""
"""Test that check rerun and PR review use their configured Seer paths."""
mock_request.return_value = self._mock_response(200, b"{}")

process_github_webhook_event._func(
github_event=GithubWebhookType.CHECK_RUN,
seer_path="/v1/code_review/check/rerun",
event_payload={"original_run_id": self.original_run_id},
enqueued_at_str=self.enqueued_at_str,
tags={},
Expand Down Expand Up @@ -489,75 +489,6 @@ def test_check_run_and_pr_events_processed_separately(self, mock_request: MagicM
pr_call = mock_request.call_args
assert pr_call[1]["path"] == "/v1/code_review/review-request"

@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_pr_event_uses_review_request_and_pr_closed_endpoints(
self, mock_request: MagicMock
) -> None:
"""Test that PR review uses review-request and pr-closed uses pr-closed endpoint."""
mock_request.return_value = self._mock_response(200, b"{}")

pr_review_payload = {
"request_type": "pr-review",
"external_owner_id": "456",
"data": {
"repo": {
"provider": "github",
"owner": "test-owner",
"name": "test-repo",
"external_id": "123456",
"base_commit_sha": "abc123",
},
"pr_id": 123,
"bug_prediction_specific_information": {
"organization_id": 789,
"organization_slug": "test-org",
},
"config": {
"features": {"bug_prediction": True},
"trigger": "on_new_commit",
"trigger_comment_id": None,
"trigger_comment_type": None,
"trigger_user": None,
},
},
}

process_github_webhook_event._func(
github_event=GithubWebhookType.PULL_REQUEST.value,
event_payload=pr_review_payload,
enqueued_at_str=self.enqueued_at_str,
tags={},
)

assert mock_request.call_count == 1
assert mock_request.call_args[1]["path"] == "/v1/code_review/review-request"

mock_request.reset_mock()
# In-flight tasks before seer_path: payload had request_type, not GitHub action.
pr_closed_payload = {**pr_review_payload, "request_type": "pr-closed"}

process_github_webhook_event._func(
github_event=GithubWebhookType.PULL_REQUEST.value,
event_payload=pr_closed_payload,
enqueued_at_str=self.enqueued_at_str,
tags={},
)

assert mock_request.call_count == 1
assert mock_request.call_args[1]["path"] == "/v1/code_review/pr-closed"

mock_request.reset_mock()
# Tags fallback when payload has neither action nor request_type (edge case).
process_github_webhook_event._func(
github_event=GithubWebhookType.PULL_REQUEST.value,
event_payload={**pr_review_payload},
enqueued_at_str=self.enqueued_at_str,
tags={"github_event_action": "closed"},
)

assert mock_request.call_count == 1
assert mock_request.call_args[1]["path"] == "/v1/code_review/pr-closed"

@patch("sentry.seer.code_review.utils.make_signed_seer_api_request")
def test_validation_converts_enum_keys_to_strings(self, mock_request: MagicMock) -> None:
"""Test that validation converts enum keys to strings for JSON serialization.
Expand Down Expand Up @@ -705,7 +636,7 @@ def test_pr_closed_validation_passes_with_required_fields(
seer_path="/v1/code_review/pr-closed",
event_payload=event_payload,
enqueued_at_str=self.enqueued_at_str,
tags={"github_event_action": "closed"},
tags={},
)

assert mock_request.call_count == 1
Expand Down
Loading