Skip to content

Commit

Permalink
Always raise MyBMWAuthError during login (#650)
Browse files Browse the repository at this point in the history
* Always raise MyBMWAuthError during login

* Fix regression

* Fix mypy
  • Loading branch information
rikroe authored Aug 24, 2024
1 parent 2487964 commit a7e91d4
Show file tree
Hide file tree
Showing 4 changed files with 64 additions and 11 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
repos:
- repo: https://github.com/charliermarsh/ruff-pre-commit
rev: v0.5.6
rev: v0.6.2
hooks:
- id: ruff
args:
Expand Down
6 changes: 3 additions & 3 deletions bimmer_connected/api/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
try:
response.raise_for_status()
except httpx.HTTPStatusError as ex:
await handle_httpstatuserror(ex, log_handler=_LOGGER)
await handle_httpstatuserror(ex, module="API", log_handler=_LOGGER)

async def login(self) -> None:
"""Get a valid OAuth token."""
Expand Down Expand Up @@ -390,7 +390,7 @@ async def raise_for_status_event_handler(response: httpx.Response):
try:
response.raise_for_status()
except httpx.HTTPStatusError as ex:
await handle_httpstatuserror(ex, log_handler=_LOGGER)
await handle_httpstatuserror(ex, module="AUTH", log_handler=_LOGGER)

kwargs["event_hooks"]["response"].append(raise_for_status_event_handler)

Expand Down Expand Up @@ -420,7 +420,7 @@ async def async_auth_flow(self, request: httpx.Request) -> AsyncGenerator[httpx.
try:
response.raise_for_status()
except httpx.HTTPStatusError as ex:
await handle_httpstatuserror(ex, log_handler=_LOGGER)
await handle_httpstatuserror(ex, module="AUTH", log_handler=_LOGGER)


def get_retry_wait_time(response: httpx.Response) -> int:
Expand Down
13 changes: 8 additions & 5 deletions bimmer_connected/api/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,14 +64,17 @@ async def handle_httpstatuserror(
# By default we will raise a MyBMWAPIError
_ex_to_raise = MyBMWAPIError

# HTTP status code is 401 or 403, raise MyBMWAuthError instead
if ex.response.status_code in [401, 403]:
_ex_to_raise = MyBMWAuthError

# Quota errors can either be 429 Too Many Requests or 403 Quota Exceeded (instead of 401 Forbidden)
if ex.response.status_code == 429 or (ex.response.status_code == 403 and "quota" in ex.response.text.lower()):
if (
ex.response.status_code == 429 or (ex.response.status_code == 403 and "quota" in ex.response.text.lower())
) and module != "AUTH":
_ex_to_raise = MyBMWQuotaError

# HTTP status code is 401 or 403, raise MyBMWAuthError instead
# Always raise MyBMWAuthError as final when logging in (e.g. HTTP 429 should still be AuthError)
elif ex.response.status_code in [401, 403] or module == "AUTH":
_ex_to_raise = MyBMWAuthError

try:
# Try parsing the known BMW API error JSON
_err = ex.response.json()
Expand Down
54 changes: 52 additions & 2 deletions bimmer_connected/tests/test_account.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
get_fingerprint_count,
load_response,
)
from .common import MyBMWMockRouter
from .conftest import prepare_account_with_vehicles


Expand Down Expand Up @@ -389,7 +390,7 @@ async def test_refresh_token_getset(bmw_fixture: respx.Router):


@pytest.mark.asyncio
async def test_429_retry_ok_login(caplog, bmw_fixture: respx.Router):
async def test_429_retry_ok_oauth_config(caplog, bmw_fixture: respx.Router):
"""Test the login flow using refresh_token."""
account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION)

Expand All @@ -416,7 +417,7 @@ async def test_429_retry_ok_login(caplog, bmw_fixture: respx.Router):


@pytest.mark.asyncio
async def test_429_retry_raise_login(caplog, bmw_fixture: respx.Router):
async def test_429_retry_raise_oauth_config(caplog, bmw_fixture: respx.Router):
"""Test the login flow using refresh_token."""
account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION)

Expand All @@ -436,6 +437,55 @@ async def test_429_retry_raise_login(caplog, bmw_fixture: respx.Router):
assert len(log_429) == 3


@pytest.mark.asyncio
async def test_429_retry_ok_authenticate(caplog, bmw_fixture: respx.Router):
"""Test the login flow using refresh_token."""
account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION)

json_429 = {"statusCode": 429, "message": "Rate limit is exceeded. Try again in 2 seconds."}

bmw_fixture.post("/gcdm/oauth/authenticate").mock(
side_effect=[
httpx.Response(429, json=json_429),
httpx.Response(429, json=json_429),
MyBMWMockRouter.authenticate_sideeffect, # type: ignore[list-item]
MyBMWMockRouter.authenticate_sideeffect, # type: ignore[list-item]
]
)
caplog.set_level(logging.DEBUG)

with mock.patch("asyncio.sleep", new_callable=mock.AsyncMock):
await account.get_vehicles()

log_429 = [
r
for r in caplog.records
if r.module == "authentication" and "seconds due to 429 Too Many Requests" in r.message
]
assert len(log_429) == 2


@pytest.mark.asyncio
async def test_429_retry_raise_authenticate(caplog, bmw_fixture: respx.Router):
"""Test the login flow using refresh_token."""
account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION)

json_429 = {"statusCode": 429, "message": "Rate limit is exceeded. Try again in 2 seconds."}

bmw_fixture.post("/gcdm/oauth/authenticate").mock(return_value=httpx.Response(429, json=json_429))
caplog.set_level(logging.DEBUG)

with mock.patch("asyncio.sleep", new_callable=mock.AsyncMock), pytest.raises(MyBMWAuthError):
await account.get_vehicles()

log_429 = [
r
for r in caplog.records
if r.module == "authentication" and "seconds due to 429 Too Many Requests" in r.message
]
assert len(log_429) == 3


@pytest.mark.asyncio
async def test_429_retry_ok_vehicles(caplog, bmw_fixture: respx.Router):
"""Test waiting on 429 for vehicles."""
Expand Down

0 comments on commit a7e91d4

Please sign in to comment.