Skip to content

Commit 66614e0

Browse files
proscar87claude
andauthored
Raise GrowattRateLimitError when a response body carries Growatt's 507 code (#160)
* Raise a typed error when Growatt refuses a login as rate limited Reworked after @johanzander pointed out the premise was wrong: Growatt does not signal 507 as an HTTP status. The request succeeds with HTTP 200 and the body carries `success: false` with `msg: "507"`, which is the only shape that can produce the `ConfigEntryError: Growatt login failed: 507` traceback in home-assistant/core#176831 and #174789. login() confirms this by design -- it goes straight to response.json()["back"] without ever reading response.status_code, so a transport-level check could not have fired. The previous transport-level handling and its Retry-After parsing are dropped entirely rather than kept "just in case": there is no evidence Growatt ever sends that shape, and speculative handling would just be untested code that looks like coverage. login() now raises GrowattRateLimitError(error_code="507") on that body, following the GrowattV1ApiError shape so consumers read the code off the exception instead of parsing a message. Every other failure keeps returning the dict unchanged. Three of the five tests fail against unmodified login(); all pass with it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Detect the 507 in the session hook so data calls are covered too Growatt rate limits each endpoint separately, so guarding login() alone misses refusals on data calls. The response hook already sees every response for the session, which makes it one place to extend instead of ~40 endpoint methods that each parse their own shape. The hook runs on everything, so it falls through untouched on shapes it does not recognise: plant_list receives `back` as a list, and non-JSON bodies are possible. Only the exact `success: false` + `msg: "507"` combination raises. Also drops the observed-lockout prose from the exception message, since that is behaviour that can change and it ended up in every log line. It lives in the docstring now, and the test asserts on error_code rather than message wording. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> --------- Co-authored-by: proscar87 <proscar87@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 6469d88 commit 66614e0

4 files changed

Lines changed: 226 additions & 1 deletion

File tree

‎growattServer/__init__.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
from .exceptions import (
88
GrowattError,
99
GrowattParameterError,
10+
GrowattRateLimitError,
1011
GrowattV1ApiError,
1112
GrowattV1ApiErrorCode,
1213
)
@@ -20,6 +21,7 @@
2021
"GrowattApi",
2122
"GrowattError",
2223
"GrowattParameterError",
24+
"GrowattRateLimitError",
2325
"GrowattV1ApiError",
2426
"GrowattV1ApiErrorCode",
2527
"OpenApiV1",

‎growattServer/base_api.py‎

Lines changed: 38 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,14 +15,50 @@
1515

1616
import requests
1717

18-
from .exceptions import GrowattError
18+
from .exceptions import GrowattError, GrowattRateLimitError
1919

2020
name = "growattServer"
2121

2222
BATT_MODE_LOAD_FIRST = 0
2323
BATT_MODE_BATTERY_FIRST = 1
2424
BATT_MODE_GRID_FIRST = 2
2525

26+
# Growatt reports rate limiting in the response body, not as an HTTP status:
27+
# HTTP 200 with `success: false` and `msg: "507"`. See GrowattRateLimitError
28+
# for the reports behind this.
29+
RATE_LIMITED_CODE = "507"
30+
31+
32+
def _raise_if_rate_limited(response: requests.Response) -> None:
33+
"""
34+
Raise GrowattRateLimitError when a response body carries Growatt's 507 code.
35+
36+
Growatt answers HTTP 200 and puts the refusal in the payload, so the body
37+
of every response has to be read. Growatt rate limits each endpoint
38+
separately, so this lives in the session hook rather than in `login()`:
39+
the reports we have are all logins, but a data call can be refused the
40+
same way.
41+
42+
Body shapes vary -- most endpoints wrap their payload in `back`, but
43+
`plant_list` returns `back` as a list, and some responses are not JSON at
44+
all. Since this runs on every response, anything unexpected falls through
45+
untouched instead of raising.
46+
"""
47+
try:
48+
data = response.json()
49+
except ValueError:
50+
return
51+
52+
if not isinstance(data, dict):
53+
return
54+
55+
back = data.get("back")
56+
if isinstance(back, dict):
57+
data = back
58+
59+
if not data.get("success", True) and str(data.get("msg", "")) == RATE_LIMITED_CODE:
60+
raise GrowattRateLimitError(error_code=RATE_LIMITED_CODE)
61+
2662

2763
def hash_password(password: str) -> str:
2864
"""
@@ -74,6 +110,7 @@ def __init__(self, add_random_user_id: bool = False, agent_identifier: str | Non
74110
def _raise_for_status(response, *args: object, **kwargs: object) -> None:
75111
_ = args
76112
_ = kwargs
113+
_raise_if_rate_limited(response)
77114
response.raise_for_status()
78115

79116
self.session.hooks = {"response": [_raise_for_status]}

‎growattServer/exceptions.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,3 +66,45 @@ def __init__(self, message: str, error_code: int, error_msg: str) -> None:
6666
super().__init__(f"{message}: [{error_code}] {error_msg}")
6767
self.error_code = error_code
6868
self.error_msg = error_msg
69+
70+
71+
class GrowattRateLimitError(GrowattError):
72+
"""
73+
Raised when Growatt refuses a request because the account is rate limited.
74+
75+
Growatt signals this **in the response body**, not as an HTTP status: the
76+
request succeeds with HTTP 200 and the payload carries ``success: false``
77+
with ``msg: "507"``. That is the shape behind the
78+
``ConfigEntryError: Growatt login failed: 507`` tracebacks in
79+
home-assistant/core#176831 and home-assistant/core#174789.
80+
81+
Every reported case so far is a login, but Growatt rate limits each
82+
endpoint separately, so detection sits in the session response hook and
83+
any call can raise this.
84+
85+
A 507 has been observed to precede an approximately 24 hour lockout rather
86+
than a short cooldown, so this library never retries on its own: retrying
87+
immediately is what deepens the lockout. Callers get a distinct exception
88+
type so they can back off for hours instead of treating it like bad
89+
credentials or a malformed response.
90+
91+
Follows the :class:`GrowattV1ApiError` shape so consumers can read the code
92+
off the exception rather than parsing a message.
93+
"""
94+
95+
def __init__(self, error_code: str, error_msg: str | None = None) -> None:
96+
"""
97+
Initialize the GrowattRateLimitError.
98+
99+
Args:
100+
error_code: The application-level code from the response body,
101+
e.g. ``"507"``.
102+
error_msg: Optional human-readable detail, when the body carries one.
103+
104+
"""
105+
message = f"Growatt rate limit reached: [{error_code}]"
106+
if error_msg:
107+
message += f" {error_msg}"
108+
super().__init__(message)
109+
self.error_code = error_code
110+
self.error_msg = error_msg

‎tests/test_login_rate_limit.py‎

Lines changed: 144 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,144 @@
1+
"""Growatt reports rate limiting in the body, not as an HTTP status."""
2+
import json
3+
4+
import pytest
5+
import requests
6+
7+
from growattServer import GrowattApi, GrowattRateLimitError
8+
9+
10+
def _response(payload=None, *, body=None, status=200):
11+
"""Build a real Response -- the check parses the body itself."""
12+
response = requests.Response()
13+
response.status_code = status
14+
if body is None:
15+
body = json.dumps(payload)
16+
response._content = body.encode()
17+
response.headers["Content-Type"] = "application/json"
18+
return response
19+
20+
21+
class _HookedSession:
22+
"""A session stand-in that runs the response hooks, as requests does.
23+
24+
The 507 check lives in the hook now, so a plain MagicMock session would
25+
skip it and every test here would pass for the wrong reason.
26+
"""
27+
28+
def __init__(self, hooks, response):
29+
self.hooks = hooks
30+
self.headers = {}
31+
self._response = response
32+
33+
def _dispatch(self, *args, **kwargs):
34+
for hook in self.hooks["response"]:
35+
hook(self._response)
36+
return self._response
37+
38+
get = _dispatch
39+
post = _dispatch
40+
41+
42+
def _api_returning(response):
43+
"""An API whose every call answers with `response`, hooks included."""
44+
api = GrowattApi()
45+
api.session = _HookedSession(api.session.hooks, response)
46+
return api
47+
48+
49+
def _api_with_login_body(body):
50+
"""An API whose login answers with the given `back` payload."""
51+
return _api_returning(_response({"back": body}))
52+
53+
54+
def test_login_raises_on_507_in_the_body():
55+
"""`success: false` + `msg: "507"` is the real lockout signal."""
56+
api = _api_with_login_body({"success": False, "msg": "507"})
57+
58+
with pytest.raises(GrowattRateLimitError) as exc:
59+
api.login("user", "pass")
60+
61+
assert exc.value.error_code == "507"
62+
63+
64+
def test_the_code_is_readable_off_the_exception():
65+
"""Callers branch on `error_code`, never on the message wording."""
66+
api = _api_with_login_body({"success": False, "msg": "507"})
67+
68+
with pytest.raises(GrowattRateLimitError) as exc:
69+
api.login("user", "pass")
70+
71+
assert exc.value.error_code == "507"
72+
assert str(exc.value) == "Growatt rate limit reached: [507]"
73+
74+
75+
def test_a_data_call_is_covered_too():
76+
"""Growatt rate limits per endpoint, so this cannot be login-only."""
77+
api = _api_returning(_response({"back": {"success": False, "msg": "507"}}))
78+
79+
with pytest.raises(GrowattRateLimitError):
80+
api.plant_list("user_1")
81+
82+
83+
def test_other_failures_are_left_alone():
84+
"""A wrong password must keep returning the dict, not raise."""
85+
api = _api_with_login_body({"success": False, "msg": "501"})
86+
87+
result = api.login("user", "pass")
88+
89+
assert result["success"] is False
90+
assert result["msg"] == "501"
91+
92+
93+
def test_numeric_msg_is_still_recognised():
94+
"""Defensive: the code is compared as a string."""
95+
api = _api_with_login_body({"success": False, "msg": 507})
96+
97+
with pytest.raises(GrowattRateLimitError):
98+
api.login("user", "pass")
99+
100+
101+
def test_successful_login_is_unaffected():
102+
"""The happy path keeps its existing shape."""
103+
api = _api_with_login_body({
104+
"success": True,
105+
"user": {"id": "u1", "rightlevel": 1},
106+
})
107+
108+
result = api.login("user", "pass")
109+
110+
assert result["userId"] == "u1"
111+
assert result["userLevel"] == 1
112+
113+
114+
def test_a_list_back_does_not_raise():
115+
"""`plant_list` answers with `back` as a list -- the guard must hold."""
116+
api = _api_returning(_response({"back": [{"success": False, "msg": "507"}]}))
117+
118+
assert api.plant_list("user_1") == [{"success": False, "msg": "507"}]
119+
120+
121+
def test_a_non_json_body_does_not_raise():
122+
"""The hook sees every response, including ones that are not JSON."""
123+
response = _response(body="<html>maintenance</html>")
124+
response.headers["Content-Type"] = "text/html"
125+
api = _api_returning(response)
126+
127+
for hook in api.session.hooks["response"]:
128+
hook(response)
129+
130+
131+
def test_a_bare_body_without_the_back_wrapper_is_checked():
132+
"""Not every endpoint wraps its payload, so read the top level too."""
133+
api = _api_returning(_response({"success": False, "msg": "507"}))
134+
135+
with pytest.raises(GrowattRateLimitError):
136+
api.plant_list("user_1")
137+
138+
139+
def test_http_errors_still_raise():
140+
"""The rate-limit check must not shadow raise_for_status."""
141+
api = _api_returning(_response({"back": {"success": True}}, status=500))
142+
143+
with pytest.raises(requests.exceptions.HTTPError):
144+
api.plant_list("user_1")

0 commit comments

Comments
 (0)