From 00a6cc52fa265e6db60e08ee08796d152ad6698d Mon Sep 17 00:00:00 2001 From: Joshua Leaper Date: Thu, 13 Aug 2026 15:49:27 +0930 Subject: [PATCH 1/3] Add opt-in AlphaESSApiError so callers can detect API failures The write endpoints answer with data: null whether they succeed or fail, and api_post returns json_response["data"] on success and None on failure, so a successful write is indistinguishable from a rejected one. That left callers guessing, which is how homeassistant-alphaESS ended up gating a write on an unrelated read. Add raise_on_error, defaulting to False. Left alone nothing changes: API-level failures still log and return None. Set it and they raise AlphaESSApiError carrying code, msg, expMsg, description and path, so success becomes "did not raise" rather than a return value nobody can interpret. Kept backwards compatible on purpose, since people upgrade without reading changelogs: - no return type or value changes anywhere - raise_on_error is appended last in __init__, so positional callers are fine - transport exceptions propagate exactly as before - the existing 34 tests pass untouched, and homeassistant-alphaESS's 342 tests pass against this build without opting in Also verified against the live API and corrected in the docs: - an empty period list is rejected with 6001 "time list is null"; the docs previously claimed [] was acceptable. Omitting one gives 10001, a code outside the 6xxx range and absent from the portal's table. - parameter validation runs before the entitlement check, so an early 6001 is not evidence that the account has permission - expMsg is often the only field naming the bad parameter, and was neither logged nor exposed - the portal registry holds 21 interfaces, not 19: getMeterOffsetConfigInfo and updateMeterOffsetConfigInfo are scoped to the commercial & industrial document and are not callable from a standard account - getMeterOffsetConfigInfo is GET; the portal documents it as POST, which 405s --- README.md | 27 ++++- alphaess/alphaess.py | 154 +++++++++++++++++++++++++++-- docs/API.md | 47 ++++++++- docs/RETURN_CODES.md | 50 ++++++++++ setup.py | 2 +- tests/test_raise_on_error.py | 186 +++++++++++++++++++++++++++++++++++ 6 files changed, 451 insertions(+), 15 deletions(-) create mode 100644 tests/test_raise_on_error.py diff --git a/README.md b/README.md index ee25608..7c0d515 100644 --- a/README.md +++ b/README.md @@ -23,9 +23,34 @@ transcribed from the portal and verified endpoint by endpoint against the live A + **[docs/RETURN_CODES.md](docs/RETURN_CODES.md)** — the complete return code table (both pages of the portal's paginated list), grouped by cause, plus codes the portal does not publish. -Four things the official documentation gets wrong are corrected in +Things the official documentation gets wrong are corrected in [docs/API.md](docs/API.md#corrections-to-the-official-documentation). +## Error handling + +By default an API-level failure (a non-`200` `code` in the response) is logged and the method +returns `None`, while transport failures — connection resets, timeouts, non-2xx HTTP — raise. + +That default makes a successful write indistinguishable from a rejected one, because the write +endpoints answer with `data: null` either way. If you need to tell them apart, construct the +client with `raise_on_error=True`: + +```python +from alphaess.alphaess import alphaess, AlphaESSApiError + +client = alphaess(appID, appSecret, raise_on_error=True) + +try: + await client.setTimeChargeBySn(sysSn, 0, charge_list, discharge_list) +except AlphaESSApiError as err: + print(err.code, err.expMsg) # e.g. 6001 "time list is null" +``` + +Success is then signalled by the absence of an exception; return values are unchanged. The flag +defaults to `False`, so upgrading does not alter existing behaviour. See +[docs/RETURN_CODES.md](docs/RETURN_CODES.md#opting-in-to-exceptions--raise_on_error-0021) for the +full exception reference. + # Methods There are public methods in this module that duplicate the AlphaESS OpenAPI and provide wrappers for diff --git a/alphaess/alphaess.py b/alphaess/alphaess.py index ec07bff..4ccea78 100644 --- a/alphaess/alphaess.py +++ b/alphaess/alphaess.py @@ -37,9 +37,40 @@ # RETURN_CODES stays a faithful copy of the documented table. UNDOCUMENTED_RETURN_CODES = { 6017: "No operation permissions", + 10001: "Parameter error (malformed request body, e.g. a required list omitted)", } +class AlphaESSApiError(Exception): + """The API answered, but reported a failure. + + Only raised when the client was created with ``raise_on_error=True``. By + default an API-level failure still returns ``None``, exactly as it did in + 0.0.20 and earlier. + + Distinct from the aiohttp transport exceptions: this means the service was + reached and rejected the request, so retrying unchanged will not help. + """ + + def __init__(self, code, msg=None, expMsg=None, path=None, description=None): + self.code = code + self.msg = msg + self.expMsg = expMsg + self.path = path + self.description = description + + detail = f"{code}" + if description: + detail += f" ({description})" + if msg: + detail += f": {msg}" + if expMsg: + detail += f" - {expMsg}" + if path: + detail += f" when calling {path}" + super().__init__(detail) + + class alphaess: """Class for Alpha ESS.""" @@ -50,9 +81,17 @@ def __init__( session: aiohttp.ClientSession | None = None, timeout: int = 30, ipaddress=None, - verify_ssl=True + verify_ssl=True, + raise_on_error: bool = False ) -> None: - """Initialize.""" + """Initialize. + + raise_on_error is opt-in and appended last so existing positional + callers are unaffected. Left False, API-level failures return None just + as they always have. Set True to have them raise AlphaESSApiError + instead, which is the only way to tell a successful write from a + rejected one: the write endpoints answer with ``data: null`` either way. + """ self.appID = appID self.appSecret = appSecret self.accesstoken = None @@ -64,6 +103,7 @@ def __init__( self.timeout = timeout self.ipaddress = ipaddress self.verify_ssl = verify_ssl + self.raise_on_error = raise_on_error async def close(self) -> None: """Close the AlphaESS API client.""" @@ -104,6 +144,28 @@ def __return_code_description(json_response) -> str: description = RETURN_CODES.get(code) or UNDOCUMENTED_RETURN_CODES.get(code) return f" ({description})" if description else "" + def __handle_failure(self, json_response, path) -> None: + """Log an API-level failure, and raise it in strict mode. + + Returns normally in the default mode so the caller can go on to return + None, preserving the pre-0.0.21 contract. + """ + expMsg = json_response.get("expMsg") + logger.error( + f"Unexpected json_response : {json_response}" + f"{self.__return_code_description(json_response)}" + f"{f' - {expMsg}' if expMsg else ''} when calling {path}") + + if self.raise_on_error: + code = json_response.get("code") + raise AlphaESSApiError( + code=code, + msg=json_response.get("msg") or json_response.get("info"), + expMsg=expMsg, + path=path, + description=RETURN_CODES.get(code) or UNDOCUMENTED_RETURN_CODES.get(code), + ) + async def getESSList(self) -> Optional(list): """According to SN to get system list data""" try: @@ -113,6 +175,9 @@ async def getESSList(self) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -126,6 +191,9 @@ async def getLastPowerData(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -141,6 +209,9 @@ async def getOneDayPowerBySn(self, sysSn, queryDate=None) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -154,6 +225,9 @@ async def getSumDataForCustomer(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -169,6 +243,9 @@ async def getOneDateEnergyBySn(self, sysSn, queryDate=None) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -182,6 +259,9 @@ async def getChargeConfigInfo(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -195,6 +275,9 @@ async def getDisChargeConfigInfo(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -208,6 +291,9 @@ async def getEvChargerConfigList(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -226,6 +312,9 @@ async def setEvChargerCurrentsBySn(self, sysSn, currentsetting) -> Optional(list return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -239,6 +328,9 @@ async def getEvChargerCurrentsBySn(self, sysSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -252,6 +344,9 @@ async def getEvChargerStatusBySn(self, sysSn, evchargerSn) -> Optional(list): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -271,6 +366,9 @@ async def remoteControlEvCharger(self, sysSn, evchargerSn, controlMode) -> Optio return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -289,6 +387,9 @@ async def bindSn(self, sysSn, code) -> Optional(dict): return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -302,6 +403,9 @@ async def getVerificationCode(self, sysSn, checkCode) -> Optional(dict): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -319,6 +423,9 @@ async def unBindSn(self, sysSn) -> Optional(dict): return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -343,6 +450,9 @@ async def updateChargeConfigInfo(self, sysSn, batHighCap, gridCharge, timeChae1, return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -367,6 +477,9 @@ async def updateDisChargeConfigInfo(self, sysSn, batUseCap, ctrDis, timeDise1, t return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -380,6 +493,9 @@ async def getTimeChargeBySn(self, sysSn) -> Optional(dict): return await self.api_get(resource) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -415,6 +531,9 @@ async def setTimeChargeBySn(self, sysSn, executeCycleType, chargeTimeList, disch return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") raise @@ -465,17 +584,21 @@ async def api_get(self, path, json=None) -> Optional(list): logger.error(f"Unexpected response received: {response.status} when calling {path}") if not self.__is_success(json_response): - logger.error( - f"Unexpected json_response : {json_response}" - f"{self.__return_code_description(json_response)} when calling {path}") + self.__handle_failure(json_response, path) return None else: if json_response["data"] is not None: return json_response["data"] else: - logger.error(f"Unexpected json_response : {json_response} when calling {path}") + # A successful response that simply carries no payload. + # Not an error, so log it at debug rather than error. + logger.debug( + f"Successful but empty json_response : {json_response} when calling {path}") return None + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(e) raise @@ -503,11 +626,12 @@ async def api_post(self, path, json) -> Optional(dict): if self.__is_success(json_response): return json_response["data"] - logger.error( - f"Unexpected json_response : {json_response}" - f"{self.__return_code_description(json_response)} when calling {path}") + self.__handle_failure(json_response, path) return None + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(e) raise @@ -583,6 +707,9 @@ async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecha return alldata + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(e) raise @@ -604,6 +731,9 @@ async def authenticate(self) -> Optional(list): success = True return success + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(e) raise @@ -626,6 +756,9 @@ async def setbatterycharge(self, serial, enabled, cp1start, cp1end, cp2start, cp logger.debug(f"Trying to call {resource} with settings {settings}") return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(e) raise @@ -648,6 +781,9 @@ async def setbatterydischarge(self, serial, enabled, dp1start, dp1end, dp2start, logger.debug(f"Trying to call {resource} with settings {settings}") return await self.api_post(resource, settings) + except AlphaESSApiError: + # Already logged by __handle_failure; don't log it a second time. + raise except Exception as e: logger.error(e) raise diff --git a/docs/API.md b/docs/API.md index c2340a6..3c27454 100644 --- a/docs/API.md +++ b/docs/API.md @@ -146,13 +146,31 @@ Interfaces 1–17 are numbered as the portal numbers them (`interface_id` 1–17 charge/discharge pair carries portal ids `110000000000000` and `110000000000001`. **The method is enforced.** Calling an endpoint with the other verb returns a plain HTTP `405` -with no `code` field. +with no `code` field. This is also a reliable way to tell an endpoint that exists from one that +does not: an unknown path returns `404`, a real path called with the wrong verb returns `405`. + +### Endpoints that exist but are not in this list + +The portal's interface registry holds **21** interfaces, but a standard developer account is +scoped to the 19 above. The registry groups interfaces into three documents — 标准文档 (standard, +"for ordinary end users"), 第三方机构专用文档 (third-party institutions), and 工商业定制文档 +(commercial & industrial, added 2024-10-29) — and two meter-offset endpoints appear **only** in +the commercial & industrial document: + +| Portal id | Endpoint | Method | Notes | +|:--|:--|:--|:--| +| 20 | `getMeterOffsetConfigInfo` | **GET** | Portal documents it as POST, which returns `405`. Its parameter table is copy-pasted from the setter and lists write fields on a read. | +| 21 | `updateMeterOffsetConfigInfo` | POST | `sysSn`, `pmOffset` (0.1 kW units, −500…500 kW, default 0), `pmOffsetEn` (1/0), `pmOffsetS1`/`E1`, `pmOffsetS2`/`E2` | + +Both are unusable from a standard account: every parameter combination tried against +`getMeterOffsetConfigInfo` returned `6001` with no `expMsg`. They are listed here for +completeness only — the library does not implement them. --- ## Corrections to the official documentation -Four points where the portal (or the bundled Postman collection) is wrong, each confirmed +Points where the portal (or the bundled Postman collection) is wrong, each confirmed against the live API: | Source says | Actual behaviour | How it was confirmed | @@ -160,7 +178,16 @@ against the live API: | `getVerificationCode` takes a JSON body ("request parameter (Json)") | **GET only**, query-string parameters | POST returns HTTP `405 Method Not Allowed` | | `bindSn` is a GET with query parameters *(Postman collection)* | **POST only**, JSON body | GET returns HTTP `405 Method Not Allowed` | | `getOneDayPowerBySn` returns `cobat` and `pChargingPile` | returns **`cbat`** and **`pchargingPile`** | live response inspection | -| 19 return codes exist | at least one more — **`6017 No operation permissions`** | returned by `getTimeChargeBySn` | +| 19 return codes exist | at least two more — **`6017`** and **`10001`** | `6017` from `getTimeChargeBySn`, `10001` from `setTimeChargeBySn` with a list omitted | +| `getMeterOffsetConfigInfo` is a POST | **GET only** | POST returns `405`, GET returns a `code` body | +| *(this document, before 0.0.21)* an empty period list is acceptable — "send `[]`, not `null`" | **wrong** — `[]` is rejected with `6001 "time list is null"` | live `setTimeChargeBySn` call | + +### The response envelope carries an undocumented `expMsg` + +Every response includes `expMsg`, which is `null` on success and on most failures, but carries a +specific reason for some parameter errors — `"time list is null"` being the one that matters for +`setTimeChargeBySn`. The generic `msg` ("Parameter error") does not say *which* parameter. Since +0.0.21 the library logs `expMsg` and exposes it on `AlphaESSApiError.expMsg`. --- @@ -680,7 +707,19 @@ default, since most systems return `6017`. - Maximum **6 groups per day**, maximum **28 groups per week**. - Charge and discharge periods **must not overlap**. -- Both lists are required even if empty — send `[]`, not `null`. +- Both lists are required and **neither may be empty**. Verified against the live API: + - `"dischargeTimeList": []` → `6001` with `expMsg: "time list is null"` — an empty list is + treated as null. + - omitting the key entirely → `10001 Parameter Error`. + - There is **no known way to express "no periods on this side"**. A `00:00`–`00:00` element + passes validation, but whether the device reads it as a zero-length window or as wrapping + midnight (i.e. all day) is unconfirmed, so it is not safe to use as a placeholder. + +> **Validation runs before the permission check.** A structurally invalid payload returns `6001` +> or `10001` even on a system that is not entitled to the endpoint at all. Only once the payload +> is valid does the `6017` entitlement check apply. Do not read an early `6001` as proof that the +> account has permission — captured live on a SMILE5-INV that returns `6017` for both the read +> and the write. **Returns:** `data` is `null`. Success is `code: 200`. diff --git a/docs/RETURN_CODES.md b/docs/RETURN_CODES.md index 123ebfd..357ab35 100644 --- a/docs/RETURN_CODES.md +++ b/docs/RETURN_CODES.md @@ -144,6 +144,22 @@ Observed in production but absent from the portal's Return Code Description page | Code | Message | Cause | |:--|:--|:--| | `6017` | `No operation permissions` | Your AppID is bound to the SN, but the account tier or the hardware is not entitled to this endpoint. Confirmed on `getTimeChargeBySn` against two bound SMILE5 systems; the same call with an unbound SN returned `6005` instead, proving the binding check passes first and the entitlement check fails second. Handle it as "feature unavailable for this system", not as an error to retry. | +| `10001` | `Parameter Error` | Structurally malformed request body — distinct from `6001`, which is a *valid* body with a bad value. Observed on `setTimeChargeBySn` when `dischargeTimeList` was omitted entirely. Note the capitalisation differs from `6001`'s `Parameter error`, and it sits outside the `6xxx` range, so a parser that assumes `6000 <= code <= 6099` will miss it. | + +### Validation order + +Parameter validation runs **before** the entitlement check. On a system that is not entitled to +an endpoint at all, a malformed request still returns `6001`/`10001` rather than `6017`; only +once the payload is structurally valid does `6017` surface. Captured on `setTimeChargeBySn` +against a SMILE5-INV: + +| Request | Response | +|:--|:--| +| `dischargeTimeList: []` | `6001` — `expMsg: "time list is null"` | +| `dischargeTimeList` omitted | `10001` | +| both lists valid | `6017` | + +Do not read an early `6001` as evidence that the account has permission. --- @@ -200,6 +216,40 @@ Two consequences worth knowing: success, and the wrappers also return `None` on failure. To confirm a write landed, either watch the log or read the value back with the corresponding `get` endpoint. +### Opting in to exceptions — `raise_on_error` (0.0.21+) + +Both consequences above go away if you construct the client with `raise_on_error=True`. API-level +failures then raise `AlphaESSApiError` carrying the code, and **success is signalled by the +absence of an exception** — which is the only way to confirm a write on endpoints that answer +`data: null` either way. + +```python +from alphaess.alphaess import alphaess, AlphaESSApiError + +client = alphaess(appID, appSecret, raise_on_error=True) + +try: + await client.setTimeChargeBySn(sysSn, 0, charge_list, discharge_list) +except AlphaESSApiError as err: + if err.code == 6017: + ... # not entitled to the periodic API — stop trying + else: + ... # err.expMsg often says which parameter was wrong +except aiohttp.ClientError: + ... # transport failure — back off and retry +``` + +| Attribute | Description | +|:--|:--| +| `code` | The `code` field, e.g. `6001` | +| `msg` | The `msg` (or `info`) field — **localised**, do not branch on it | +| `expMsg` | Exception detail, e.g. `"time list is null"`. Often the only field naming the bad parameter. | +| `description` | The English description from `RETURN_CODES` / `UNDOCUMENTED_RETURN_CODES`, if known | +| `path` | The URL that was called | + +The flag defaults to `False`, so upgrading changes nothing unless you ask for it. Transport +errors are unaffected either way — they always propagate. + ### Transport-level errors → the exception propagates Connection resets, DNS failures, timeouts and non-2xx HTTP statuses (`raise_for_status` is set, diff --git a/setup.py b/setup.py index 848d4c1..bfe24e6 100644 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ setup( name="alphaessopenapi", - version="0.0.20", + version="0.0.21", author="Charles Gillanders", author_email="charles@charlesgillanders.com", description="A python library to retrieve energy statistics from your Alpha ESS inverter by polling the Official Alpha ESS Open API.", diff --git a/tests/test_raise_on_error.py b/tests/test_raise_on_error.py new file mode 100644 index 0000000..5ea6517 --- /dev/null +++ b/tests/test_raise_on_error.py @@ -0,0 +1,186 @@ +"""Tests for opt-in strict error handling (raise_on_error / AlphaESSApiError). + +Half of this file is deliberately regression testing: the default behaviour of +0.0.21 must be indistinguishable from 0.0.20, so that anyone who upgrades +without reading the changelog sees no difference at all. + +The aiohttp session is fully mocked with unittest.mock -- no network access. +""" +import inspect +import logging +from unittest.mock import AsyncMock, MagicMock + +import pytest + +from alphaess.alphaess import ( + UNDOCUMENTED_RETURN_CODES, + AlphaESSApiError, + alphaess, +) + +from .test_time_charge import _GetContextManager, _make_response + +# A real rejection, captured from the live API: setTimeChargeBySn with an empty +# dischargeTimeList. expMsg carries the only useful detail. +EMPTY_LIST_REJECTION = { + "code": 6001, "msg": "Parameter error", + "expMsg": "time list is null", "data": None, "extra": None, +} +NOT_ENTITLED = { + "code": 6017, "msg": "No operation permissions", + "expMsg": None, "data": None, "extra": None, +} + + +def _client(get_body=None, post_body=None, **kwargs): + session = MagicMock() + if get_body is not None: + session.get = MagicMock(return_value=_GetContextManager(_make_response(get_body))) + if post_body is not None: + session.post = AsyncMock(return_value=_make_response(post_body)) + return alphaess("appid", "appsecret", session=session, **kwargs) + + +# -------------------------------------------------------------------------- +# Backwards compatibility -- the default must not change +# -------------------------------------------------------------------------- + +def test_raise_on_error_defaults_to_false(): + assert alphaess("appid", "appsecret", session=MagicMock()).raise_on_error is False + + +def test_raise_on_error_is_appended_last_in_the_signature(): + """Existing positional callers must keep working unchanged.""" + params = list(inspect.signature(alphaess.__init__).parameters) + assert params == [ + "self", "appID", "appSecret", "session", "timeout", + "ipaddress", "verify_ssl", "raise_on_error", + ] + + +async def test_default_mode_still_returns_none_on_api_error(): + client = _client(get_body=NOT_ENTITLED) + assert await client.getTimeChargeBySn("SN") is None + + +async def test_default_mode_still_returns_none_on_post_rejection(): + client = _client(post_body=EMPTY_LIST_REJECTION) + assert await client.setTimeChargeBySn("SN", 0, [], []) is None + + +async def test_default_mode_never_raises_the_new_exception(): + client = _client(get_body=EMPTY_LIST_REJECTION, post_body=EMPTY_LIST_REJECTION) + assert await client.getChargeConfigInfo("SN") is None + assert await client.updateChargeConfigInfo( + "SN", 100, 0, "00:00", "00:00", "00:00", "00:00") is None + + +async def test_success_return_value_is_unchanged_in_strict_mode(): + """Strict mode only affects failures; successes return exactly as before.""" + body = {"code": 200, "msg": "Success", "data": {"gridCharge": 1}} + assert await _client(get_body=body).getChargeConfigInfo("SN") == {"gridCharge": 1} + assert await _client(get_body=body, raise_on_error=True).getChargeConfigInfo( + "SN") == {"gridCharge": 1} + + +async def test_write_success_returns_none_in_both_modes(): + """The write endpoints answer with data: null on success. That stays true -- + strict mode signals success by NOT raising, not by a new return value.""" + body = {"code": 200, "msg": "Success", "data": None} + assert await _client(post_body=body).setTimeChargeBySn("SN", 0, [], []) is None + assert await _client(post_body=body, raise_on_error=True).setTimeChargeBySn( + "SN", 0, [], []) is None + + +# -------------------------------------------------------------------------- +# Strict mode +# -------------------------------------------------------------------------- + +async def test_strict_mode_raises_on_get_failure(): + client = _client(get_body=NOT_ENTITLED, raise_on_error=True) + with pytest.raises(AlphaESSApiError) as excinfo: + await client.getTimeChargeBySn("SN") + assert excinfo.value.code == 6017 + + +async def test_strict_mode_raises_on_post_failure(): + client = _client(post_body=EMPTY_LIST_REJECTION, raise_on_error=True) + with pytest.raises(AlphaESSApiError): + await client.setTimeChargeBySn("SN", 0, [], []) + + +async def test_exception_carries_the_diagnostic_fields(): + client = _client(post_body=EMPTY_LIST_REJECTION, raise_on_error=True) + with pytest.raises(AlphaESSApiError) as excinfo: + await client.setTimeChargeBySn("SN", 0, [], []) + + err = excinfo.value + assert err.code == 6001 + assert err.msg == "Parameter error" + assert err.expMsg == "time list is null" + assert err.description == "Parameter error" + assert err.path.endswith("/setTimeChargeBySn") + # expMsg is the only thing that says WHICH parameter was wrong. + assert "time list is null" in str(err) + + +async def test_undocumented_code_is_described_on_the_exception(): + client = _client(get_body=NOT_ENTITLED, raise_on_error=True) + with pytest.raises(AlphaESSApiError) as excinfo: + await client.getTimeChargeBySn("SN") + assert excinfo.value.description == UNDOCUMENTED_RETURN_CODES[6017] + + +async def test_10001_is_recognised(): + """Returned when a required list is omitted entirely rather than empty.""" + client = _client(post_body={"code": 10001, "msg": "Parameter Error", "data": None}, + raise_on_error=True) + with pytest.raises(AlphaESSApiError) as excinfo: + await client.setTimeChargeBySn("SN", 0, [], []) + assert excinfo.value.description == UNDOCUMENTED_RETURN_CODES[10001] + + +async def test_strict_mode_leaves_transport_errors_alone(): + """A transport failure must stay distinguishable from an API rejection.""" + import aiohttp + session = MagicMock() + session.get = MagicMock(side_effect=aiohttp.ClientConnectionError("boom")) + client = alphaess("appid", "appsecret", session=session, raise_on_error=True) + + with pytest.raises(aiohttp.ClientConnectionError): + await client.getLastPowerData("SN") + + +async def test_failure_is_logged_once_not_once_per_wrapper(caplog): + """__handle_failure logs; the wrappers must not log the same thing again.""" + client = _client(get_body=NOT_ENTITLED, raise_on_error=True) + + with caplog.at_level(logging.ERROR, logger="alphaess.alphaess"): + with pytest.raises(AlphaESSApiError): + await client.getTimeChargeBySn("SN") + + assert len([r for r in caplog.records if r.levelno >= logging.ERROR]) == 1 + + +# -------------------------------------------------------------------------- +# Logging improvements, which apply in both modes +# -------------------------------------------------------------------------- + +async def test_expmsg_is_included_in_the_error_log(caplog): + client = _client(post_body=EMPTY_LIST_REJECTION) + + with caplog.at_level(logging.ERROR, logger="alphaess.alphaess"): + await client.setTimeChargeBySn("SN", 0, [], []) + + assert "time list is null" in caplog.text + + +async def test_successful_but_empty_response_is_not_logged_as_an_error(caplog): + """getVerificationCode and the write endpoints legitimately return no data.""" + client = _client(get_body={"code": 200, "msg": "Success", "data": None}) + + with caplog.at_level(logging.DEBUG, logger="alphaess.alphaess"): + result = await client.getVerificationCode("SN", "CHECK") + + assert result is None + assert not [r for r in caplog.records if r.levelno >= logging.ERROR] From 466f04064f0ab5cb2dda69700e3eef7e56f63692 Mon Sep 17 00:00:00 2001 From: Joshua Leaper Date: Thu, 13 Aug 2026 15:55:14 +0930 Subject: [PATCH 2/3] Drop the repeated comment and reword the docs The "already logged by __handle_failure" note was on all 25 passthroughs and said the same thing every time, which is noise once you've read it once. Also reworded the new doc sections so they read like someone explaining what they found rather than a spec, and cut the paragraph restating what the table above it already said. --- README.md | 17 +++++++------- alphaess/alphaess.py | 55 +++++++++++-------------------------------- docs/API.md | 56 +++++++++++++++++++++----------------------- docs/RETURN_CODES.md | 31 ++++++++++++------------ 4 files changed, 64 insertions(+), 95 deletions(-) diff --git a/README.md b/README.md index 7c0d515..c854bce 100644 --- a/README.md +++ b/README.md @@ -28,12 +28,12 @@ Things the official documentation gets wrong are corrected in ## Error handling -By default an API-level failure (a non-`200` `code` in the response) is logged and the method -returns `None`, while transport failures — connection resets, timeouts, non-2xx HTTP — raise. +By default, if the API answers with a non-`200` `code` the method logs it and returns `None`. +Transport failures — connection resets, timeouts, non-2xx HTTP — raise instead. -That default makes a successful write indistinguishable from a rejected one, because the write -endpoints answer with `data: null` either way. If you need to tell them apart, construct the -client with `raise_on_error=True`: +The catch is that this makes a successful write look identical to a rejected one, since the write +endpoints answer with `data: null` either way. If you need to tell them apart, build the client +with `raise_on_error=True`: ```python from alphaess.alphaess import alphaess, AlphaESSApiError @@ -46,10 +46,9 @@ except AlphaESSApiError as err: print(err.code, err.expMsg) # e.g. 6001 "time list is null" ``` -Success is then signalled by the absence of an exception; return values are unchanged. The flag -defaults to `False`, so upgrading does not alter existing behaviour. See -[docs/RETURN_CODES.md](docs/RETURN_CODES.md#opting-in-to-exceptions--raise_on_error-0021) for the -full exception reference. +Success then just means nothing was raised — return values don't change. It's off by default, so +upgrading won't alter how your existing code behaves. Full reference in +[docs/RETURN_CODES.md](docs/RETURN_CODES.md#opting-in-to-exceptions--raise_on_error-0021). # Methods diff --git a/alphaess/alphaess.py b/alphaess/alphaess.py index 4ccea78..0714cbd 100644 --- a/alphaess/alphaess.py +++ b/alphaess/alphaess.py @@ -42,14 +42,13 @@ class AlphaESSApiError(Exception): - """The API answered, but reported a failure. + """The API answered, and said no. - Only raised when the client was created with ``raise_on_error=True``. By - default an API-level failure still returns ``None``, exactly as it did in - 0.0.20 and earlier. + Only raised if you built the client with ``raise_on_error=True``. Otherwise + an API-level failure still returns ``None``, same as it always has. - Distinct from the aiohttp transport exceptions: this means the service was - reached and rejected the request, so retrying unchanged will not help. + Not the same thing as the aiohttp transport errors: we reached the service + and it rejected the request, so sending it again unchanged won't help. """ def __init__(self, code, msg=None, expMsg=None, path=None, description=None): @@ -86,11 +85,11 @@ def __init__( ) -> None: """Initialize. - raise_on_error is opt-in and appended last so existing positional - callers are unaffected. Left False, API-level failures return None just - as they always have. Set True to have them raise AlphaESSApiError - instead, which is the only way to tell a successful write from a - rejected one: the write endpoints answer with ``data: null`` either way. + raise_on_error goes last in the signature so existing positional callers + keep working. Leave it False and API-level failures return None like they + always have. Turn it on and they raise AlphaESSApiError instead, which is + the only way to tell a successful write from a rejected one — the write + endpoints answer with ``data: null`` whichever way it went. """ self.appID = appID self.appSecret = appSecret @@ -145,10 +144,10 @@ def __return_code_description(json_response) -> str: return f" ({description})" if description else "" def __handle_failure(self, json_response, path) -> None: - """Log an API-level failure, and raise it in strict mode. + """Log an API-level failure, and raise it if raise_on_error is set. - Returns normally in the default mode so the caller can go on to return - None, preserving the pre-0.0.21 contract. + Returns normally otherwise, so the caller goes on to return None and + nothing changes for anyone who hasn't opted in. """ expMsg = json_response.get("expMsg") logger.error( @@ -176,7 +175,6 @@ async def getESSList(self) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -192,7 +190,6 @@ async def getLastPowerData(self, sysSn) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -210,7 +207,6 @@ async def getOneDayPowerBySn(self, sysSn, queryDate=None) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -226,7 +222,6 @@ async def getSumDataForCustomer(self, sysSn) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -244,7 +239,6 @@ async def getOneDateEnergyBySn(self, sysSn, queryDate=None) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -260,7 +254,6 @@ async def getChargeConfigInfo(self, sysSn) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -276,7 +269,6 @@ async def getDisChargeConfigInfo(self, sysSn) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -292,7 +284,6 @@ async def getEvChargerConfigList(self, sysSn) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -313,7 +304,6 @@ async def setEvChargerCurrentsBySn(self, sysSn, currentsetting) -> Optional(list return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -329,7 +319,6 @@ async def getEvChargerCurrentsBySn(self, sysSn) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -345,7 +334,6 @@ async def getEvChargerStatusBySn(self, sysSn, evchargerSn) -> Optional(list): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -367,7 +355,6 @@ async def remoteControlEvCharger(self, sysSn, evchargerSn, controlMode) -> Optio return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -388,7 +375,6 @@ async def bindSn(self, sysSn, code) -> Optional(dict): return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -404,7 +390,6 @@ async def getVerificationCode(self, sysSn, checkCode) -> Optional(dict): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -424,7 +409,6 @@ async def unBindSn(self, sysSn) -> Optional(dict): return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -451,7 +435,6 @@ async def updateChargeConfigInfo(self, sysSn, batHighCap, gridCharge, timeChae1, return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -478,7 +461,6 @@ async def updateDisChargeConfigInfo(self, sysSn, batUseCap, ctrDis, timeDise1, t return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -494,7 +476,6 @@ async def getTimeChargeBySn(self, sysSn) -> Optional(dict): return await self.api_get(resource) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -532,7 +513,6 @@ async def setTimeChargeBySn(self, sysSn, executeCycleType, chargeTimeList, disch return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(f"Error: {e} when calling {resource}") @@ -590,14 +570,12 @@ async def api_get(self, path, json=None) -> Optional(list): if json_response["data"] is not None: return json_response["data"] else: - # A successful response that simply carries no payload. - # Not an error, so log it at debug rather than error. + # Succeeded, just had nothing to give us. Not an error. logger.debug( f"Successful but empty json_response : {json_response} when calling {path}") return None except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(e) @@ -630,7 +608,6 @@ async def api_post(self, path, json) -> Optional(dict): return None except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(e) @@ -708,7 +685,6 @@ async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecha return alldata except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(e) @@ -732,7 +708,6 @@ async def authenticate(self) -> Optional(list): return success except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(e) @@ -757,7 +732,6 @@ async def setbatterycharge(self, serial, enabled, cp1start, cp1end, cp2start, cp return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(e) @@ -782,7 +756,6 @@ async def setbatterydischarge(self, serial, enabled, dp1start, dp1end, dp2start, return await self.api_post(resource, settings) except AlphaESSApiError: - # Already logged by __handle_failure; don't log it a second time. raise except Exception as e: logger.error(e) diff --git a/docs/API.md b/docs/API.md index 3c27454..cf66c89 100644 --- a/docs/API.md +++ b/docs/API.md @@ -149,22 +149,20 @@ charge/discharge pair carries portal ids `110000000000000` and `110000000000001` with no `code` field. This is also a reliable way to tell an endpoint that exists from one that does not: an unknown path returns `404`, a real path called with the wrong verb returns `405`. -### Endpoints that exist but are not in this list +### Two endpoints you can't reach -The portal's interface registry holds **21** interfaces, but a standard developer account is -scoped to the 19 above. The registry groups interfaces into three documents — 标准文档 (standard, -"for ordinary end users"), 第三方机构专用文档 (third-party institutions), and 工商业定制文档 -(commercial & industrial, added 2024-10-29) — and two meter-offset endpoints appear **only** in -the commercial & industrial document: +There are 21 endpoints in the portal, not 19. The other two are for meter offset, and they only +show up in the commercial & industrial document — the portal splits its endpoints across three +documents (standard, third-party institutions, and C&I) and a normal developer account only gets +the standard one. -| Portal id | Endpoint | Method | Notes | +| Portal id | Endpoint | Method | Parameters | |:--|:--|:--|:--| -| 20 | `getMeterOffsetConfigInfo` | **GET** | Portal documents it as POST, which returns `405`. Its parameter table is copy-pasted from the setter and lists write fields on a read. | +| 20 | `getMeterOffsetConfigInfo` | **GET** — the portal says POST, which just `405`s | `sysSn` | | 21 | `updateMeterOffsetConfigInfo` | POST | `sysSn`, `pmOffset` (0.1 kW units, −500…500 kW, default 0), `pmOffsetEn` (1/0), `pmOffsetS1`/`E1`, `pmOffsetS2`/`E2` | -Both are unusable from a standard account: every parameter combination tried against -`getMeterOffsetConfigInfo` returned `6001` with no `expMsg`. They are listed here for -completeness only — the library does not implement them. +They're here so nobody else has to go digging through the portal to work out what the missing two +are. The library doesn't implement them. --- @@ -182,12 +180,12 @@ against the live API: | `getMeterOffsetConfigInfo` is a POST | **GET only** | POST returns `405`, GET returns a `code` body | | *(this document, before 0.0.21)* an empty period list is acceptable — "send `[]`, not `null`" | **wrong** — `[]` is rejected with `6001 "time list is null"` | live `setTimeChargeBySn` call | -### The response envelope carries an undocumented `expMsg` +### `expMsg` is worth reading -Every response includes `expMsg`, which is `null` on success and on most failures, but carries a -specific reason for some parameter errors — `"time list is null"` being the one that matters for -`setTimeChargeBySn`. The generic `msg` ("Parameter error") does not say *which* parameter. Since -0.0.21 the library logs `expMsg` and exposes it on `AlphaESSApiError.expMsg`. +Every response carries an `expMsg` field. It's `null` most of the time, but on some parameter +errors it's the only thing that tells you what you actually got wrong — `msg` just says +"Parameter error" without naming the parameter, whereas `expMsg` says `"time list is null"`. +Since 0.0.21 it gets logged, and it's on `AlphaESSApiError.expMsg`. --- @@ -707,19 +705,19 @@ default, since most systems return `6017`. - Maximum **6 groups per day**, maximum **28 groups per week**. - Charge and discharge periods **must not overlap**. -- Both lists are required and **neither may be empty**. Verified against the live API: - - `"dischargeTimeList": []` → `6001` with `expMsg: "time list is null"` — an empty list is - treated as null. - - omitting the key entirely → `10001 Parameter Error`. - - There is **no known way to express "no periods on this side"**. A `00:00`–`00:00` element - passes validation, but whether the device reads it as a zero-length window or as wrapping - midnight (i.e. all day) is unconfirmed, so it is not safe to use as a placeholder. - -> **Validation runs before the permission check.** A structurally invalid payload returns `6001` -> or `10001` even on a system that is not entitled to the endpoint at all. Only once the payload -> is valid does the `6017` entitlement check apply. Do not read an early `6001` as proof that the -> account has permission — captured live on a SMILE5-INV that returns `6017` for both the read -> and the write. +- **Both lists need at least one period in them.** Sending `[]` gets you `6001` with + `expMsg: "time list is null"` — an empty list counts as null. Leaving the key out altogether + gets you `10001` instead. + +That last one is awkward if you only care about charging: there's no obvious way to say "I have +no discharge periods". A `00:00`–`00:00` element does get past validation, but we don't know +whether the device treats that as a zero-length window or as one that wraps midnight and runs all +day, so it isn't safe to use as a filler. + +> **Validation happens before the permission check.** Send a malformed payload to a system that +> isn't entitled to this endpoint and you'll still get `6001` or `10001` — `6017` only appears +> once the payload is valid. So an early `6001` tells you nothing about whether your account has +> access. Captured on a SMILE5-INV that returns `6017` for both the read and the write. **Returns:** `data` is `null`. Success is `code: 200`. diff --git a/docs/RETURN_CODES.md b/docs/RETURN_CODES.md index 357ab35..3b47f8b 100644 --- a/docs/RETURN_CODES.md +++ b/docs/RETURN_CODES.md @@ -144,14 +144,13 @@ Observed in production but absent from the portal's Return Code Description page | Code | Message | Cause | |:--|:--|:--| | `6017` | `No operation permissions` | Your AppID is bound to the SN, but the account tier or the hardware is not entitled to this endpoint. Confirmed on `getTimeChargeBySn` against two bound SMILE5 systems; the same call with an unbound SN returned `6005` instead, proving the binding check passes first and the entitlement check fails second. Handle it as "feature unavailable for this system", not as an error to retry. | -| `10001` | `Parameter Error` | Structurally malformed request body — distinct from `6001`, which is a *valid* body with a bad value. Observed on `setTimeChargeBySn` when `dischargeTimeList` was omitted entirely. Note the capitalisation differs from `6001`'s `Parameter error`, and it sits outside the `6xxx` range, so a parser that assumes `6000 <= code <= 6099` will miss it. | +| `10001` | `Parameter Error` | The request body was malformed, as opposed to `6001` which is a valid body with a bad value in it. Seen on `setTimeChargeBySn` when `dischargeTimeList` was left out entirely. Two things to watch: the capitalisation differs from `6001`'s `Parameter error`, and it's outside the `6xxx` range, so anything assuming `6000 <= code <= 6099` will miss it. | -### Validation order +### Errors come back in a particular order -Parameter validation runs **before** the entitlement check. On a system that is not entitled to -an endpoint at all, a malformed request still returns `6001`/`10001` rather than `6017`; only -once the payload is structurally valid does `6017` surface. Captured on `setTimeChargeBySn` -against a SMILE5-INV: +Parameters get validated before permissions. If your payload is malformed you'll get `6001` or +`10001` even on a system that has no access to the endpoint at all — `6017` only shows up once +the payload is valid. Here's the same endpoint on the same SMILE5-INV, three times: | Request | Response | |:--|:--| @@ -159,7 +158,8 @@ against a SMILE5-INV: | `dischargeTimeList` omitted | `10001` | | both lists valid | `6017` | -Do not read an early `6001` as evidence that the account has permission. +So getting a `6001` back doesn't mean your account has permission — it just means you never got +far enough to find out. --- @@ -218,10 +218,9 @@ Two consequences worth knowing: ### Opting in to exceptions — `raise_on_error` (0.0.21+) -Both consequences above go away if you construct the client with `raise_on_error=True`. API-level -failures then raise `AlphaESSApiError` carrying the code, and **success is signalled by the -absence of an exception** — which is the only way to confirm a write on endpoints that answer -`data: null` either way. +Both of those go away if you build the client with `raise_on_error=True`. API-level failures then +raise `AlphaESSApiError` with the code attached, and success just means nothing was raised — +which is the only way to confirm a write on the endpoints that return `data: null` either way. ```python from alphaess.alphaess import alphaess, AlphaESSApiError @@ -242,13 +241,13 @@ except aiohttp.ClientError: | Attribute | Description | |:--|:--| | `code` | The `code` field, e.g. `6001` | -| `msg` | The `msg` (or `info`) field — **localised**, do not branch on it | -| `expMsg` | Exception detail, e.g. `"time list is null"`. Often the only field naming the bad parameter. | -| `description` | The English description from `RETURN_CODES` / `UNDOCUMENTED_RETURN_CODES`, if known | +| `msg` | The `msg` (or `info`) field. Localised, so don't branch on it | +| `expMsg` | Exception detail, e.g. `"time list is null"` — often the only thing naming the bad parameter | +| `description` | The English description from `RETURN_CODES` / `UNDOCUMENTED_RETURN_CODES`, if we know the code | | `path` | The URL that was called | -The flag defaults to `False`, so upgrading changes nothing unless you ask for it. Transport -errors are unaffected either way — they always propagate. +The flag is off by default, so upgrading won't change anything unless you ask it to. Transport +errors behave the same either way — they always propagate. ### Transport-level errors → the exception propagates From bcba215a699e9de7e64b47466adf84f244d680c5 Mon Sep 17 00:00:00 2001 From: Joshua Leaper Date: Thu, 13 Aug 2026 16:18:23 +0930 Subject: [PATCH 3/3] Keep getdata best-effort when raise_on_error is on getdata is documented as returning whatever it managed to gather, but with the flag set the first endpoint the account can't use would raise and take the whole poll with it. getTimeChargeBySn answers 6017 on most systems, so that was not a corner case. Each call inside getdata now goes through _best_effort, which leaves that one key None on a refusal. Transport errors still stop it, since those affect every endpoint anyway. --- alphaess/alphaess.py | 33 ++++++++++---- docs/RETURN_CODES.md | 4 ++ tests/test_getdata_best_effort.py | 74 +++++++++++++++++++++++++++++++ 3 files changed, 102 insertions(+), 9 deletions(-) create mode 100644 tests/test_getdata_best_effort.py diff --git a/alphaess/alphaess.py b/alphaess/alphaess.py index 0714cbd..6ee9917 100644 --- a/alphaess/alphaess.py +++ b/alphaess/alphaess.py @@ -613,6 +613,21 @@ async def api_post(self, path, json) -> Optional(dict): logger.error(e) raise + async def _best_effort(self, method, *args): + """Call an endpoint for getdata, tolerating a refusal. + + getdata is best-effort by contract: it returns whatever it could + gather. With raise_on_error set that would otherwise stop dead on the + first endpoint this account cannot use, losing the rest of the data + too, so an API-level refusal leaves that one key empty instead. + Transport errors still propagate -- those affect every endpoint. + """ + try: + return await method(*args) + except AlphaESSApiError as err: + logger.debug(f"{getattr(method, '__name__', method)} refused: {err}") + return None + async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecharge=False) -> Optional(list): """Get All Data For All serial numbers from Alpha ESS""" try: @@ -631,7 +646,7 @@ async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecha } } - units = await self.getESSList() + units = await self._best_effort(self.getESSList) if units is None: logger.warning( "getESSList returned no data (Alpha ESS API busy or unavailable); " @@ -641,32 +656,32 @@ async def getdata(self, get_power=False, get_ev=False, self_delay=0, get_timecha for idx, unit in enumerate(units): if "sysSn" in unit: serial = unit["sysSn"] - unit['SumData'] = await self.getSumDataForCustomer(serial) + unit['SumData'] = await self._best_effort(self.getSumDataForCustomer, serial) await asyncio.sleep(self_delay) - unit['OneDateEnergy'] = await self.getOneDateEnergyBySn(serial, time.strftime("%Y-%m-%d")) + unit['OneDateEnergy'] = await self._best_effort(self.getOneDateEnergyBySn, serial, time.strftime("%Y-%m-%d")) await asyncio.sleep(self_delay) - unit['LastPower'] = await self.getLastPowerData(serial) + unit['LastPower'] = await self._best_effort(self.getLastPowerData, serial) await asyncio.sleep(self_delay) - unit['ChargeConfig'] = await self.getChargeConfigInfo(serial) + unit['ChargeConfig'] = await self._best_effort(self.getChargeConfigInfo, serial) await asyncio.sleep(self_delay) - unit['DisChargeConfig'] = await self.getDisChargeConfigInfo(serial) + unit['DisChargeConfig'] = await self._best_effort(self.getDisChargeConfigInfo, serial) await asyncio.sleep(self_delay) if get_power: await asyncio.sleep(self_delay) - unit['OneDayPower'] = await self.getOneDayPowerBySn(serial, time.strftime("%Y-%m-%d")) + unit['OneDayPower'] = await self._best_effort(self.getOneDayPowerBySn, serial, time.strftime("%Y-%m-%d")) if get_timecharge: await asyncio.sleep(self_delay) - unit['TimeCharge'] = await self.getTimeChargeBySn(serial) + unit['TimeCharge'] = await self._best_effort(self.getTimeChargeBySn, serial) if get_ev: await asyncio.sleep(self_delay) - unit['EVData'] = await self.getEvChargerConfigList(serial) + unit['EVData'] = await self._best_effort(self.getEvChargerConfigList, serial) await asyncio.sleep(self_delay) try: ev_serial = unit['EVData'][0].get('evchargerSn', None) diff --git a/docs/RETURN_CODES.md b/docs/RETURN_CODES.md index 3b47f8b..9f625fa 100644 --- a/docs/RETURN_CODES.md +++ b/docs/RETURN_CODES.md @@ -249,6 +249,10 @@ except aiohttp.ClientError: The flag is off by default, so upgrading won't change anything unless you ask it to. Transport errors behave the same either way — they always propagate. +`getdata()` is the one exception, and deliberately so. It stays best-effort even with the flag +on: an endpoint your account can't use leaves that one key `None` instead of costing you the +whole poll. Transport errors still stop it, since those affect every endpoint anyway. + ### Transport-level errors → the exception propagates Connection resets, DNS failures, timeouts and non-2xx HTTP statuses (`raise_for_status` is set, diff --git a/tests/test_getdata_best_effort.py b/tests/test_getdata_best_effort.py new file mode 100644 index 0000000..69ea0d7 --- /dev/null +++ b/tests/test_getdata_best_effort.py @@ -0,0 +1,74 @@ +"""getdata keeps its best-effort contract when raise_on_error is set. + +getdata is documented as returning whatever it managed to gather. Strict mode +must not turn one refused endpoint into a total loss of the poll. +""" +from unittest.mock import AsyncMock, MagicMock + +import aiohttp +import pytest + +from alphaess.alphaess import AlphaESSApiError, alphaess + + +def _client(): + client = alphaess("appid", "appsecret", session=MagicMock(), raise_on_error=True) + client.getESSList = AsyncMock(return_value=[{"sysSn": "SN"}]) + for name in ("getSumDataForCustomer", "getOneDateEnergyBySn", "getLastPowerData", + "getChargeConfigInfo", "getDisChargeConfigInfo", "getOneDayPowerBySn", + "getTimeChargeBySn", "getEvChargerConfigList"): + setattr(client, name, AsyncMock(return_value={"ok": True})) + return client + + +async def test_one_refused_endpoint_does_not_lose_the_rest(): + client = _client() + client.getChargeConfigInfo = AsyncMock( + side_effect=AlphaESSApiError(code=6017, description="No operation permissions")) + + result = await client.getdata() + + assert len(result) == 1 + assert result[0]["ChargeConfig"] is None + assert result[0]["SumData"] == {"ok": True} + assert result[0]["DisChargeConfig"] == {"ok": True} + + +async def test_refused_timecharge_still_returns_the_unit(): + """getTimeChargeBySn answers 6017 on most systems -- the common case.""" + client = _client() + client.getTimeChargeBySn = AsyncMock(side_effect=AlphaESSApiError(code=6017)) + + result = await client.getdata(get_timecharge=True) + + assert result[0]["TimeCharge"] is None + assert result[0]["LastPower"] == {"ok": True} + + +async def test_refused_esslist_returns_empty_not_an_exception(): + client = _client() + client.getESSList = AsyncMock(side_effect=AlphaESSApiError(code=6042)) + + assert await client.getdata() == [] + + +async def test_transport_errors_still_propagate(): + """A connection failure affects every endpoint -- don't paper over it.""" + client = _client() + client.getChargeConfigInfo = AsyncMock(side_effect=aiohttp.ClientConnectionError("boom")) + + with pytest.raises(aiohttp.ClientConnectionError): + await client.getdata() + + +async def test_default_mode_is_unaffected(): + """Without raise_on_error nothing raises in the first place.""" + client = alphaess("appid", "appsecret", session=MagicMock()) + client.getESSList = AsyncMock(return_value=[{"sysSn": "SN"}]) + for name in ("getSumDataForCustomer", "getOneDateEnergyBySn", "getLastPowerData", + "getChargeConfigInfo", "getDisChargeConfigInfo"): + setattr(client, name, AsyncMock(return_value=None)) + + result = await client.getdata() + + assert result[0]["ChargeConfig"] is None