Skip to content

Commit 32333c6

Browse files
aliu39cursoragent
andauthored
fix(rpc): fix seer RPC exception handling to default to 500 and preserve rest APIException (#112435)
<!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Problem When Seer makes RPC calls to Sentry and encounters downstream errors (like 429 rate limits or 500 errors from Snuba), the current code catches all exceptions and re-raises them as `ValidationError` (400). This misrepresents the actual issue, making it appear as if the client sent bad request parameters when the real cause is a downstream service error. Example issue: https://sentry.dev.getsentry.net:7999/issues/7380803020/events/d3e6e1d5ddf44d7b94c86e6ce2435ea6/ ## Solution This PR fixes the exception handling in the seer RPC endpoint to raise a generic 500 APIException by default, implying an error happened on sentry server. Also re-raises APIException's (base class for rest framework excs) so developers can bubble their own status codes up to response for Seer. 1. **Added a handler to re-raise REST framework exceptions directly** - Any `APIException` (which includes `Throttled`, custom API exceptions, etc.) is now re-raised as-is, preserving its status code 2. **Changed generic exception handler to raise `APIException` (500) instead of `ValidationError` (400)** - Unknown errors now correctly return 500 Internal Server Error instead of 400 Bad Request ### Changes - Modified `SeerRpcServiceEndpoint.post()` in `src/sentry/seer/endpoints/seer_rpc.py`: - Added `except APIException: raise` handler before the generic exception handler - Changed `raise ValidationError from e` to `raise APIException from e` for generic exceptions - Added test cases in `tests/sentry/seer/endpoints/test_seer_rpc.py`: - `test_rest_framework_exceptions_are_reraised` - verifies custom API exceptions preserve their status codes - `test_generic_exceptions_return_500` - verifies generic exceptions return 500 instead of 400 ## Testing Pre-commit checks pass ✅ <!-- CURSOR_AGENT_PR_BODY_END --> [Slack Thread](https://sentry.slack.com/archives/C0AAF0JD0GK/p1775600719171269?thread_ts=1775600719.171269&cid=C0AAF0JD0GK) <div><a href="https://cursor.com/agents/bc-8bf9c0b1-d369-5efa-81a7-a650bb7e83cc"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/background-agent?bcId=bc-8bf9c0b1-d369-5efa-81a7-a650bb7e83cc"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div> --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com> Co-authored-by: Andrew Liu <aliu39@users.noreply.github.com>
1 parent a6cee27 commit 32333c6

File tree

2 files changed

+47
-1
lines changed

2 files changed

+47
-1
lines changed

src/sentry/seer/endpoints/seer_rpc.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -271,13 +271,15 @@ def post(self, request: Request, method_name: str) -> Response:
271271
except SnubaRPCRateLimitExceeded as e:
272272
sentry_sdk.capture_exception()
273273
raise Throttled(detail="Rate limit exceeded") from e
274+
except APIException:
275+
raise
274276
except Exception as e:
275277
if in_test_environment():
276278
raise
277279
if settings.DEBUG:
278280
raise Exception(f"Problem processing seer rpc endpoint {method_name}") from e
279281
sentry_sdk.capture_exception()
280-
raise ValidationError from e
282+
raise APIException from e
281283
return Response(data=result)
282284

283285

tests/sentry/seer/endpoints/test_seer_rpc.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,50 @@ def test_snuba_rate_limit_returns_429(self) -> None:
8787
assert response.status_code == 429
8888
assert "Rate limit exceeded" in response.data["detail"]
8989

90+
def test_rest_framework_exceptions_are_reraised(self) -> None:
91+
"""Test that REST framework exceptions preserve their status codes."""
92+
from rest_framework.exceptions import APIException
93+
94+
class CustomAPIException(APIException):
95+
status_code = 503
96+
default_detail = "Service temporarily unavailable"
97+
98+
path = self._get_path("get_organization_slug")
99+
data: dict[str, Any] = {"args": {"org_id": 1}, "meta": {}}
100+
101+
with patch(
102+
"sentry.seer.endpoints.seer_rpc.SeerRpcServiceEndpoint._dispatch_to_local_method"
103+
) as mock_dispatch:
104+
mock_dispatch.side_effect = CustomAPIException()
105+
106+
response = self.client.post(
107+
path, data=data, HTTP_AUTHORIZATION=self.auth_header(path, data)
108+
)
109+
110+
assert response.status_code == 503
111+
assert "Service temporarily unavailable" in response.data["detail"]
112+
113+
def test_generic_exceptions_return_500(self) -> None:
114+
"""Test that generic exceptions return 500 instead of 400."""
115+
path = self._get_path("get_organization_slug")
116+
data: dict[str, Any] = {"args": {"org_id": 1}, "meta": {}}
117+
118+
for is_test_environment in [True, False]:
119+
with patch(
120+
"sentry.seer.endpoints.seer_rpc.in_test_environment",
121+
return_value=is_test_environment,
122+
):
123+
with patch(
124+
"sentry.seer.endpoints.seer_rpc.SeerRpcServiceEndpoint._dispatch_to_local_method"
125+
) as mock_dispatch:
126+
mock_dispatch.side_effect = RuntimeError("Unexpected internal error")
127+
128+
response = self.client.post(
129+
path, data=data, HTTP_AUTHORIZATION=self.auth_header(path, data)
130+
)
131+
132+
assert response.status_code == 500
133+
90134

91135
class TestSeerRpcMethods(APITestCase):
92136
"""Test individual RPC methods"""

0 commit comments

Comments
 (0)