admin-valentin/arcade-mcp
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fixing 403 error always being RateLimiting (#685)

Browse source 
Reponse 403 was returning RateLimiting all the time, but it was due only
checking if rate limiting header exists, but it should be checked if it
is 0 also.

---------

Co-authored-by: Francisco Liberal <francisco@arcade.dev>
This commit is contained in:
jottakka 2025-11-13 22:20:32 -03:00 committed by GitHub
parent bddb383034
commit b4720c2988
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 176 additions and 46 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

60
libs/arcade-tdk/arcade_tdk/providers/http/error_adapter.py
View file

@ -1,4 +1,5 @@
import logging
import re
from datetime import datetime, timezone
from typing import Any
from urllib.parse import urlparse
@ -8,6 +9,8 @@ from arcade_core.errors import (
UpstreamRateLimitError,
)
_NUMERIC_HEADER_PATTERN = re.compile(r"^-?\d+(?:\.\d+)?$")
logger = logging.getLogger(__name__)
RATE_HEADERS = ("retry-after", "x-ratelimit-reset", "x-ratelimit-reset-ms")
@ -16,6 +19,21 @@ RATE_HEADERS = ("retry-after", "x-ratelimit-reset", "x-ratelimit-reset-ms")
class BaseHTTPErrorMapper:
"""Base class for HTTP error mapping functionality."""
def _parse_numeric_header(self, value: str | None) -> float | None:
"""Convert numeric header values to float without relying on exceptions."""
if value is None:
return None
stripped = value.strip()
if not stripped:
return None
if _NUMERIC_HEADER_PATTERN.fullmatch(stripped):
return float(stripped)
return None
def _parse_retry_ms(self, headers: dict[str, str]) -> int:
"""
Parses a rate limit header and returns the number
@ -100,7 +118,8 @@ class BaseHTTPErrorMapper:
"""
Determine if a 403 error is actually a rate limiting error.
Simply checks if any rate limiting headers are present.
Checks if rate limit quota is exhausted. Simply having rate limit headers
is not sufficient since some services include them in all responses.
Args:
headers: HTTP response headers
@ -109,8 +128,43 @@ class BaseHTTPErrorMapper:
Returns:
True if this 403 should be treated as rate limiting
"""
# Check if any rate limiting headers are present
return any(header.lower() in [h.lower() for h in headers] for header in RATE_HEADERS)
# Check if rate limit is actually exhausted (remaining requests is 0)
# Support common header variations used by different APIs
headers_lower = {k.lower(): v for k, v in headers.items()}
remaining_header_names = [
"x-ratelimit-remaining",
"x-rate-limit-remaining",
"ratelimit-remaining",
"x-app-rate-limit-remaining",
]
# Check if remaining quota is 0
for header_name in remaining_header_names:
remaining_value = self._parse_numeric_header(headers_lower.get(header_name))
if remaining_value is not None and remaining_value == 0:
return True
# Check if retry-after is present with a meaningful value along with rate limit headers
# This combination often indicates rate limiting even if remaining isn't explicitly 0
retry_after = headers_lower.get("retry-after")
has_meaningful_retry_after = False
if retry_after:
retry_after_value = self._parse_numeric_header(retry_after)
if retry_after_value is not None:
has_meaningful_retry_after = retry_after_value > 0
else:
# Non-numeric retry-after values (e.g., HTTP dates) indicate rate limiting windows
has_meaningful_retry_after = True
has_rate_limit_headers = any(
header_name in headers_lower
for header_name in [
"x-ratelimit-limit",
"x-rate-limit-limit",
"ratelimit-limit",
]
)
return bool(has_meaningful_retry_after and has_rate_limit_headers)
class _HTTPXExceptionHandler:

2
libs/arcade-tdk/pyproject.toml
View file

@ -1,6 +1,6 @@
[project]
name = "arcade-tdk"
version = "3.0.0"
version = "3.0.1"
description = "Arcade TDK - Toolkit Development Kit for building Arcade tools"
readme = "README.md"
license = {text = "MIT"}

160
libs/tests/sdk/test_httpx_adapter.py
View file

@ -332,76 +332,151 @@ class TestHTTPErrorAdapter:
"""Test that the adapter has the correct slug."""
assert HTTPErrorAdapter.slug == "_http"
def test_map_status_to_error_403_with_rate_limit_headers(self):
"""Test mapping 403 with rate limit headers to rate limit error."""
def test_map_status_to_error_403_with_exhausted_quota(self):
"""Test mapping 403 with exhausted quota to rate limit error."""
headers = {
"retry-after": "30",
"x-ratelimit-remaining": "0",
}
result = self.adapter._map_status_to_error(
status=403,
headers=headers,
msg="Forbidden",
request_url="https://api.github.com/user/repos",
request_url="https://api.example.com/user/repos",
request_method="GET",
)
assert isinstance(result, UpstreamRateLimitError)
assert result.retry_after_ms == 30_000
assert result.message == "Forbidden"
assert result.extra["service"] == "_http"
assert result.extra["endpoint"] == "https://api.github.com/user/repos"
assert result.extra["http_method"] == "GET"
def test_map_status_to_error_403_without_rate_limit_headers(self):
"""Test mapping 403 without rate limiting headers to regular upstream error."""
headers = {}
def test_map_status_to_error_403_with_remaining_quota(self):
"""Test mapping 403 with remaining quota to regular upstream error."""
headers = {
"x-ratelimit-remaining": "4941",
"x-ratelimit-reset": "1762795446",
}
result = self.adapter._map_status_to_error(
status=403,
headers=headers,
msg="Access denied due to insufficient permissions",
request_url="https://api.github.com/user/repos",
msg="Forbidden",
request_url="https://api.example.com/user/repos",
request_method="GET",
)
assert isinstance(result, UpstreamError)
assert not isinstance(result, UpstreamRateLimitError)
assert result.status_code == 403
assert result.message == "Access denied due to insufficient permissions"
assert result.message == "Forbidden"
def test_is_rate_limit_403_with_retry_after_header(self):
"""Test detecting rate limit 403 based on retry-after header."""
headers = {"Retry-After": "60"}
def test_is_rate_limit_403_with_exhausted_quota(self):
"""Test detecting rate limit 403 when quota is exhausted."""
headers = {"x-ratelimit-remaining": "0"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is True
def test_is_rate_limit_403_with_x_ratelimit_reset_header(self):
"""Test detecting rate limit 403 based on x-ratelimit-reset header."""
headers = {"x-ratelimit-reset": "1640995200"}
def test_is_rate_limit_403_with_remaining_quota(self):
"""Test that 403 with remaining quota is NOT detected as rate limiting."""
headers = {"x-ratelimit-remaining": "4941"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is True
assert result is False
def test_is_rate_limit_403_with_x_ratelimit_reset_ms_header(self):
"""Test detecting rate limit 403 based on x-ratelimit-reset-ms header."""
headers = {"X-RateLimit-Reset-Ms": "5000"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is True
def test_is_rate_limit_403_without_headers(self):
"""Test that 403 without rate limiting headers is not detected as rate limiting."""
def test_is_rate_limit_403_without_header(self):
"""Test that 403 without rate limit headers is not detected as rate limiting."""
headers = {}
msg = "Access denied due to insufficient permissions"
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is False
def test_is_rate_limit_403_with_x_rate_limit_remaining_variant(self):
"""Test detecting rate limit 403 with x-rate-limit-remaining header variant."""
headers = {"x-rate-limit-remaining": "0"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is True
def test_is_rate_limit_403_with_ratelimit_remaining_variant(self):
"""Test detecting rate limit 403 with ratelimit-remaining header variant."""
headers = {"ratelimit-remaining": "0"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is True
def test_is_rate_limit_403_with_app_rate_limit_remaining_variant(self):
"""Test detecting rate limit 403 with x-app-rate-limit-remaining header variant."""
headers = {"x-app-rate-limit-remaining": "0"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is True
def test_is_rate_limit_403_with_non_digit_value(self):
"""Test that non-digit remaining value is handled gracefully."""
headers = {"x-ratelimit-remaining": "invalid"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is False
def test_is_rate_limit_403_with_float_value(self):
"""Test that float remaining value is handled (converted to int)."""
headers = {"x-ratelimit-remaining": "0.0"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is True
def test_is_rate_limit_403_with_retry_after_and_rate_limit_headers(self):
"""Test detecting rate limit when retry-after is present with rate limit headers."""
headers = {
"retry-after": "60",
"x-ratelimit-limit": "5000",
"x-ratelimit-remaining": "100", # Still has quota but retry-after suggests rate limit
}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is True
def test_is_rate_limit_403_with_retry_after_only(self):
"""Test that retry-after alone without rate limit headers is not treated as rate limit."""
headers = {"retry-after": "60"}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is False
def test_is_rate_limit_403_with_rate_limit_headers_no_retry_after(self):
"""Test that rate limit headers without retry-after and with remaining quota is not rate limit."""
headers = {
"x-ratelimit-limit": "5000",
"x-ratelimit-remaining": "100",
}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is False
def test_is_rate_limit_403_with_retry_after_zero(self):
"""Test that retry-after with value 0 is not treated as rate limiting."""
headers = {
"retry-after": "0",
"x-ratelimit-limit": "5000",
}
msg = "Forbidden"
result = self.adapter._is_rate_limit_403(headers, msg)
assert result is False
def test_httpx_403_rate_limit_handling(self):
"""Test handling httpx 403 rate limit with rate limiting headers."""
"""Test handling httpx 403 rate limit with exhausted quota."""
# Create a mock HTTPStatusError class
class MockHTTPStatusError(Exception):
@ -409,10 +484,14 @@ class TestHTTPErrorAdapter:
mock_response = Mock()
mock_response.status_code = 403
mock_response.headers = {"x-ratelimit-reset": "1640995200", "retry-after": "120"}
mock_response.headers = {
"x-ratelimit-reset": "1640995200",
"retry-after": "120",
"x-ratelimit-remaining": "0", # Quota is exhausted
}
mock_request = Mock()
mock_request.url = "https://api.github.com/search/repositories"
mock_request.url = "https://api.example.com/search"
mock_request.method = "GET"
mock_exc = MockHTTPStatusError("403 Forbidden")
@ -425,12 +504,9 @@ class TestHTTPErrorAdapter:
assert isinstance(result, UpstreamRateLimitError)
assert result.retry_after_ms == 120_000
assert result.message == "403 Forbidden"
assert result.extra["service"] == "_http"
assert result.extra["endpoint"] == "https://api.github.com/search/repositories"
assert result.extra["http_method"] == "GET"
def test_requests_403_rate_limit_handling(self):
"""Test handling requests 403 rate limit with rate limiting headers."""
"""Test handling requests 403 rate limit with exhausted quota."""
# Create a mock HTTPError class
class MockHTTPError(Exception):
@ -438,10 +514,13 @@ class TestHTTPErrorAdapter:
mock_response = Mock()
mock_response.status_code = 403
mock_response.headers = {"x-ratelimit-reset-ms": "30000"}
mock_response.headers = {
"x-ratelimit-reset-ms": "30000",
"x-ratelimit-remaining": "0", # Quota is exhausted
}
mock_request = Mock()
mock_request.url = "https://api.github.com/user/repos"
mock_request.url = "https://api.example.com/user/repos"
mock_request.method = "POST"
mock_response.request = mock_request
@ -455,6 +534,3 @@ class TestHTTPErrorAdapter:
assert isinstance(result, UpstreamRateLimitError)
assert result.retry_after_ms == 30_000
assert result.message == "403 Forbidden"
assert result.extra["service"] == "_http"
assert result.extra["endpoint"] == "https://api.github.com/user/repos"
assert result.extra["http_method"] == "POST"