Repository: bimmerconnected/bimmer_connected
Pull request #543 (2023-06-17 10:17:31)

Handle quota exceeded in HTTP 403
## Proposed change
<!--
  Describe the big picture of your changes here to communicate to the
  maintainers why we should accept this pull request. If it fixes a bug
  or resolves a feature request, be sure to link to that issue in the
  additional information section.
-->
This PR updates our API client to handle not directly resolvable quota issues with a new exception (`MyBMWQuotaError`).
Additional logic was needed, as sometimes this error not only occurs with HTTP status code 429, but also 403 Forbidden.

After this is merged, we can update the HA component to not require re-entering the password on quota issues (see 

## Type of change
- [ ] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (which adds functionality to this library)
- [ ] Breaking change (fix/feature causing existing functionality to break)
- [ ] Code quality improvements to existing code or addition of tests

## Additional information
<!--
  Details are important, and help maintainers processing your PR.
  Please be sure to fill out additional details, if applicable.
-->

- This PR fixes or closes issue: fixes #
- This PR is related to issue: https://github.com/home-assistant/core/issues/85962

## Checklist
<!--
  Put an `x` in the boxes that apply. You can also fill these out after
  creating the PR. If you're unsure about any of them, don't hesitate to ask.
  We're here to help! This is simply a reminder of what we are going to look
  for before merging your code.
-->

- [x] The code change is tested and works locally.
- [x] Tests have been added to verify that the new code works.


codecov[bot]: ## [Codecov](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/543?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) Report
Patch coverage: **`100.00`**% and no project coverage change.
> Comparison is base [(`b60fb7f`)](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/commit/b60fb7fe45174eeebf4a2f7344c7e119fa32b724?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) 100.00% compared to head [(`52a7c0a`)](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/543?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) 100.00%.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##            master      #543   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           18        18           
  Lines         1479      1483    +4     
=========================================
+ Hits          1479      1483    +4     
```

| Flag | Coverage Δ | |
|---|---|---|
| 3.10 | `100.00% <100.00%> (ø)` | |
| 3.11 | `?` | |
| 3.7 | `100.00% <100.00%> (ø)` | |
| 3.8 | `?` | |
| 3.9 | `?` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected) | Coverage Δ | |
|---|---|---|
| [bimmer\_connected/api/authentication.py](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected#diff-YmltbWVyX2Nvbm5lY3RlZC9hcGkvYXV0aGVudGljYXRpb24ucHk=) | `100.00% <100.00%> (ø)` | |
| [bimmer\_connected/api/utils.py](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected#diff-YmltbWVyX2Nvbm5lY3RlZC9hcGkvdXRpbHMucHk=) | `100.00% <100.00%> (ø)` | |
| [bimmer\_connected/models.py](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/543?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected#diff-YmltbWVyX2Nvbm5lY3RlZC9tb2RlbHMucHk=) | `100.00% <100.00%> (ø)` | |


</details>

[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/bimmerconnected/bimmer_connected/pull/543?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=bimmerconnected).


Patch:
diff --git a/bimmer_connected/api/authentication.py b/bimmer_connected/api/authentication.py
index 4c01dba..06b07d7 100644
--- a/bimmer_connected/api/authentication.py
+++ b/bimmer_connected/api/authentication.py
@@ -83,7 +83,9 @@ class MyBMWAuthentication(httpx.Auth):
 
         # Try getting a response
         response: httpx.Response = (yield request)
+        await response.aread()
 
+        # Handle "classic" 401 Unauthorized
         if response.status_code == 401:
             async with self.login_lock:
                 _LOGGER.debug("Received unauthorized response, refreshing token.")
@@ -91,7 +93,8 @@ class MyBMWAuthentication(httpx.Auth):
             request.headers["authorization"] = f"Bearer {self.access_token}"
             request.headers["bmw-session-id"] = self.session_id
             yield request
-        elif response.status_code == 429:
+        # Quota errors can either be 429 Too Many Requests or 403 Quota Exceeded (instead of 403 Forbidden)
+        elif response.status_code == 429 or (response.status_code == 403 and "quota exceeded" in response.text.lower()):
             for _ in range(3):
                 if response.status_code == 429:
                     await response.aread()
diff --git a/bimmer_connected/api/utils.py b/bimmer_connected/api/utils.py
index 93de5f6..050a660 100644
--- a/bimmer_connected/api/utils.py
+++ b/bimmer_connected/api/utils.py
@@ -17,7 +17,7 @@ from Crypto.Cipher import AES
 from Crypto.Hash import SHA256
 from Crypto.Util.Padding import pad
 
-from bimmer_connected.models import AnonymizedResponse, MyBMWAPIError, MyBMWAuthError
+from bimmer_connected.models import AnonymizedResponse, MyBMWAPIError, MyBMWAuthError, MyBMWQuotaError
 
 UNICODE_CHARACTER_SET = string.ascii_letters + string.digits + "-._~"
 RE_VIN = re.compile(r"(?P<vin>WB[a-zA-Z0-9]{15})")
@@ -56,6 +56,8 @@ async def handle_httpstatuserror(
     _logger = log_handler or logging.getLogger(__name__)
     _level = logging.DEBUG if dont_raise else logging.ERROR
 
+    await ex.response.aread()
+
     # By default we will raise a MyBMWAPIError
     _ex_to_raise = MyBMWAPIError
 
@@ -63,7 +65,11 @@ async def handle_httpstatuserror(
     if ex.response.status_code in [401, 403]:
         _ex_to_raise = MyBMWAuthError
 
-    await ex.response.aread()
+    # 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 exceeded" in ex.response.text.lower()
+    ):
+        _ex_to_raise = MyBMWQuotaError
 
     try:
         # Try parsing the known BMW API error JSON
@@ -73,7 +79,7 @@ async def handle_httpstatuserror(
         # If format has changed or is not JSON
         _err_message = f"{type(ex).__name__}: {ex.response.text or str(ex)}"
 
-    _logger.log(_level, "MyBMW %s error: %s", module, _err_message)
+    _logger.log(_level, "%s due to %s", _ex_to_raise.__name__, _err_message)
 
     if not dont_raise:
         raise _ex_to_raise(_err_message) from ex
diff --git a/bimmer_connected/models.py b/bimmer_connected/models.py
index 7162843..822a3b9 100644
--- a/bimmer_connected/models.py
+++ b/bimmer_connected/models.py
@@ -153,5 +153,9 @@ class MyBMWAuthError(MyBMWAPIError):
     """Auth-related error from BMW API (HTTP status codes 401 and 403)."""
 
 
+class MyBMWQuotaError(MyBMWAPIError):
+    """Quota exceeded on BMW API."""
+
+
 class MyBMWRemoteServiceError(MyBMWAPIError):
     """Error when executing remote services."""
diff --git a/bimmer_connected/utils.py b/bimmer_connected/utils.py
index 273e0b7..365b42f 100644
--- a/bimmer_connected/utils.py
+++ b/bimmer_connected/utils.py
@@ -9,7 +9,7 @@ import pathlib
 import time
 import traceback
 from enum import Enum
-from typing import TYPE_CHECKING, Dict, List, Optional
+from typing import TYPE_CHECKING, Dict, List, Optional, Union
 
 from bimmer_connected.models import AnonymizedResponse
 
@@ -62,7 +62,7 @@ def parse_datetime(date_str: str) -> Optional[datetime.datetime]:
 class MyBMWJSONEncoder(json.JSONEncoder):
     """JSON Encoder that handles data classes, properties and additional data types."""
 
-    def default(self, o):  # noqa: D102
+    def default(self, o) -> Union[str, dict]:  # noqa: D102
         if isinstance(o, (datetime.datetime, datetime.date, datetime.time)):
             return o.isoformat()
         if not isinstance(o, Enum) and hasattr(o, "__dict__") and isinstance(o.__dict__, Dict):


Test patch:
diff --git a/test/test_account.py b/test/test_account.py
index 0383bcf..5a5e33e 100644
--- a/test/test_account.py
+++ b/test/test_account.py
@@ -23,7 +23,7 @@ from bimmer_connected.api.authentication import MyBMWAuthentication, MyBMWLoginR
 from bimmer_connected.api.client import MyBMWClient
 from bimmer_connected.api.regions import get_region_from_name
 from bimmer_connected.const import ATTR_CAPABILITIES, Regions
-from bimmer_connected.models import GPSPosition, MyBMWAPIError, MyBMWAuthError
+from bimmer_connected.models import GPSPosition, MyBMWAPIError, MyBMWAuthError, MyBMWQuotaError
 
 from . import (
     ALL_CHARGING_SETTINGS,
@@ -584,7 +584,7 @@ async def test_429_retry_raise_vehicles(caplog):
         caplog.set_level(logging.DEBUG)
 
         with mock.patch("asyncio.sleep", new_callable=mock.AsyncMock):
-            with pytest.raises(MyBMWAPIError):
+            with pytest.raises(MyBMWQuotaError):
                 await account.get_vehicles()
 
         log_429 = [
@@ -595,6 +595,25 @@ async def test_429_retry_raise_vehicles(caplog):
         assert len(log_429) == 3
 
 
+@pytest.mark.asyncio
+async def test_403_quota_exceeded_vehicles_usa(caplog):
+    """Test 403 quota issues for vehicle state and fail if it happens too often."""
+    with account_mock() as mock_api:
+        account = MyBMWAccount(TEST_USERNAME, TEST_PASSWORD, TEST_REGION)
+        # get vehicles once
+        await account.get_vehicles()
+
+        mock_api.get("/eadrax-vcs/v4/vehicles/state").mock(return_value=httpx.Response(403, text="403 Quota Exceeded"))
+        caplog.set_level(logging.DEBUG)
+
+        with mock.patch("asyncio.sleep", new_callable=mock.AsyncMock):
+            with pytest.raises(MyBMWQuotaError):
+                await account.get_vehicles()
+
+        log_quota = [r for r in caplog.records if "Quota Exceeded" in r.message]
+        assert len(log_quota) == 1
+
+
 @account_mock()
 @pytest.mark.asyncio
 async def test_client_async_only():