fix(snuba_rpc): Fix error handling for non-protofbuf responses for Snuba RPC#107995
fix(snuba_rpc): Fix error handling for non-protofbuf responses for Snuba RPC#107995
Conversation
github-actions
bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| except (UnicodeDecodeError, AttributeError): | ||
| body = "<non-text response body>" | ||
| raise SnubaRPCError(f"Snuba RPC returned HTTP {http_resp.status}: {body}") | ||
| return error |
There was a problem hiding this comment.
Non-protobuf 404/429 responses bypass status-specific error handling
Medium Severity
_parse_error raises SnubaRPCError directly when protobuf decoding fails, which bypasses the caller's status-specific handling for 404 (NotFound) and 429 (SnubaRPCRateLimitExceeded). If an infrastructure component (e.g., load balancer) returns a non-protobuf 429 response, callers that catch SnubaRPCRateLimitExceeded (like in api/utils.py to return Throttled) will instead see a generic SnubaRPCError, resulting in a misleading "Internal error" message rather than a proper rate-limit response.
Additional Locations (1)
There was a problem hiding this comment.
This is not a regression since if we failed parsing we bypassed this status-specific handling anyway. Basically if the response is not a protobuf one then we give on being status specific and return a best effort
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
…uba RPC When the Snuba RPC fails on infra errors and not responses from Snuba (e.g 503 errors) it fails on decode expecting a protobuf response. Added some protection so these are handled correctly, replacing decode errors with what actually happened.
yuvmen
force-pushed
the
yuvmen/snuba-rpc-error-parsing
branch
from
be9a42b to
e24b846
Compare
…uba RPC (#107995) When the Snuba RPC fails on infra errors and not responses from Snuba (e.g 503 errors) it fails on decode expecting a protobuf response. Added some protection so these are handled correctly, replacing decode errors with what actually happened by attempting to parse the data as utf-8.
2 participants
When the Snuba RPC fails on infra errors and not responses from Snuba (e.g 503 errors) it fails on decode expecting a protobuf response. Added some protection so these are handled correctly, replacing decode errors with what actually happened by attempting to parse the data as utf-8.
noticed it because of this issue