Fix session consistency in request handler methods#23
Merged
lmaertin merged 1 commit intolmaertin:mainfrom Mar 13, 2026
Merged
Conversation
…ice and get_access_point - set_value(): Use _get_session() instead of creating a standalone aiohttp.ClientSession, so externally provided sessions (e.g. from Home Assistant) are properly used and internal sessions are closed. - get_device_language(): Same fix - use _get_session() for session consistency with the rest of the request handler. - reboot_device(): Same fix - use _get_session() for session consistency. - get_access_point(): Fix double-read bug where resp.text() consumed the body and resp.json() would fail. Now parses JSON from the already-read text using json.loads(). Added 10 tests covering: - External session passthrough (not closed by handler) - Internal session creation and cleanup - Session cleanup on errors - Access point text/JSON parsing
Owner
|
Very clean enhancement, thanks for the tests as well - easy to merge. |
Contributor
Author
|
Hi @lmaertin Thanks for all the merges. I noticed the CI checks failed after the merge. From the logs it looks like the pylint step might be complaining about a few unused variables after the switch to I also saw a reference to Happy to open a small follow-up PR if something needs to be cleaned up. |
Owner
|
No Issue, I fixed this in the release already. |
21 tasks
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
2 participants
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.
Three methods in
RequestHandler(set_value(),get_device_language(),reboot_device()) create their ownaiohttp.ClientSessioninstead of using the shared_get_session()method. This bypasses externally provided sessions (e.g. from Home Assistant'saiohttp_client) and can cause resource leaks.Additionally,
get_access_point()has a double-read bug whereresp.text()consumes the response body and the subsequentresp.json()can fail.Changes
Session consistency (
set_value,get_device_language,reboot_device)aiohttp.ClientSession(connector=connector)withawait self._get_session()try/finallyblocks to close internally-created sessionsget_raw_data,get_debug_config, etc.)Double-read fix (
get_access_point)resp.text()already consumed the response bodyresp.json()withjson.loads(text[json_start:json_end])to parse from the already-read textTesting