fix: tolerate empty optional endpoints on factory-reset ISY-994#506
Merged
Conversation
A factory-reset ISY-994 has no variables and no networking resources
configured, so its REST API returns 404 for /rest/vars/definitions/{1,2}
and /rest/networking/resources. The package handled this functionally
(empty managers, the variable parser already recognizes the 404 body
strings) but emitted ERROR-level log noise on every initialize().
Two paths cleaned up:
- get_variable_defs() now passes ok404=True per request, so the per-type
404s come back as benign "" instead of an ERROR "Invalid Command
Received" log line. The Variables parser already tolerated this body
via EMPTY_VARIABLE_RESPONSES; tightened it to also treat "" the same
as None ("no variables defined").
- Connection.request() now treats aiohttp.ClientResponseError as benign
when the caller already opted into ok404=True. ISY-994 firmware
desyncs keep-alive after a 404 for missing optional resources — the
next request on the reused socket reads the prior 404's HTML body
where an HTTP status line should be, surfacing as a "400 Expected
HTTP/, RTSP/ or ICE/:" parser error. The opt-in already says "this
endpoint may legitimately be empty", so absorb the 400-reframing the
same way.
Smoke-tested against a factory-reset ISY-994: previously emitted three
ERROR lines and one 400 traceback during initialize(); now emits only
the existing INFO ("No Type N variables defined") and WARNING ("No
valid variables defined") signals.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A factory-reset ISY-994 has no variables and no networking resources configured. Its REST API returns 404 for
/rest/vars/definitions/{1,2}and/rest/networking/resources— and on top of that its keep-alive framing desyncs after a 404, so the next request on the reused socket parses as a400 Expected HTTP/, RTSP/ or ICE/:error containing the prior 404's HTML body.PyISY handled all of this functionally (the variable parser already tolerated the 404 body strings via
EMPTY_VARIABLE_RESPONSES, the network manager handledNone), but everyinitialize()against an empty controller emitted three ERROR-level log lines + one 400 traceback for what is really an empty-config success.What changed
get_variable_defs()now passesok404=Trueper request so the per-type 404 returns benign""instead of anERROR Invalid Command Receivedlog line.Variables.parse_definitions()now treats""the same asNone("no variables defined"), matching the existingEMPTY_VARIABLE_RESPONSEShandling.Connection.request()treatsaiohttp.ClientResponseErroras benign when the caller already opted intook404=True. The ISY-994 keep-alive desync after a 404 surfaces as a parser error, but the caller already said "this endpoint may legitimately be empty" — so we absorb it the same way.Before / after (factory-reset ISY-994 at TLS 1.1, HTTP)
Before:
After:
(plus one DEBUG line per absorbed framing error.)
Test plan
pytest -q(438 passed)initialize(); now only the existing INFO/WARNING signalstest_get_variable_defs_passes_ok404test_request_client_response_error_with_ok404_returns_empty🤖 Generated with Claude Code