Skip to content

fix(rpc): fix seer RPC exception handling to default to 500 and preserve rest APIException #112435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
aliu39 merged 2 commits into from
Apr 11, 2026
Merged

fix(rpc): fix seer RPC exception handling to default to 500 and preserve rest APIException #112435

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
388d7ab
Fix seer RPC exception handling to preserve HTTP status codes
cursoragent Apr 7, 2026
63b85e0
mock in_test_environ
aliu39 Apr 11, 2026
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
4 changes: 3 additions & 1 deletion src/sentry/seer/endpoints/seer_rpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,13 +269,15 @@ def post(self, request: Request, method_name: str) -> Response:
except SnubaRPCRateLimitExceeded as e:
sentry_sdk.capture_exception()
raise Throttled(detail="Rate limit exceeded") from e
except APIException:
raise
except Exception as e:
if in_test_environment():
raise
if settings.DEBUG:
raise Exception(f"Problem processing seer rpc endpoint {method_name}") from e
sentry_sdk.capture_exception()
raise ValidationError from e
raise APIException from e
return Response(data=result)


Expand Down
44 changes: 44 additions & 0 deletions tests/sentry/seer/endpoints/test_seer_rpc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,50 @@ def test_snuba_rate_limit_returns_429(self) -> None:
assert response.status_code == 429
assert "Rate limit exceeded" in response.data["detail"]

def test_rest_framework_exceptions_are_reraised(self) -> None:
"""Test that REST framework exceptions preserve their status codes."""
from rest_framework.exceptions import APIException

class CustomAPIException(APIException):
status_code = 503
default_detail = "Service temporarily unavailable"

path = self._get_path("get_organization_slug")
data: dict[str, Any] = {"args": {"org_id": 1}, "meta": {}}

with patch(
"sentry.seer.endpoints.seer_rpc.SeerRpcServiceEndpoint._dispatch_to_local_method"
) as mock_dispatch:
mock_dispatch.side_effect = CustomAPIException()

response = self.client.post(
path, data=data, HTTP_AUTHORIZATION=self.auth_header(path, data)
)

assert response.status_code == 503
assert "Service temporarily unavailable" in response.data["detail"]

def test_generic_exceptions_return_500(self) -> None:
"""Test that generic exceptions return 500 instead of 400."""
path = self._get_path("get_organization_slug")
data: dict[str, Any] = {"args": {"org_id": 1}, "meta": {}}

for is_test_environment in [True, False]:
with patch(
"sentry.seer.endpoints.seer_rpc.in_test_environment",
return_value=is_test_environment,
):
with patch(
"sentry.seer.endpoints.seer_rpc.SeerRpcServiceEndpoint._dispatch_to_local_method"
) as mock_dispatch:
mock_dispatch.side_effect = RuntimeError("Unexpected internal error")

response = self.client.post(
path, data=data, HTTP_AUTHORIZATION=self.auth_header(path, data)
)

assert response.status_code == 500
Comment on lines +119 to +129
Copy link
Copy Markdown
Contributor

@sentry sentry bot Apr 11, 2026

Choose a reason for hiding this comment

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

Bug: The test test_generic_exceptions_return_500 crashes when in_test_environment() is true because a raw RuntimeError is re-raised, preventing the response assertion from being executed.
Severity: LOW

Suggested Fix

The test should be modified to handle the expected RuntimeError when in_test_environment() returns True. This can be done by wrapping the self.client.post(...) call in a pytest.raises(RuntimeError) context manager for the test environment case. This will correctly assert that the exception is raised as intended, allowing the test to pass and properly validate the code path.

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: tests/sentry/seer/endpoints/test_seer_rpc.py#L115-L129

Potential issue: The test `test_generic_exceptions_return_500` is designed to check for
a 500 error response. However, it loops through `is_test_environment` being `True` and
`False`. When `is_test_environment` is mocked to `True`, the code under test re-raises a
raw `RuntimeError`. This exception is not handled by the DRF exception handler.
Consequently, the Django test client re-raises the exception, causing the test to crash
at the `self.client.post(...)` call. The assertion `assert response.status_code == 500`
is never reached because the `response` variable is never assigned. This means the test
fails to validate the intended behavior for the test environment scenario.

Did we get this right? 👍 / 👎 to inform future reviews.



class TestSeerRpcMethods(APITestCase):
"""Test individual RPC methods"""
Expand Down
Loading