From 34bddab2b9aef0d227650425eca2bf8c8652d363 Mon Sep 17 00:00:00 2001 From: Brett Adams Date: Sat, 5 Sep 2026 18:07:48 +1000 Subject: [PATCH 1/2] fix(exceptions): classify 502 as TeslaFleetError, add EnergyGatewayUnreachable A JSON-bodied 502 fell through raise_for_status's status table and hit the trailing resp.raise_for_status(), leaking a raw aiohttp.ClientResponseError instead of a TeslaFleetError; a bodyless 502 already raised ResponseError. Add an explicit 502 branch (BadGateway) so both shapes raise a typed error, and specialize it to EnergyGatewayUnreachable for the gateway-relay endpoints the Powerwall local-control pairing/authorized-clients flow uses, so consumers (see home-assistant/core#181320) no longer need to duplicate this classification themselves. Claude-Session: https://claude.ai/code/session_01Aukcht9BRfsyGsnM1x6mnr --- docs/energy_local_control.md | 7 ++ tesla_fleet_api/exceptions.py | 35 ++++++++ tests/test_bad_gateway_classification.py | 106 +++++++++++++++++++++++ 3 files changed, 148 insertions(+) create mode 100644 tests/test_bad_gateway_classification.py diff --git a/docs/energy_local_control.md b/docs/energy_local_control.md index b172187..f9160d0 100644 --- a/docs/energy_local_control.md +++ b/docs/energy_local_control.md @@ -200,6 +200,13 @@ a genuinely empty client list. Catch `InvalidResponse` (or will not catch it. Either way, a `null` response here tells you nothing about whether the key actually works. +A 502 from any of the gateway-relay command endpoints +(`add_authorized_client`, `authorized_clients`, `remove_authorized_client`, +`networking_status`) means the Powerwall gateway itself is unreachable and +raises `tesla_fleet_api.exceptions.EnergyGatewayUnreachable`, distinct from +the generic `tesla_fleet_api.exceptions.BadGateway` a 502 from any other +endpoint raises. + To revoke a key, call `remove_authorized_client(public_key)` with its DER bytes or the base64 string returned by `list_authorized_clients()`. Removal does not require physical presence proof: any paired key can revoke every other key, diff --git a/tesla_fleet_api/exceptions.py b/tesla_fleet_api/exceptions.py index e9a478f..0fe39ea 100644 --- a/tesla_fleet_api/exceptions.py +++ b/tesla_fleet_api/exceptions.py @@ -1,9 +1,15 @@ +import re from typing import Any import aiohttp from tesla_fleet_api.const import LOGGER +_ENERGY_GATEWAY_RELAY_PATH_RE = re.compile( + r"/api/1/energy_sites/[^/]+/command/" + r"(add_authorized_client|authorized_clients|remove_authorized_client|networking_status)$" +) + class TeslaFleetError(BaseException): """Base class for all Tesla exceptions.""" @@ -381,6 +387,31 @@ class InternalServerError(TeslaFleetError): status = 500 +class BadGateway(TeslaFleetError): + """The server, acting as a gateway, received an invalid response from an upstream server.""" + + message = ( + "The server, acting as a gateway, received an invalid response from an " + "upstream server." + ) + status = 502 + + +class EnergyGatewayUnreachable(BadGateway): + """The Powerwall energy gateway could not be reached via the gateway relay. + + Teslemetry's gateway relay answers with a 502 (with or without a JSON + body) when a customer's Powerwall gateway has dropped off the network - + a retryable condition, not an ordinary API failure. Raised only for the + gateway-relay endpoints the local-control pairing/authorized-clients flow + uses (``add_authorized_client``, ``authorized_clients``, + ``remove_authorized_client``, ``networking_status``); a 502 from any + other endpoint raises the generic ``BadGateway`` instead. + """ + + message = "The Powerwall energy gateway could not be reached via the gateway relay." + + class ServiceUnavailable(TeslaFleetError): """Either an internal service or a vehicle did not respond (timeout).""" @@ -1368,6 +1399,10 @@ async def raise_for_status(resp: aiohttp.ClientResponse) -> None: raise ClientClosedRequest(data) elif resp.status == 500: raise InternalServerError(data) + elif resp.status == 502: + if _ENERGY_GATEWAY_RELAY_PATH_RE.search(resp.url.path): + raise EnergyGatewayUnreachable(data) + raise BadGateway(data) elif resp.status == 503: raise ServiceUnavailable(data) elif resp.status == 504: diff --git a/tests/test_bad_gateway_classification.py b/tests/test_bad_gateway_classification.py new file mode 100644 index 0000000..f7fbaaf --- /dev/null +++ b/tests/test_bad_gateway_classification.py @@ -0,0 +1,106 @@ +"""502 classification: gateway-relay endpoints vs. everything else. + +A 502 must always become a ``TeslaFleetError`` subclass, whether or not the +response carries a JSON body - see ``tesla_fleet_api.exceptions.raise_for_status``. +Only the Powerwall local-control gateway-relay endpoints get the specific +``EnergyGatewayUnreachable``; every other 502 gets the generic ``BadGateway``. +""" + +from contextlib import asynccontextmanager +from unittest import IsolatedAsyncioTestCase +from unittest.mock import AsyncMock, MagicMock + +from yarl import URL + +from tesla_fleet_api.const import Method +from tesla_fleet_api.exceptions import BadGateway, EnergyGatewayUnreachable +from tesla_fleet_api.tesla.fleet import TeslaFleetApi + + +class _RequestTestApi(TeslaFleetApi): + """Expose the protected _request for testing.""" + + async def request( + self, + method: Method, + path: str, + params: dict[str, object] | None = None, + json: dict[str, object] | None = {}, + ) -> dict[str, object]: + return await self._request(method, path, params=params, json=json) + + +def _make_api(*, response: object) -> _RequestTestApi: + session = MagicMock() + + @asynccontextmanager + async def _ctx(*args, **kwargs): + yield response + + session.request = MagicMock(side_effect=lambda *a, **k: _ctx(*a, **k)) + return _RequestTestApi( + session=session, + access_token="access-token", + server="https://fleet.example.com", + ) + + +def _fake_response( + *, + path: str, + content_type: str = "application/json", + json_body: object = None, + text_body: str = "", +): + resp = MagicMock() + resp.status = 502 + resp.ok = False + resp.content_type = content_type + resp.url = URL(f"https://fleet.example.com{path}") + resp.headers = {} + resp.json = AsyncMock(return_value=json_body if json_body is not None else {}) + resp.text = AsyncMock(return_value=text_body) + return resp + + +class BadGatewayClassificationTests(IsolatedAsyncioTestCase): + async def test_gateway_relay_502_with_json_body_raises_energy_gateway_unreachable( + self, + ) -> None: + resp = _fake_response( + path="/api/1/energy_sites/123/command/add_authorized_client", + json_body={"error": "gateway unreachable"}, + ) + api = _make_api(response=resp) + with self.assertRaises(EnergyGatewayUnreachable): + await api.request( + Method.POST, "api/1/energy_sites/123/command/add_authorized_client" + ) + + async def test_gateway_relay_bodyless_502_raises_energy_gateway_unreachable( + self, + ) -> None: + resp = _fake_response( + path="/api/1/energy_sites/123/command/authorized_clients", + content_type="text/plain", + text_body="", + ) + api = _make_api(response=resp) + with self.assertRaises(EnergyGatewayUnreachable): + await api.request( + Method.GET, "api/1/energy_sites/123/command/authorized_clients" + ) + + async def test_non_gateway_endpoint_502_raises_generic_bad_gateway(self) -> None: + resp = _fake_response( + path="/api/1/vehicles/123/vehicle_data", + json_body={"error": "upstream error"}, + ) + api = _make_api(response=resp) + with self.assertRaises(BadGateway): + await api.request(Method.GET, "api/1/vehicles/123/vehicle_data") + # And it must not be misclassified as gateway-unreachable. + try: + await api.request(Method.GET, "api/1/vehicles/123/vehicle_data") + except BadGateway as e: + self.assertNotIsInstance(e, EnergyGatewayUnreachable) From c43cffde0775066526ecf1fb40e3fac46d91c59f Mon Sep 17 00:00:00 2001 From: Brett Adams Date: Sat, 5 Sep 2026 19:14:59 +1000 Subject: [PATCH 2/2] fix(exceptions): drop EnergyGatewayUnreachable, keep generic BadGateway for all 502s Captain review: a Powerwall gateway-relay 502 is not semantically distinct from any other 502, so a dedicated exception and per-endpoint mapping added unwarranted surface. Keep only the classification fix - any 502, bodied or bodyless, now raises BadGateway(TeslaFleetError) instead of leaking a raw aiohttp.ClientResponseError for the JSON-bodied case. Claude-Session: https://claude.ai/code/session_01Aukcht9BRfsyGsnM1x6mnr --- docs/energy_local_control.md | 9 +++---- tesla_fleet_api/exceptions.py | 23 ------------------ tests/test_bad_gateway_classification.py | 30 ++++++++---------------- 3 files changed, 13 insertions(+), 49 deletions(-) diff --git a/docs/energy_local_control.md b/docs/energy_local_control.md index f9160d0..3332aac 100644 --- a/docs/energy_local_control.md +++ b/docs/energy_local_control.md @@ -200,12 +200,9 @@ a genuinely empty client list. Catch `InvalidResponse` (or will not catch it. Either way, a `null` response here tells you nothing about whether the key actually works. -A 502 from any of the gateway-relay command endpoints -(`add_authorized_client`, `authorized_clients`, `remove_authorized_client`, -`networking_status`) means the Powerwall gateway itself is unreachable and -raises `tesla_fleet_api.exceptions.EnergyGatewayUnreachable`, distinct from -the generic `tesla_fleet_api.exceptions.BadGateway` a 502 from any other -endpoint raises. +Any 502 response, including one from a gateway-relay command endpoint, raises +`tesla_fleet_api.exceptions.BadGateway` regardless of whether the response +carries a JSON body. To revoke a key, call `remove_authorized_client(public_key)` with its DER bytes or the base64 string returned by `list_authorized_clients()`. Removal does not diff --git a/tesla_fleet_api/exceptions.py b/tesla_fleet_api/exceptions.py index 0fe39ea..acc62e6 100644 --- a/tesla_fleet_api/exceptions.py +++ b/tesla_fleet_api/exceptions.py @@ -1,15 +1,9 @@ -import re from typing import Any import aiohttp from tesla_fleet_api.const import LOGGER -_ENERGY_GATEWAY_RELAY_PATH_RE = re.compile( - r"/api/1/energy_sites/[^/]+/command/" - r"(add_authorized_client|authorized_clients|remove_authorized_client|networking_status)$" -) - class TeslaFleetError(BaseException): """Base class for all Tesla exceptions.""" @@ -397,21 +391,6 @@ class BadGateway(TeslaFleetError): status = 502 -class EnergyGatewayUnreachable(BadGateway): - """The Powerwall energy gateway could not be reached via the gateway relay. - - Teslemetry's gateway relay answers with a 502 (with or without a JSON - body) when a customer's Powerwall gateway has dropped off the network - - a retryable condition, not an ordinary API failure. Raised only for the - gateway-relay endpoints the local-control pairing/authorized-clients flow - uses (``add_authorized_client``, ``authorized_clients``, - ``remove_authorized_client``, ``networking_status``); a 502 from any - other endpoint raises the generic ``BadGateway`` instead. - """ - - message = "The Powerwall energy gateway could not be reached via the gateway relay." - - class ServiceUnavailable(TeslaFleetError): """Either an internal service or a vehicle did not respond (timeout).""" @@ -1400,8 +1379,6 @@ async def raise_for_status(resp: aiohttp.ClientResponse) -> None: elif resp.status == 500: raise InternalServerError(data) elif resp.status == 502: - if _ENERGY_GATEWAY_RELAY_PATH_RE.search(resp.url.path): - raise EnergyGatewayUnreachable(data) raise BadGateway(data) elif resp.status == 503: raise ServiceUnavailable(data) diff --git a/tests/test_bad_gateway_classification.py b/tests/test_bad_gateway_classification.py index f7fbaaf..9aefc2c 100644 --- a/tests/test_bad_gateway_classification.py +++ b/tests/test_bad_gateway_classification.py @@ -1,9 +1,8 @@ -"""502 classification: gateway-relay endpoints vs. everything else. +"""502 classification: always a TeslaFleetError, regardless of body shape. -A 502 must always become a ``TeslaFleetError`` subclass, whether or not the -response carries a JSON body - see ``tesla_fleet_api.exceptions.raise_for_status``. -Only the Powerwall local-control gateway-relay endpoints get the specific -``EnergyGatewayUnreachable``; every other 502 gets the generic ``BadGateway``. +A 502 must always become a ``TeslaFleetError`` subclass instead of leaking a +raw ``aiohttp.ClientResponseError`` - see +``tesla_fleet_api.exceptions.raise_for_status``. """ from contextlib import asynccontextmanager @@ -13,7 +12,7 @@ from yarl import URL from tesla_fleet_api.const import Method -from tesla_fleet_api.exceptions import BadGateway, EnergyGatewayUnreachable +from tesla_fleet_api.exceptions import BadGateway from tesla_fleet_api.tesla.fleet import TeslaFleetApi @@ -64,34 +63,30 @@ def _fake_response( class BadGatewayClassificationTests(IsolatedAsyncioTestCase): - async def test_gateway_relay_502_with_json_body_raises_energy_gateway_unreachable( - self, - ) -> None: + async def test_json_bodied_502_raises_bad_gateway(self) -> None: resp = _fake_response( path="/api/1/energy_sites/123/command/add_authorized_client", json_body={"error": "gateway unreachable"}, ) api = _make_api(response=resp) - with self.assertRaises(EnergyGatewayUnreachable): + with self.assertRaises(BadGateway): await api.request( Method.POST, "api/1/energy_sites/123/command/add_authorized_client" ) - async def test_gateway_relay_bodyless_502_raises_energy_gateway_unreachable( - self, - ) -> None: + async def test_bodyless_502_raises_bad_gateway(self) -> None: resp = _fake_response( path="/api/1/energy_sites/123/command/authorized_clients", content_type="text/plain", text_body="", ) api = _make_api(response=resp) - with self.assertRaises(EnergyGatewayUnreachable): + with self.assertRaises(BadGateway): await api.request( Method.GET, "api/1/energy_sites/123/command/authorized_clients" ) - async def test_non_gateway_endpoint_502_raises_generic_bad_gateway(self) -> None: + async def test_non_gateway_endpoint_502_also_raises_bad_gateway(self) -> None: resp = _fake_response( path="/api/1/vehicles/123/vehicle_data", json_body={"error": "upstream error"}, @@ -99,8 +94,3 @@ async def test_non_gateway_endpoint_502_raises_generic_bad_gateway(self) -> None api = _make_api(response=resp) with self.assertRaises(BadGateway): await api.request(Method.GET, "api/1/vehicles/123/vehicle_data") - # And it must not be misclassified as gateway-unreachable. - try: - await api.request(Method.GET, "api/1/vehicles/123/vehicle_data") - except BadGateway as e: - self.assertNotIsInstance(e, EnergyGatewayUnreachable)