From 7f641fab3ebf7238c0113853b3881933e81fed7f Mon Sep 17 00:00:00 2001 From: firstmate crewmate Date: Sun, 12 Jul 2026 20:21:15 +1000 Subject: [PATCH 1/3] fix(fleet): guard _log_request_result against non-dict JSON bodies A JSON-legal response body that isn't a dict (null, list, or scalar - e.g. Teslemetry's list_authorized_clients returning null) crashed _request AFTER the HTTP request had already succeeded, since _log_request_result unconditionally called data.get() assuming a dict. It's DEBUG-level logging added purely for troubleshooting (#64) and must never break a successful request. --- tesla_fleet_api/tesla/fleet.py | 8 ++++- tests/test_command_logging.py | 55 ++++++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/tesla_fleet_api/tesla/fleet.py b/tesla_fleet_api/tesla/fleet.py index 51f2987..4138dd7 100644 --- a/tesla_fleet_api/tesla/fleet.py +++ b/tesla_fleet_api/tesla/fleet.py @@ -33,7 +33,13 @@ def _normalize_query_value(value: Any) -> Any: return value -def _log_request_result(command: str, transport: str, data: dict[str, Any]) -> None: +def _log_request_result(command: str, transport: str, data: Any) -> None: + """Log the outcome of a completed request; a logging convenience that must + never raise, since the request it describes has already succeeded.""" + if not isinstance(data, dict): + LOGGER.debug("command=%s transport=%s result=success", command, transport) + return + data = cast("dict[str, Any]", data) response = data.get("response") if isinstance(response, dict) and "result" in response: result_data = cast("dict[str, Any]", response) diff --git a/tests/test_command_logging.py b/tests/test_command_logging.py index 3edf36f..11a5195 100644 --- a/tests/test_command_logging.py +++ b/tests/test_command_logging.py @@ -98,12 +98,15 @@ async def test_verify_commands_resolution_logs_verified_outcome(self) -> None: ) +_UNSET = object() + + def _fake_response( *, status: int = 200, ok: bool = True, content_type: str = "application/json", - json_body: object = None, + json_body: object = _UNSET, ): resp = MagicMock() resp.status = status @@ -111,7 +114,7 @@ def _fake_response( resp.content_type = content_type resp.url = "https://example.com/x" resp.headers = {} - resp.json = AsyncMock(return_value=json_body if json_body is not None else {}) + resp.json = AsyncMock(return_value={} if json_body is _UNSET else json_body) resp.text = AsyncMock(return_value="") return resp @@ -230,6 +233,54 @@ async def test_fleet_failure_logs_command_transport_and_error(self) -> None: captured.output, ) + async def test_null_json_body_returns_none_without_raising(self) -> None: + resp = _fake_response(json_body=None) + api = TeslaFleetApi( + session=_make_session(resp), + access_token="token", + server="https://fleet.example.com", + ) + + with self.assertLogs(LOGGER_NAME, level="DEBUG") as captured: + result = await api._request( + Method.GET, "api/1/vehicles/VIN123/list_authorized_clients" + ) + + self.assertIsNone(result) + self.assertTrue( + any( + "command=list_authorized_clients" in line + and "transport=fleet" in line + and "result=success" in line + for line in captured.output + ), + captured.output, + ) + + async def test_list_json_body_returns_list_without_raising(self) -> None: + resp = _fake_response(json_body=[{"id": 1}]) + api = TeslaFleetApi( + session=_make_session(resp), + access_token="token", + server="https://fleet.example.com", + ) + + result = await api._request(Method.GET, "api/1/vehicles/VIN123/clients") + + self.assertEqual(result, [{"id": 1}]) + + async def test_scalar_json_body_returns_scalar_without_raising(self) -> None: + resp = _fake_response(json_body=True) + api = TeslaFleetApi( + session=_make_session(resp), + access_token="token", + server="https://fleet.example.com", + ) + + result = await api._request(Method.GET, "api/1/vehicles/VIN123/flag") + + self.assertEqual(result, True) + async def test_teslemetry_success_logs_transport_teslemetry(self) -> None: resp = _fake_response(json_body={"response": {"result": True}}) api = Teslemetry(session=_make_session(resp), access_token="token") From 70d2479c2e3db3241fc85fe214e2c0dc1cc43bd4 Mon Sep 17 00:00:00 2001 From: firstmate crewmate Date: Sun, 12 Jul 2026 20:21:51 +1000 Subject: [PATCH 2/3] docs: note _log_request_result's non-dict body guard in AGENTS.md --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index c3cfbf7..507c5c5 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -140,6 +140,7 @@ Source protobuf definitions live in `proto/`; generated Python files live in `te - **Idle BLE keepalive (`keepalive_interval`)**: an idle held BLE link to the vehicle drops at ~42s mean lifetime (link supervision timeout ~720ms underneath, bluetoothd-verified on macOS CoreBluetooth); a single trivial passive GATT read every ~20s extends lifetime ~10x (~400s observed). `VehicleBluetooth.__init__`'s `keepalive_interval` (default `DEFAULT_KEEPALIVE_INTERVAL` = 20.0, `None`/`0` disables; threaded through `Vehicles`/`VehiclesBluetooth.create*`) starts one asyncio task per connection (`_keepalive_loop`, `bluetooth.py`) that reads `VERSION_UUID` only after `keepalive_interval` seconds of genuine GATT idleness. **Idle-triggered, not periodic**: `_last_activity` is bumped on every `_send` write and every `_on_notify` frame, so an active session never gets extra traffic; the loop recomputes the wait each pass and fires only when idle. The read is **bounded** (`_keepalive_timeout`, 2s) and **best-effort** - a prior un-timed RSSI read hung indefinitely against a sleeping car, so every attempt carries a timeout and swallows all failures (`_keepalive_read` catches `Exception`, never `CancelledError`); a failed keepalive never raises into user code, never triggers reconnect (the existing `connect_if_needed` machinery owns recovery), and never wakes the car. Task lifecycle is tied to the connection: started at the end of `connect()` (after `start_notify`), cancelled-and-awaited in `disconnect()` and restarted cleanly on reconnect (`_start_keepalive`/`_stop_keepalive`). **Sleep tradeoff**: these reads keep an *awake* car awake and defer vehicle sleep - consumers wanting the car to sleep should disable keepalive or disconnect when idle. Tests in `tests/test_ble_keepalive.py`. - **Cross-transport parity (cloud REST `VehicleFleet` vs BLE `Commands`)**: the same-named command on both paths should build a semantically equivalent instruction from identical args - a divergence there is a bug, but response *bodies* legitimately differ (REST JSON dict vs decoded protobuf) and are not. `tests/test_cross_transport_parity.py` locks the equivalence in with mocked-both-transports tests. Known **non-bug FORM differences** (do not "fix"): `set_scheduled_departure`'s `preconditioning_enabled`/`off_peak_charging_enabled` (no proto fields), `window_control` lat/lon and `navigation_sc_request` `id` (no proto fields), `navigation_request`'s `type`/`locale`/`timestamp_ms` (REST share-intent framing), and `media_volume_up` (no Tesla REST endpoint - BLE-only; cloud raises volume via `adjust_volume`). Two **open divergences left unfixed** pending live verification: `clear_pin_to_drive_admin` builds `DrivingClearSpeedLimitPinAction` (speed-limit PIN, not PIN-to-Drive - suspected mismapping, security-sensitive), and `navigation_gps_request`'s `order` is required on BLE but optional on cloud (signature mismatch; null-order wire semantics undecided). - **Per-command debug logging chokepoints and the `command=` name it derives**: `LOGGER.debug` lines of the form `command= transport= result=...` are emitted from exactly four places, not per-method - `Commands._sendVehicleSecurity`/`_getVehicleSecurity`/`_sendInfotainment`/`_getInfotainment` (`commands.py`, covers both BLE and Fleet-signed) and `TeslaFleetApi._request` (`fleet.py`, covers Fleet/Teslemetry/Tessie REST). `transport` comes from a `_transport_name` `ClassVar` set per concrete class (`"bluetooth"`/`"fleet"`/`"teslemetry"`/`"tessie"`), mirroring the existing `_auth_method` pattern - add that ClassVar to any new `Commands`/`TeslaFleetApi` subclass. For BLE/Fleet-signed, `command` is **not** the Python method name; it's derived from the populated protobuf oneof field (`vcsec_command_name`/`infotainment_command_name` in `commands.py`), e.g. `door_lock()` logs as `RKE_ACTION_LOCK` and `set_charge_limit()` as `chargingSetLimitAction` - deliberately robust to call-site changes since it reads the message being sent, not the call stack. `VehicleBluetooth`'s `verify_commands` resolution logs a second, separate line (`verify_commands=resolved`/`unresolved`) rather than duplicating the base class's raw-attempt line. `Router._dispatch` (`router/base.py`) logs `command=... backend= result=...` per backend tried, independent of the above. See `docs/bluetooth_vehicles.md`'s "Troubleshooting: Enable Debug Logging" section for the user-facing format; `tests/test_command_logging.py` locks in the exact line shapes. +- **`_log_request_result` (`fleet.py`) must tolerate any JSON-legal REST body, not just dicts**: it runs after the HTTP request already succeeded, so it's a logging convenience only - a non-dict body (`null`, a list, a bare scalar; live case: Teslemetry's `list_authorized_clients` returning `null`) must never raise there. It guards with `isinstance(data, dict)` before calling `.get()`, logging `result=success` and returning for anything else. Regression tests in `tests/test_command_logging.py` (`test_null_json_body_returns_none_without_raising` etc.) cover null/list/scalar bodies. ## Maintaining this file From e35866d2e903234f75de1b5e0ca66c1f86bab30a Mon Sep 17 00:00:00 2001 From: firstmate crewmate Date: Sun, 12 Jul 2026 20:28:51 +1000 Subject: [PATCH 3/3] no-mistakes(document): Document REST non-object responses --- README.md | 2 ++ docs/bluetooth_vehicles.md | 2 ++ docs/fleet_api_energy_sites.md | 3 +++ docs/fleet_api_vehicles.md | 3 +++ docs/teslemetry.md | 3 +++ docs/tessie.md | 3 +++ tesla_fleet_api/tesla/fleet.py | 6 +++++- tesla_fleet_api/teslemetry/energysite.py | 3 +++ 8 files changed, 24 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index defbe67..1ab110e 100644 --- a/README.md +++ b/README.md @@ -257,6 +257,8 @@ Command log lines use `transport=bluetooth`, `fleet`, `teslemetry`, or `tessie`. Routers also emit `backend=` lines for each backend tried. See [Bluetooth for Vehicles](docs/bluetooth_vehicles.md#troubleshooting-enable-debug-logging) for examples and the signed-command naming details. +REST responses that are valid JSON but not objects, such as `null`, lists, or +scalars, are returned unchanged and log as `result=success`. ### Teslemetry diff --git a/docs/bluetooth_vehicles.md b/docs/bluetooth_vehicles.md index c18df66..29ba904 100644 --- a/docs/bluetooth_vehicles.md +++ b/docs/bluetooth_vehicles.md @@ -531,6 +531,8 @@ command=mediaPlayAction transport=bluetooth result=error error=BluetoothUnconfir commands, `command` is the underlying VCSEC/infotainment field name (e.g. `RKE_ACTION_LOCK`, `chargingSetLimitAction`), not the Python method name; for REST commands it is the endpoint's final path segment (e.g. `set_charge_limit`). +REST responses that are valid JSON but not objects, such as `null`, lists, or +scalars, are returned unchanged and log as `result=success`. For BLE commands run with `confirmation="verify"`, a resolved state-read logs a second line with `verify_commands=resolved` and the confirmed result; an unresolved read logs `verify_commands=unresolved` before the exception diff --git a/docs/fleet_api_energy_sites.md b/docs/fleet_api_energy_sites.md index 593f207..f82f8a3 100644 --- a/docs/fleet_api_energy_sites.md +++ b/docs/fleet_api_energy_sites.md @@ -42,6 +42,9 @@ logging.basicConfig(level=logging.DEBUG) logging.getLogger("tesla_fleet_api").setLevel(logging.DEBUG) ``` +Responses that are valid JSON but not objects, such as `null`, lists, or +scalars, are returned unchanged and log as `result=success`. + ## Backup Reserve You can adjust the backup reserve for a specific energy site using its ID: diff --git a/docs/fleet_api_vehicles.md b/docs/fleet_api_vehicles.md index 39fdfc5..e0565e8 100644 --- a/docs/fleet_api_vehicles.md +++ b/docs/fleet_api_vehicles.md @@ -48,6 +48,9 @@ command=vehicle_data transport=fleet result=success command=set_charge_limit transport=fleet result=True reason= ``` +Responses that are valid JSON but not objects, such as `null`, lists, or +scalars, are returned unchanged and log as `result=success`. + ## Create a Vehicle You can create a `VehicleFleet` instance for a specific vehicle using its VIN: diff --git a/docs/teslemetry.md b/docs/teslemetry.md index 0e5b109..5071425 100644 --- a/docs/teslemetry.md +++ b/docs/teslemetry.md @@ -33,6 +33,9 @@ logging.basicConfig(level=logging.DEBUG) logging.getLogger("tesla_fleet_api").setLevel(logging.DEBUG) ``` +Responses that are valid JSON but not objects, such as `null`, lists, or +scalars, are returned unchanged and log as `result=success`. + ## Ping The `ping` method sends a ping request to the Teslemetry server. diff --git a/docs/tessie.md b/docs/tessie.md index c4b463d..8c33747 100644 --- a/docs/tessie.md +++ b/docs/tessie.md @@ -40,6 +40,9 @@ logging.basicConfig(level=logging.DEBUG) logging.getLogger("tesla_fleet_api").setLevel(logging.DEBUG) ``` +Responses that are valid JSON but not objects, such as `null`, lists, or +scalars, are returned unchanged and log as `result=success`. + ## Top-Level Client Methods These methods exist on `Tessie` itself. diff --git a/tesla_fleet_api/tesla/fleet.py b/tesla_fleet_api/tesla/fleet.py index 4138dd7..d39b234 100644 --- a/tesla_fleet_api/tesla/fleet.py +++ b/tesla_fleet_api/tesla/fleet.py @@ -145,7 +145,11 @@ async def _request( params: dict[str, Any] | None = None, json: dict[str, Any] | None = {}, ) -> dict[str, Any]: - """Send a request to the Tesla Fleet API.""" + """Send a request to the Tesla Fleet API. + + Returns the decoded JSON body as provided by the service, including + JSON-legal non-object bodies such as ``null``, lists, or scalars. + """ # Trailing path segment (e.g. "door_lock", "vehicle_data") as the # debug-log command name; the full path (with VIN) is already logged diff --git a/tesla_fleet_api/teslemetry/energysite.py b/tesla_fleet_api/teslemetry/energysite.py index b0e9908..aad6733 100644 --- a/tesla_fleet_api/teslemetry/energysite.py +++ b/tesla_fleet_api/teslemetry/energysite.py @@ -68,6 +68,9 @@ async def get_networking_status(self) -> dict[str, Any]: async def list_authorized_clients(self) -> dict[str, Any]: """List authorized clients on the energy gateway via the Teslemetry custom endpoint. + + Teslemetry may return JSON ``null`` when no authorized-client payload + is available; that value is returned unchanged. """ return await self._request( Method.GET,