googleapis/python-bigquery

Should a rateLimitExceeded have a "429 Too Many Requests" instead of "403 Forbidden"?

cozos opened this issue · 1 comments

In many cases, users want to be able to retry rateLimitExceeded errors. Many retry libraries and codebases support this by automatically retrying HTTP exceptions with a 429 Too Many Requests HTTP error, and any 5XX error codes. For example, BigQuery itself. However BigQuery will raise a 403 Forbidden for rate limit problems - see:

"rateLimitExceeded": http.client.FORBIDDEN,

A 403 Forbidden implies that the API user does not have the permissions or authentication credentials to do the thing. That seems inappropriate for a rateLimitError, right?

This mis-coding has two consequences for retry logic:

  • In codebases where you'd expect to get retries for free, you don't, because libraries won't retry a 403 Forbidden, and rightly so - why would you retry an access error.
  • Users who do want to implement retries for BQ rateLimitExceeded errors now have to retry on 403 Forbidden, possibly causing them to retry genuine permissions/auth errors that will fail no matter how many attempts.

A few quick notes for future me (or anyone else picking this up).

We are looking at three different sources of errors here.

  • Errors native to Python in the http Standard Library (in base.py)
  • Errors native to the requests module (in retry.py)
  • Errors native to the GoogleAPIs python-api-core module (in retry.py)

Each of these libraries covers different classes of errors.

It appears that the error we are most interested in raising is exceptions.TooManyRequests, from the google-api-core.exceptions module. We reference it in the [BigQuery retry.py file] after importing exceptions from python-api-core: (

_UNSTRUCTURED_RETRYABLE_TYPES = (
ConnectionError,
exceptions.TooManyRequests,
exceptions.InternalServerError,
exceptions.BadGateway,
exceptions.ServiceUnavailable,
requests.exceptions.ChunkedEncodingError,
requests.exceptions.ConnectionError,
requests.exceptions.Timeout,
auth_exceptions.TransportError,
)
).