Skip to content

feat(api): return TOO_MANY_BYTES as clickhouse error instead of 429#7782

Open
cvxluo wants to merge 3 commits intomasterfrom
cvxluo/too-many-bytes-error-changes
Open

feat(api): return TOO_MANY_BYTES as clickhouse error instead of 429#7782
cvxluo wants to merge 3 commits intomasterfrom
cvxluo/too-many-bytes-error-changes

Conversation

@cvxluo
Copy link
Contributor

@cvxluo cvxluo commented Mar 2, 2026

Stop converting ClickHouse TOO_MANY_BYTES (code 307) errors into RateLimitExceeded. This lets consumers (e.g. sentry) distinguish "query scanned too many bytes" from snuba-level rate limiting.

@cvxluo cvxluo force-pushed the cvxluo/too-many-bytes-error-changes branch from e0c6985 to 6a3004f Compare March 2, 2026 23:31
@cvxluo cvxluo changed the title test(api): change test to check for clickhouse error feat(api): return TOO_MANY_BYTES as clickhouse error instead of 429 Mar 2, 2026
…limit

Stop converting ClickHouse TOO_MANY_BYTES (code 307) errors into
RateLimitExceeded. This lets consumers (e.g. sentry) distinguish
"query scanned too many bytes" from snuba-level rate limiting.

- Remove RateLimitExceeded conversion for TOO_MANY_BYTES in db_query.py
- Add TOO_MANY_BYTES to ACCEPTABLE_CLICKHOUSE_ERROR_CODES (returns 400)
- Update test to assert clickhouse error type with code 307

Co-Authored-By: Claude <noreply@anthropic.com>
@cvxluo cvxluo force-pushed the cvxluo/too-many-bytes-error-changes branch from 6a3004f to c66b632 Compare March 2, 2026 23:39
@cvxluo cvxluo marked this pull request as ready for review March 2, 2026 23:52
@cvxluo cvxluo requested a review from a team as a code owner March 2, 2026 23:52
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

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.

"Query scanned more than the allocated amount of bytes",
quota_allowance=stats["quota_allowance"],
)

Copy link

Choose a reason for hiding this comment

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

calculated_cause variable is now redundant dead code

Low Severity

The calculated_cause variable is assigned to cause on line 472 and is never reassigned in any branch. It was previously needed because the TOO_MANY_BYTES branch would reassign it to a RateLimitExceeded instance. With that branch removed, calculated_cause is always identical to cause, making it dead code. The from calculated_cause on line 509 can simply use from cause directly.

Fix in Cursor Fix in Web

# queries that can never return the amount of data requested by the user are not an internal error
ErrorCodes.MEMORY_LIMIT_EXCEEDED,
# query exceeded the max_bytes_to_read limit set by an allocation policy
ErrorCodes.TOO_MANY_BYTES,
Copy link

Choose a reason for hiding this comment

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

TOO_MANY_BYTES missing from non-retryable error codes set

Medium Severity

TOO_MANY_BYTES is added to ACCEPTABLE_CLICKHOUSE_ERROR_CODES but not to NON_RETRYABLE_CLICKHOUSE_ERROR_CODES. Previously this error was converted to RateLimitExceeded, so it never hit the non-retryable check. Now it flows through as a ClickhouseError, so in the subscription executor consumer it triggers a SubscriptionQueryException (retry), even though the error is deterministic and retrying won't help since the max_bytes_to_read limit is fixed by allocation policy.

Fix in Cursor Fix in Web

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant