From 1b717d11c4f49b871eff514c3058af59ff4b3693 Mon Sep 17 00:00:00 2001 From: townsmcp <14061617+townsmcp@users.noreply.github.com> Date: Sun, 25 Oct 2026 12:12:34 +0000 Subject: [PATCH 1/2] feat: keep SAIC's return code and message on every error (#54) SaicApiException now keeps the return code and SAIC's message as separate attributes (return_code, saic_message) and offers checks for the common cases (is_vehicle_unreachable, is_request_rejected, is_vehicle_not_locked, is_logged_out, is_unexpected_failure), with the known codes in SaicReturnCode. str(e) is unchanged, so code that reads the text keeps working. --- README.md | 10 +- src/saic_ismart_client_ng/exceptions.py | 70 ++++++++++++- tests/test_exceptions.py | 133 ++++++++++++++++++++++++ 3 files changed, 211 insertions(+), 2 deletions(-) create mode 100644 tests/test_exceptions.py diff --git a/README.md b/README.md index 1bfdab2..40dc50d 100644 --- a/README.md +++ b/README.md @@ -35,4 +35,12 @@ Supported functionality (partial list, check the code for more) ## Prerequisites -You have an iSMART account (can be created in the iSMART app) \ No newline at end of file +You have an iSMART account (can be created in the iSMART app) + +## Errors + +Every error is a `SaicApiException` (or a subclass). Besides the message text, it carries SAIC's reply as separate values, so you don't need to parse the text: + +* `return_code`: SAIC's numeric code (see `SaicReturnCode`), or `None` +* `saic_message`: the message on its own +* checks for the common cases: `is_vehicle_unreachable` (code 4), `is_request_rejected` (code 8), `is_vehicle_not_locked`, `is_logged_out` and `is_unexpected_failure` diff --git a/src/saic_ismart_client_ng/exceptions.py b/src/saic_ismart_client_ng/exceptions.py index c7fad69..c236981 100644 --- a/src/saic_ismart_client_ng/exceptions.py +++ b/src/saic_ismart_client_ng/exceptions.py @@ -1,19 +1,87 @@ from __future__ import annotations +from enum import IntEnum + + +class SaicReturnCode(IntEnum): + """SAIC return codes with a known meaning. + + SAIC replies with a numeric ``code`` and a ``message``. Codes not listed + here still come through unchanged in ``SaicApiException.return_code``. + """ + + SUCCESS = 0 + # "The remote control instruction failed, please try again later." Seen + # when the car can't be reached (e.g. asleep), and briefly while it wakes. + VEHICLE_UNREACHABLE = 4 + SERVICE_UNAVAILABLE = 6 + # The request was refused. SAIC uses this for several different reasons + # (vehicle not locked, too many remote commands, ...): read the message. + REQUEST_REJECTED = 8 + UNAUTHORIZED = 401 + FORBIDDEN = 403 + # Not from SAIC: the library's own code for a request that failed in an + # unexpected way (no usable reply, connection error, ...). + UNEXPECTED_FAILURE = 500 + class SaicApiException(Exception): + """An error from the SAIC API (or from talking to it). + + ``str(e)`` is unchanged: ``"return code: , message: "``, or + just the message when there is no code. The two parts are also available + on their own, so callers don't have to parse that text: + + - ``return_code``: the numeric code, or ``None`` + - ``saic_message``: the message on its own (SAIC's words when the error + came from a SAIC reply) + """ + def __init__(self, msg: str, return_code: int | None = None) -> None: + self.return_code: int | None = return_code + self.saic_message: str = msg if return_code is not None: self.message = f"return code: {return_code}, message: {msg}" else: self.message = msg + super().__init__(self.message) def __str__(self) -> str: return self.message + @property + def is_vehicle_unreachable(self) -> bool: + """SAIC couldn't reach the car (return code 4).""" + return self.return_code == SaicReturnCode.VEHICLE_UNREACHABLE + + @property + def is_request_rejected(self) -> bool: + """SAIC refused the request (return code 8), for any reason.""" + return self.return_code == SaicReturnCode.REQUEST_REJECTED + + @property + def is_vehicle_not_locked(self) -> bool: + """Refused because the vehicle isn't locked (code 8, "not locked").""" + return self.is_request_rejected and "not locked" in self.saic_message.lower() + + @property + def is_logged_out(self) -> bool: + """The session is no longer valid; log in again.""" + return self.return_code in ( + SaicReturnCode.UNAUTHORIZED, + SaicReturnCode.FORBIDDEN, + ) + + @property + def is_unexpected_failure(self) -> bool: + """The library's own code for a request that failed unexpectedly.""" + return self.return_code == SaicReturnCode.UNEXPECTED_FAILURE + class SaicLogoutException(SaicApiException): - pass + @property + def is_logged_out(self) -> bool: + return True class SaicApiClosedException(SaicApiException): diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py new file mode 100644 index 0000000..3f6f134 --- /dev/null +++ b/tests/test_exceptions.py @@ -0,0 +1,133 @@ +from __future__ import annotations + +import asyncio +import json +import pickle + +import httpx +import pytest + +from saic_ismart_client_ng import SaicApi +from saic_ismart_client_ng.exceptions import ( + SaicApiException, + SaicApiRetryException, + SaicLogoutException, + SaicReturnCode, +) +from saic_ismart_client_ng.model import SaicApiConfiguration + + +def test_text_is_unchanged() -> None: + e = SaicApiException("Vehicle not locked.", return_code=8) + assert str(e) == "return code: 8, message: Vehicle not locked." + assert e.message == "return code: 8, message: Vehicle not locked." + assert str(SaicApiException("plain")) == "plain" + + +def test_retry_exception_text_is_unchanged() -> None: + e = SaicApiRetryException("try again", event_id="123", return_code=4) + assert str(e) == "return code: 4, message: try again, event_id: 123" + assert e.event_id == "123" + + +def test_code_and_message_kept_separately() -> None: + e = SaicApiException("Vehicle not locked.", return_code=8) + assert e.return_code == SaicReturnCode.REQUEST_REJECTED + assert e.saic_message == "Vehicle not locked." + plain = SaicApiException("plain") + assert plain.return_code is None + assert plain.saic_message == "plain" + + +def test_args_are_set() -> None: + assert SaicApiException("x", return_code=6).args == ("return code: 6, message: x",) + + +@pytest.mark.parametrize( + ("code", "message", "unreachable", "rejected", "not_locked", "logged_out"), + [ + (4, "The remote control instruction failed", True, False, False, False), + (8, "Vehicle not locked.", False, True, True, False), + ( + 8, + "Too many commands, start the vehicle with the key", + False, + True, + False, + False, + ), + (6, "Service not available", False, False, False, False), + (401, "token expired", False, False, False, True), + (403, "forbidden", False, False, False, True), + (None, "no code", False, False, False, False), + ], +) +def test_checks( + code: int | None, + message: str, + unreachable: bool, + rejected: bool, + not_locked: bool, + logged_out: bool, +) -> None: + e = SaicApiException(message, return_code=code) + assert e.is_vehicle_unreachable is unreachable + assert e.is_request_rejected is rejected + assert e.is_vehicle_not_locked is not_locked + assert e.is_logged_out is logged_out + + +def test_retry_exception_has_the_checks_too() -> None: + e = SaicApiRetryException("failed", event_id="9", return_code=4) + assert e.is_vehicle_unreachable + assert isinstance(e, SaicApiException) + + +def test_logout_exception_is_always_logged_out() -> None: + assert SaicLogoutException("bye").is_logged_out + assert SaicLogoutException('{"code":401}', 401).is_logged_out + + +def test_unexpected_failure() -> None: + e = SaicApiException("API call POST /vehicle/control failed unexpectedly", 500) + assert e.is_unexpected_failure + assert not SaicApiException("x", 8).is_unexpected_failure + + +def test_picklable() -> None: + e = pickle.loads(pickle.dumps(SaicApiException("x", return_code=8))) # noqa: S301 + assert str(e) == "return code: 8, message: x" + + +# -- Through the real response handling -------------------------------------- + + +def _deserialize(payload: dict[str, object], *, event_id: str | None = None) -> None: + api = SaicApi(SaicApiConfiguration(username="offline@example.com", password="x")) # noqa: S106 + headers = {"event-id": event_id} if event_id else {} + request = httpx.Request("POST", "https://example.invalid/x", headers=headers) + response = httpx.Response( + 200, content=json.dumps(payload).encode(), request=request + ) + deserialize = getattr(api, "_AbstractSaicApi__deserialize") # noqa: B009 + asyncio.run(deserialize(request, response, None, allow_null_body=True)) + + +def test_rejection_from_a_real_reply_keeps_saics_words() -> None: + with pytest.raises(SaicApiException) as info: + _deserialize({"code": 8, "message": "Vehicle not locked."}) + e = info.value + assert e.return_code == 8 + assert e.saic_message == "Vehicle not locked." + assert e.is_vehicle_not_locked + assert str(e) == "return code: 8, message: Vehicle not locked." + + +def test_unreachable_from_a_real_reply_during_event_polling() -> None: + with pytest.raises(SaicApiRetryException) as info: + _deserialize( + {"code": 4, "message": "The remote control instruction failed"}, + event_id="1118530795", + ) + assert info.value.is_vehicle_unreachable + assert info.value.saic_message == "The remote control instruction failed" From 722688199e2a1894755a2331a430a5d870501268 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 25 Oct 2026 15:14:43 +0100 Subject: [PATCH 2/2] fix: keep return code and event id when exceptions are pickled --- src/saic_ismart_client_ng/exceptions.py | 16 ++++++++++++++++ tests/test_exceptions.py | 19 +++++++++++++++++++ 2 files changed, 35 insertions(+) diff --git a/src/saic_ismart_client_ng/exceptions.py b/src/saic_ismart_client_ng/exceptions.py index c236981..c197c2d 100644 --- a/src/saic_ismart_client_ng/exceptions.py +++ b/src/saic_ismart_client_ng/exceptions.py @@ -1,6 +1,8 @@ from __future__ import annotations from enum import IntEnum +from functools import partial +from typing import Any class SaicReturnCode(IntEnum): @@ -49,6 +51,11 @@ def __init__(self, msg: str, return_code: int | None = None) -> None: def __str__(self) -> str: return self.message + def __reduce__(self) -> tuple[Any, ...]: + # ``args`` holds the formatted text, so the default would rebuild the + # exception with ``return_code=None`` and the text as its message. + return self.__class__, (self.saic_message, self.return_code) + @property def is_vehicle_unreachable(self) -> bool: """SAIC couldn't reach the car (return code 4).""" @@ -99,5 +106,14 @@ def __init__( def event_id(self) -> str: return self.__event_id + def __reduce__(self) -> tuple[Any, ...]: + # ``event_id`` is keyword-only, so it can't be passed positionally. + return ( + partial( + self.__class__, event_id=self.event_id, return_code=self.return_code + ), + (self.saic_message,), + ) + def __str__(self) -> str: return f"{self.message}, event_id: {self.event_id}" diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py index 3f6f134..99e42e9 100644 --- a/tests/test_exceptions.py +++ b/tests/test_exceptions.py @@ -97,6 +97,25 @@ def test_unexpected_failure() -> None: def test_picklable() -> None: e = pickle.loads(pickle.dumps(SaicApiException("x", return_code=8))) # noqa: S301 assert str(e) == "return code: 8, message: x" + assert e.return_code == 8 + assert e.saic_message == "x" + + +def test_retry_exception_picklable() -> None: + e = pickle.loads( # noqa: S301 + pickle.dumps(SaicApiRetryException("x", event_id="9", return_code=4)) + ) + assert isinstance(e, SaicApiRetryException) + assert str(e) == "return code: 4, message: x, event_id: 9" + assert e.return_code == 4 + assert e.saic_message == "x" + assert e.event_id == "9" + + +def test_logout_exception_picklable() -> None: + e = pickle.loads(pickle.dumps(SaicLogoutException("bye", 401))) # noqa: S301 + assert isinstance(e, SaicLogoutException) + assert e.return_code == 401 # -- Through the real response handling --------------------------------------