Send as form, not just form_encoded #118
Conversation
WalkthroughAdds Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Client as AirOS
participant Device as Device HTTP
User->>Client: login(username, password)
Client->>Device: POST /login (v8 attempt, allow_redirects=true)
alt v8 succeeds
Device-->>Client: 200 OK (JSON)
Client->>User: login success
else v8 404 / fail
Device-->>Client: error
Client->>Device: GET /index.cgi (prefetch cookies, allow_redirects=true)
Device-->>Client: 200 OK (sets cookies)
Client->>Device: POST /login (v6 attempt: multipart/form-data or form-encoded, allow_redirects=true)
alt v6 succeeds
Device-->>Client: 200 OK (text/html or JSON)
Client->>Client: store auth cookies/CSRF, mark connected
Client->>Device: GET /index.cgi (finalize session)
Client->>User: login success
else all attempts fail
Device-->>Client: error
Client->>User: raise AirOSDeviceConnectionError
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #118 +/- ##
==========================================
- Coverage 89.57% 83.09% -6.49%
==========================================
Files 8 8
Lines 681 763 +82
==========================================
+ Hits 610 634 +24
- Misses 71 129 +58 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
CHANGELOG.md (1)
5-10: Consider adding more detail to the changelog entry.The changelog entry is accurate but quite brief. For library users, it would be helpful to include:
- What specifically changed (form-encoded → multipart/form-data)
- Why the change was necessary (e.g., "Fixed v6 login compatibility issue")
- Any backward compatibility considerations
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.md(1 hunks)airos/base.py(2 hunks)pyproject.toml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
🔇 Additional comments (2)
pyproject.toml (1)
7-7: LGTM! Version bump is appropriate.The version bump to
0.5.7a0is consistent with the changelog entry and appropriate for an alpha pre-release.airos/base.py (1)
230-230: LGTM! Signature expansion maintains backward compatibility.The expanded signature allowing
aiohttp.FormDatain addition todict[str, Any]is a good enhancement that maintains backward compatibility with existing code.
CoMPaTech
force-pushed
the
v6loginfix2
branch
from
85bddd3 to
65ff0b1
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airos/base.py(5 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (1)
airos/exceptions.py (3)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
🔇 Additional comments (2)
airos/base.py (2)
80-80: Clarify v6 login URI usage.
The v6 alternative URL includes?uri=/, but the login payload’surifield is set to"/index.cgi". Confirm which URI the endpoint expects (query parameter, form field, or both) and ensure they match.
335-342: Do not changeauthenticated=True; it’s required to inject CSRF and form-content headers for the v6 login.
Add tests covering the v6_alternative login flow.Likely an incorrect or invalid review comment.
airos/base.py
Outdated
| _LOGGER.error("Not connected, login first") | ||
| raise AirOSDeviceConnectionError from None | ||
|
|
||
| _LOGGER.error("TESTv6 - Trying with URL: %s", url) |
There was a problem hiding this comment.
Replace test logs with appropriate log level.
These log statements use _LOGGER.error() for test/debug messages. They should use _LOGGER.debug() instead, as they're not actual errors but informational logs for debugging the v6 login flow.
Apply this diff:
- _LOGGER.error("TESTv6 - Trying with URL: %s", url)
+ _LOGGER.debug("Requesting URL: %s", url)- _LOGGER.error("TESTv6 - Response: %s", response_text)
+ _LOGGER.debug("Response from %s: %s", url, response_text)Also applies to: 266-266
🤖 Prompt for AI Agents
In airos/base.py around lines 255 and 266, the two test/debug log statements
currently call _LOGGER.error; change both to _LOGGER.debug since they are
informational debug messages for the v6 login flow, not errors, and keep the
existing message and formatting intact so only the log level is adjusted.
airos/base.py
Outdated
| """Login to AirOS device.""" | ||
| payload = {"username": self.username, "password": self.password} | ||
| try: | ||
| _LOGGER.error("TESTv6 - Trying default v8 login URL") |
There was a problem hiding this comment.
Replace test logs with appropriate log level.
Multiple test logs use _LOGGER.error() instead of _LOGGER.debug(). These are not actual errors but informational messages for debugging the v6 login fallback.
Apply this diff:
- _LOGGER.error("TESTv6 - Trying default v8 login URL")
+ _LOGGER.debug("Attempting v8 login")- _LOGGER.error("TESTv6 - gives URL not found, trying alternative v6 URL")
+ _LOGGER.debug("v8 login not found, attempting v6 login")- _LOGGER.error("TESTv6 - Trying to get / first for cookies")
+ _LOGGER.debug("Prefetching cookies from / for v6 login")- _LOGGER.error("TESTv6 - Cookie response: %s", cookieresponse)
+ _LOGGER.debug("Cookie prefetch response: %s", cookieresponse)- _LOGGER.error(
- "TESTv6 - Trying to authenticate v6 responded in : %s", v6_response
- )
+ _LOGGER.debug("v6 login response: %s", v6_response)Also applies to: 309-309, 317-317, 322-322, 343-345
🤖 Prompt for AI Agents
In airos/base.py around lines 304, 309, 317, 322 and 343-345, several test/debug
messages are incorrectly logged with _LOGGER.error(); change those calls to
_LOGGER.debug() (preserving the existing message text and any interpolation) so
they are treated as debug/informational logs rather than errors.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (8)
airos/base.py (8)
256-256: Replace test logs with debug level.This log statement uses
_LOGGER.error()for a test/debug message. It should use_LOGGER.debug()instead.Based on past review comments
267-267: Replace test logs with debug level.This log statement uses
_LOGGER.error()for a test/debug message. It should use_LOGGER.debug()instead.Based on past review comments
270-279: Fix URL comparison bug.Line 270 checks
url == "/"but the actual URL used isf"{self.base_url}/"(e.g.,"https://192.168.1.1/"). This comparison will always fail, preventing proper auth data storage and cookie handling.Based on past review comments
305-305: Replace test logs with debug level.This log statement uses
_LOGGER.error()for a test/debug message. It should use_LOGGER.debug()instead.Based on past review comments
309-309: Replace test logs with debug level.This log statement uses
_LOGGER.error()for a test/debug message. It should use_LOGGER.debug()instead.Based on past review comments
317-323: Narrow exception suppression scope.Using
contextlib.suppress(Exception)is too broad and will hide programming errors, network issues, and other unexpected failures.Based on past review comments
351-354: Replace test logs with debug level.These log statements use
_LOGGER.error()for test/debug messages. They should use_LOGGER.debug()instead.Based on past review comments
381-384: Replace test logs with debug level.These log statements use
_LOGGER.error()for test/debug messages. They should use_LOGGER.debug()instead.Based on past review comments
🧹 Nitpick comments (6)
airos/base.py (6)
348-411: Add delays between retry attempts.The loop attempts 4 login combinations (2 URLs × 2 encoding methods) without any delays. This could overwhelm the device or trigger rate limiting. Consider adding a small delay (e.g., 0.5-1 second) between failed attempts.
Add an async delay between attempts:
import asyncio # Inside the loop, after each failed attempt block: await asyncio.sleep(0.5) # Small delay before next attempt
364-367: Replace test logs with debug level.This log statement uses
_LOGGER.warning()but could be_LOGGER.debug()since this is expected fallback behavior.- _LOGGER.warning( + _LOGGER.debug( "TESTv6 - V6 URL not found (%s) for form-urlencoded, trying multipart.", url_to_try, )
369-373: Replace test logs with debug level.This log statement uses
_LOGGER.warning()but could be_LOGGER.debug()since this is expected fallback behavior.- _LOGGER.warning( + _LOGGER.debug( "TESTv6 - V6 connection setup failed (%s) for form-urlencoded, trying multipart. Error: %s", url_to_try, err, )
393-396: Replace test logs with debug level.This log statement uses
_LOGGER.warning()but could be_LOGGER.debug()since this is expected fallback behavior.- _LOGGER.warning( + _LOGGER.debug( "TESTv6 - V6 URL not found (%s) for multipart, trying next URL.", url_to_try, )
398-402: Replace test logs with debug level.This log statement uses
_LOGGER.warning()but could be_LOGGER.debug()since this is expected fallback behavior.- _LOGGER.warning( + _LOGGER.debug( "TESTv6 - V6 connection setup failed (%s) for multipart, trying next URL. Error: %s", url_to_try, err, )
330-335: Consider making redirect URI configurable.The URI field is hardcoded to
"/index.cgi"for all v6 login attempts. If different AirOS v6 devices use different redirect URIs, this could fail. Consider making this configurable via a parameter or class attribute.Do you want me to generate a solution that makes the redirect URI configurable?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airos/base.py(6 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (1)
airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
airos/base.py (4)
258-259: Use debug level for non-error traces; adjust messagesThese are diagnostic traces, not errors.
Apply this diff:
@@ - _LOGGER.error("TESTv6 - Trying with URL: %s", url) + _LOGGER.debug("Requesting URL: %s", url) @@ - _LOGGER.error("TESTv6 - Response code: %s", response.status) + _LOGGER.debug("Response status: %s", response.status) @@ - _LOGGER.error("TESTv6 - Response: %s", response_text) + _LOGGER.debug("Response from %s: %s", url, response_text) @@ - _LOGGER.error("TESTv6 - Trying default v8 login URL") + _LOGGER.debug("Attempting v8 login") @@ - _LOGGER.error("TESTv6 - gives URL not found, trying alternative v6 URL") + _LOGGER.debug("v8 login not found, attempting v6 login") @@ - _LOGGER.error("TESTv6 - failed to login to v8 URL") + _LOGGER.debug("v8 login failed") @@ - _LOGGER.error("TESTv6 - returning from v8 login") + _LOGGER.debug("v8 login successful")Also applies to: 266-271, 308-320
80-82: Should uri be configurable for v6 login?Hard-coding uri="/" may not fit all v6 variants. Consider exposing it as a parameter or class attribute with a sensible default.
330-334: Add tests for v6 alternative (multipart and urlencoded) and verify device behaviorNo tests appear to cover v6_alternative with multipart/form-data. Add unit tests asserting Content-Type and fields, and verify against real hardware.
#!/bin/bash # Search for coverage of v6 alternative login and multipart usage set -euo pipefail echo "Searching for tests referencing v6 alternative, FormData, or login.cgi..." rg -nP -C3 --type=py 'v6_alternative|login\.cgi|FormData|multipart/form-data|uri=/' || true echo echo "List test files:" fd -e py -t f test || true
322-329: Narrow exception handling for cookie prefetch; avoid suppressing all exceptionsReplace contextlib.suppress(Exception) with targeted catch and debug logs.
Apply this diff:
- _LOGGER.error("TESTv6 - Trying to get / first for cookies") - with contextlib.suppress(Exception): - cookieresponse = await self._request_json( - "GET", f"{self.base_url}/", authenticated=False - ) - _LOGGER.error("TESTv6 - Cookie response: %s", cookieresponse) + _LOGGER.debug("Prefetching cookies from / for v6 login") + try: + cookieresponse = await self._request_json( + "GET", f"{self.base_url}/", authenticated=False + ) + _LOGGER.debug("Cookie prefetch response: %s", cookieresponse) + except (aiohttp.ClientError, asyncio.TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)
🧹 Nitpick comments (2)
airos/base.py (2)
335-360: Normalize remaining v6 flow logs to debug and drop "TESTv6 -" prefixFor consistency and to reduce noise, convert all remaining informational logs in the v6 flow to _LOGGER.debug and remove the "TESTv6 -" prefix. Keep error/warning levels only for actual failures that require attention.
Also applies to: 365-475
228-238: Guard against both json_data and form_data being setPrevent ambiguous requests by asserting only one body type is provided.
Apply this diff:
@@ async def _request_json( - ) -> dict[str, Any] | Any: + ) -> dict[str, Any] | Any: """Make an authenticated API request and return JSON response.""" + if json_data is not None and form_data is not None: + raise ValueError("Provide either json_data or form_data, not both")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
airos/base.py(7 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
airos/base.py
Outdated
| _LOGGER.error("TESTv6 - auth_cookie found: AIROS_%s", self._auth_cookie) | ||
| headers["Cookie"] = f"AIROS_{self._auth_cookie}" | ||
|
|
There was a problem hiding this comment.
Do not log auth cookies; downgrade to debug and redact
Current logs expose cookie contents at error level. Mask the value and use debug.
Apply this diff:
@@ def _get_authenticated_headers(...
- if self._auth_cookie: # pragma: no cover
- _LOGGER.error("TESTv6 - auth_cookie found: AIROS_%s", self._auth_cookie)
- headers["Cookie"] = f"AIROS_{self._auth_cookie}"
+ if self._auth_cookie: # pragma: no cover
+ cookie_name = self._auth_cookie.split("=", 1)[0]
+ _LOGGER.debug("Using auth cookie: AIROS_%s=***", cookie_name)
+ headers["Cookie"] = f"AIROS_{self._auth_cookie}"
@@ def _store_auth_data(...
- for key, morsel in cookie.items():
- _LOGGER.error("TESTv6 - cookie handling: %s with %s", key, morsel.value)
+ for key, morsel in cookie.items():
+ _LOGGER.debug("Cookie set: %s=***", key)
if key.startswith("AIROS_"):
self._auth_cookie = morsel.key[6:] + "=" + morsel.value
breakAlso applies to: 223-226
🤖 Prompt for AI Agents
In airos/base.py around lines 209-211 and 223-226, the code currently logs full
auth cookie values at error level and sets Cookie header with the raw value;
change the log level to debug and redact the cookie when logging (e.g., log only
a masked value or a constant like "REDACTED"), and ensure headers["Cookie"] is
still set but without logging the raw secret anywhere; update both locations to
use _LOGGER.debug(...) with a masked string and remove or replace any
error-level logging of self._auth_cookie.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
airos/base.py (1)
390-399: Content-Type mismatch in update_check(force=True).You send JSON but set form content-type. Use ct_json=True (or switch to form_data).
- return await self._request_json( - "POST", - self._update_check_url, - json_data={"force": True}, - authenticated=True, - ct_form=True, - ) + return await self._request_json( + "POST", + self._update_check_url, + json_data={"force": True}, + authenticated=True, + ct_json=True, + )
♻️ Duplicate comments (4)
airos/base.py (4)
257-266: Use debug for diagnostics; reserve error for real failures.These are debug traces; use _LOGGER.debug. Keep true errors at error level.
@@ - _LOGGER.error("TESTv6 - Trying with URL: %s", url) + _LOGGER.debug("Requesting URL: %s", url) @@ - _LOGGER.error("TESTv6 - Response code: %s", response.status) + _LOGGER.debug("Response code: %s", response.status) @@ - _LOGGER.error("TESTv6 - Trying default v8 login URL") + _LOGGER.debug("Attempting v8 login") @@ - _LOGGER.error("TESTv6 - gives URL not found, trying alternative v6 URL") + _LOGGER.debug("v8 login not found, attempting v6 login") @@ - _LOGGER.error("TESTv6 - failed to login to v8 URL") + _LOGGER.error("Failed to login to v8 URL") @@ - _LOGGER.error("TESTv6 - returning from v8 login") + _LOGGER.debug("v8 login succeeded") @@ - # Start of v6, go for cookies - _LOGGER.error("TESTv6 - Trying to get / first for cookies") + # Start of v6, go for cookies + _LOGGER.debug("Prefetching cookies from / for v6 login") @@ - _LOGGER.error("TESTv6 - start v6 attempts") + _LOGGER.debug("Starting v6 login attempts") @@ - _LOGGER.error( - "TESTv6 - Trying V6 POST to %s with SIMPLE multipart/form-data", - self._login_urls["v6_login"], - ) + _LOGGER.debug( + "Trying v6 POST to %s with multipart/form-data", + self._login_urls["v6_login"], + ) @@ - _LOGGER.error( - "TESTv6 - V6 simple multipart failed (%s) on %s. Error: %s", + _LOGGER.debug( + "v6 multipart attempt failed (%s) on %s. Error: %s", type(err).__name__, self._login_urls["v6_login"], err, ) @@ - _LOGGER.error("TESTv6 - autherror during extended multipart") + _LOGGER.error("Authentication error during v6 multipart") @@ - _LOGGER.error("TESTv6 - returning from simple multipart") + _LOGGER.debug("v6 multipart login succeeded")Also applies to: 304-316, 318-324, 335-361
300-361: Add tests for v6 multipart login and cookie prefetch.No tests cover the v6 multipart form login path or root cookie prefetch.
Run to confirm coverage:
#!/bin/bash rg -nP --type=py -C3 'login\.cgi|FormData|multipart/form-data|v6_login' || true fd -e py testI can draft tests that assert Content-Type is multipart/form-data and fields uri/username/password are sent, plus root-cookie prefetch behavior. Do you want that?
207-210: Redact cookies and downgrade to debug to avoid secret leakage.Don’t log raw auth cookies or Set-Cookie values. Mask and use debug.
@@ - if self._auth_cookie: # pragma: no cover - _LOGGER.error("TESTv6 - auth_cookie found: AIROS_%s", self._auth_cookie) - headers["Cookie"] = f"AIROS_{self._auth_cookie}" + if self._auth_cookie: # pragma: no cover + cookie_name = self._auth_cookie.split("=", 1)[0] + _LOGGER.debug("Using auth cookie: AIROS_%s=***", cookie_name) + headers["Cookie"] = f"AIROS_{self._auth_cookie}" @@ - for key, morsel in cookie.items(): - _LOGGER.error("TESTv6 - cookie handling: %s with %s", key, morsel.value) + for key, morsel in cookie.items(): + _LOGGER.debug("Cookie set: %s=***", key) if key.startswith("AIROS_"): self._auth_cookie = morsel.key[6:] + "=" + morsel.value breakAlso applies to: 221-226
318-324: Narrow exception suppression for cookie prefetch.Replace suppress(Exception) with specific network exceptions, and log as debug.
- _LOGGER.error("TESTv6 - Trying to get / first for cookies") - with contextlib.suppress(Exception): - cookieresponse = await self._request_json( - "GET", f"{self.base_url}/", authenticated=False - ) - _LOGGER.error("TESTv6 - Cookie response: %s", cookieresponse) + _LOGGER.debug("Prefetching cookies from / for v6 login") + try: + cookieresponse = await self._request_json( + "GET", f"{self.base_url}/", authenticated=False + ) + _LOGGER.debug("Cookie prefetch response: %s", cookieresponse) + except (aiohttp.ClientError, asyncio.TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)
🧹 Nitpick comments (1)
airos/base.py (1)
290-292: Also catch asyncio.TimeoutError.Include asyncio.TimeoutError alongside built-in TimeoutError.
- except (TimeoutError, aiohttp.ClientError) as err: + except (TimeoutError, asyncio.TimeoutError, aiohttp.ClientError) as err: _LOGGER.exception("Error during API call to %s", url) raise AirOSDeviceConnectionError from err
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
airos/base.py(7 hunks)pyproject.toml(1 hunks)tests/test_airos_request.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Run pytest using Python 3.14
- GitHub Check: Run pytest using Python 3.13
🔇 Additional comments (1)
tests/test_airos_request.py (1)
61-62: LGTM: assert allow_redirects=False.Matches internal behavior; keeps redirects explicit in tests.
Also applies to: 158-159
CoMPaTech
force-pushed
the
v6loginfix2
branch
from
3a6123b to
7c64369
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
airos/base.py (1)
413-421: Content-Type mismatch in update_check(force=True).You send JSON with
json_data={"force": True}but setct_form=True(x-www-form-urlencoded). Usect_json=Trueor switch to form data.- return await self._request_json( - "POST", - self._update_check_url, - json_data={"force": True}, - authenticated=True, - ct_form=True, - ) + return await self._request_json( + "POST", + self._update_check_url, + json_data={"force": True}, + authenticated=True, + ct_json=True, + )
♻️ Duplicate comments (8)
airos/base.py (8)
318-334: Login flow logs: downgrade to debug and tighten messages.These are flow diagnostics, not errors.
- _LOGGER.error("TESTv6 - Trying default v8 login URL") + _LOGGER.debug("Attempting v8 login") ... - _LOGGER.error("TESTv6 - failed to login to v8 URL") + _LOGGER.debug("v8 login request failed; raising setup error") ... - _LOGGER.error("TESTv6 - gives URL not found, trying alternative v6 URL") + _LOGGER.debug("v8 login URL not found; trying v6 fallback") ... - _LOGGER.error("TESTv6 - returning from v8 login") + _LOGGER.debug("v8 login successful")
353-381: Adjust logs; build fresh FormData per attempt; fix message wording.Use debug, build FormData in place, and fix “extended” -> “simple”.
- _LOGGER.error("TESTv6 - start v6 attempts") + _LOGGER.debug("Starting v6 login attempts") ... - _LOGGER.error( - "TESTv6 - Trying V6 POST to %s with SIMPLE multipart/form-data", - self._login_urls["v6_login"], - ) + _LOGGER.debug( + "Trying v6 POST to %s with multipart/form-data (simple)", + self._login_urls["v6_login"], + ) + v6_simple_multipart_form_data = aiohttp.FormData() + v6_simple_multipart_form_data.add_field("uri", "/index.cgi") + v6_simple_multipart_form_data.add_field("username", self.username) + v6_simple_multipart_form_data.add_field("password", self.password) ... - except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err: - _LOGGER.error( + except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err: + _LOGGER.debug( "TESTv6 - V6 simple multipart failed (%s) on %s. Error: %s", type(err).__name__, self._login_urls["v6_login"], err, ) - except AirOSConnectionAuthenticationError: - _LOGGER.error("TESTv6 - autherror during extended multipart") + except AirOSConnectionAuthenticationError: + _LOGGER.debug("TESTv6 - autherror during simple multipart") raise else: - _LOGGER.error("TESTv6 - returning from simple multipart") + _LOGGER.debug("v6 simple multipart login successful") return # Success
318-381: Add tests for v6 multipart login and root prefetch behavior.No tests exercise the v6 multipart path and root prefetch. Please add unit tests that:
- Assert Content-Type is multipart/form-data with fields uri/username/password.
- Cover root “/” prefetch returning {} and not raising.
- Cover v6 login 302/empty body handling and auth cookie storage.
#!/bin/bash # Find existing tests touching login paths or FormData usage rg -nP --type=py -C3 'login\.cgi|FormData|_request_json\(.+allow_redirects' # Recommend adding tests/tests_login_v6.py with mocked aiohttp server
205-211: Redact secrets; lower log level for CSRF/cookie headers.Don’t log CSRF IDs or auth cookies; use debug and mask values. Also keep setting headers unchanged.
- if self._csrf_id: # pragma: no cover - _LOGGER.error("TESTv6 - CSRF ID found %s", self._csrf_id) + if self._csrf_id: # pragma: no cover + _LOGGER.debug("Using CSRF ID: ***") headers["X-CSRF-ID"] = self._csrf_id - if self._auth_cookie: # pragma: no cover - _LOGGER.error("TESTv6 - auth_cookie found: AIROS_%s", self._auth_cookie) - headers["Cookie"] = f"AIROS_{self._auth_cookie}" + if self._auth_cookie: # pragma: no cover + cookie_name = self._auth_cookie.split("=", 1)[0] + _LOGGER.debug("Using auth cookie: AIROS_%s=***", cookie_name) + headers["Cookie"] = f"AIROS_{self._auth_cookie}"
221-229: Stop logging raw Set-Cookie; mask and downgrade level.Log cookies at debug and redact values; still parse and store AIROS_*.
- for set_cookie in response.headers.getall("Set-Cookie", []): - _LOGGER.error("TESTv6 - regular cookie handling: %s", set_cookie) + for set_cookie in response.headers.getall("Set-Cookie", []): + _LOGGER.debug("Set-Cookie received: %s", set_cookie.split(";", 1)[0] + "=***") cookie.load(set_cookie) - for key, morsel in cookie.items(): - _LOGGER.error( - "TESTv6 - AIROS_cookie handling: %s with %s", key, morsel.value - ) + for key, morsel in cookie.items(): + _LOGGER.debug("Cookie set: %s=***", key) if key.startswith("AIROS_"): self._auth_cookie = morsel.key[6:] + "=" + morsel.value break
260-266: Blocker: auth gate breaks cookie prefetch and root handling.Gate should respect
authenticatedand compare root againstf"{self.base_url}/". Currenturl != "/"always true; root prefetch is wrongly blocked.- if ( - url not in self._login_urls.values() - and url != "/" - and not self.connected - ): + if ( + authenticated + and url not in self._login_urls.values() + and url != f"{self.base_url}/" + and not self.connected + ): _LOGGER.error("Not connected, login first") raise AirOSDeviceConnectionError from None
279-298: v6 login/root handling: skip raise_for_status, return {} for root, avoid double store.
- Don’t raise on v6 login or root prefetch.
- Return
{}for root prefetch; don’t JSON-decode.- Store auth for root as well; avoid duplicate
_store_auth_datafor v6.- # v6 responds with a 302 redirect and empty body - if url != self._login_urls["v6_login"]: - response.raise_for_status() + # v6 login often 302/empty; root prefetch may redirect; skip raise here + if url not in {self._login_urls["v6_login"], f"{self.base_url}/"}: + response.raise_for_status() response_text = await response.text() - _LOGGER.error("Successfully fetched %s from %s", response_text, url) + _LOGGER.debug("Fetched response from %s (len=%d)", url, len(response_text)) # If this is the login request, we need to store the new auth data - if url in self._login_urls.values(): + if url in self._login_urls.values() or url == f"{self.base_url}/": self._store_auth_data(response) self.connected = True - _LOGGER.error("TESTv6 - response: %s", response_text) - # V6 responds with empty body on login, not JSON - if url == self._login_urls["v6_login"]: - self._store_auth_data(response) - self.connected = True - return {} + # Root prefetch: no JSON expected + if method == "GET" and url == f"{self.base_url}/": + return {} + # v6 login: often empty body/redirect; do not JSON-decode + if url == self._login_urls["v6_login"]: + return {} if not response_text.strip() else response_text return json.loads(response_text)
336-343: Avoid suppressing all exceptions; catch expected network errors, and root prefetch shouldn’t be authenticated.Use try/except for expected errors and call root prefetch with
authenticated=Falseto avoid gate and header injection.- _LOGGER.error("TESTv6 - Trying to get / first for cookies") - with contextlib.suppress(Exception): - cookieresponse = await self._request_json( - "GET", f"{self.base_url}/", authenticated=True - ) - _LOGGER.error("TESTv6 - Cookie response: %s", cookieresponse) + _LOGGER.debug("Prefetching cookies from / for v6 login") + try: + _ = await self._request_json( + "GET", f"{self.base_url}/", authenticated=False + ) + _LOGGER.debug("Cookie prefetch completed") + except (aiohttp.ClientError, asyncio.TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)
🧹 Nitpick comments (2)
airos/base.py (2)
268-285: Debug-level request logging; avoid logging full bodies.Switch to debug and don’t log entire response bodies (risk of secrets; noisy). Log URL and status; optionally content length.
- _LOGGER.error("TESTv6 - Trying with URL: %s", url) + _LOGGER.debug("Requesting %s %s", method, url) ... - _LOGGER.error("TESTv6 - Response code: %s", response.status) + _LOGGER.debug("Response %s: %s", url, response.status) ... - response_text = await response.text() - _LOGGER.error("Successfully fetched %s from %s", response_text, url) + response_text = await response.text() + _LOGGER.debug("Fetched response from %s (len=%d)", url, len(response_text))
344-347: Create FormData close to use (clarity, future-proof against reuse).Construct the FormData inside the attempt block to avoid accidental reuse later.
- v6_simple_multipart_form_data = aiohttp.FormData() - v6_simple_multipart_form_data.add_field("uri", "/index.cgi") - v6_simple_multipart_form_data.add_field("username", self.username) - v6_simple_multipart_form_data.add_field("password", self.password) + # FormData will be constructed per attempt below
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/verify.yml(1 hunks)airos/base.py(7 hunks)pyproject.toml(1 hunks)tests/test_airos_request.py(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- tests/test_airos_request.py
- .github/workflows/verify.yml
- pyproject.toml
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
🔇 Additional comments (3)
airos/base.py (3)
382-390: Status headers LGTM.Passing Accept and X-Requested-With explicitly for status.cgi is good; consistent with server expectations.
430-446: stakick flow LGTM.Form-encoded POST, uppercased MAC, and boolean return are consistent.
447-461: provmode flow LGTM.Form-encoded with action=start/stop; looks correct.
| # Potential XM fix - not sure, might have been login issue | ||
| if url == self._status_cgi_url: | ||
| request_headers["Referrer"] = f"{self.base_url}/login.cgi" | ||
| request_headers["Accept"] = "application/json, text/javascript, */*; q=0.01" | ||
| request_headers["X-Requested-With"] = "XMLHttpRequest" | ||
|
|
There was a problem hiding this comment.
HTTP header typo: use “Referer”, not “Referrer”.
“Referrer” is ignored; should be “Referer”.
- request_headers["Referrer"] = f"{self.base_url}/login.cgi"
+ request_headers["Referer"] = f"{self.base_url}/login.cgi"📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| # Potential XM fix - not sure, might have been login issue | |
| if url == self._status_cgi_url: | |
| request_headers["Referrer"] = f"{self.base_url}/login.cgi" | |
| request_headers["Accept"] = "application/json, text/javascript, */*; q=0.01" | |
| request_headers["X-Requested-With"] = "XMLHttpRequest" | |
| # Potential XM fix - not sure, might have been login issue | |
| if url == self._status_cgi_url: | |
| request_headers["Referer"] = f"{self.base_url}/login.cgi" | |
| request_headers["Accept"] = "application/json, text/javascript, */*; q=0.01" | |
| request_headers["X-Requested-With"] = "XMLHttpRequest" |
🤖 Prompt for AI Agents
In airos/base.py around lines 253 to 258, the code sets the HTTP header
"Referrer" which is a typo and ignored by servers; change the header key to the
correct "Referer" (exact spelling) when assigning request_headers["Referrer"] so
it becomes request_headers["Referer"] and keep the same value string; no other
logic changes required.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
airos/base.py (2)
451-461: Blocker: Mismatched Content-Type in update_check(force=True).Sends JSON payload while forcing x-www-form-urlencoded header. Servers may reject or misparse.
if force: return await self._request_json( "POST", self._update_check_url, - json_data={"force": True}, - authenticated=True, - ct_form=True, + json_data={"force": True}, + authenticated=True, + ct_json=True, )
243-254: Guard against conflicting payloads (json vs form).Defensive check prevents ambiguous Content-Type and handler confusion.
async def _request_json( @@ ) -> dict[str, Any] | Any: """Make an authenticated API request and return JSON response.""" + if json_data is not None and form_data is not None: + raise ValueError("Provide either json_data or form_data, not both.")
♻️ Duplicate comments (4)
airos/base.py (4)
352-367: Downgrade non-error flow logs to debug; avoid dumping full bodies.These are diagnostics, not errors. Also avoid logging entire response bodies.
- _LOGGER.error("TESTv%s - Trying with URL: %s", self.api_version, url) + _LOGGER.debug("Requesting URL: %s (v%s)", url, self.api_version) @@ - _LOGGER.error( - "TESTv%s - Response code: %s", self.api_version, response.status - ) + _LOGGER.debug("Response code: %s (v%s)", response.status, self.api_version) @@ - _LOGGER.error("Successfully fetched %s from %s", response_text, url) + _LOGGER.debug("Fetched response from %s", url) @@ - _LOGGER.error("TESTv%s - Trying default v8 login URL", self.api_version) + _LOGGER.debug("Attempting v8 login (v%s)", self.api_version) @@ - _LOGGER.error( - "TESTv%s - gives URL not found, trying alternative v6 URL", - self.api_version, - ) + _LOGGER.debug("v8 URL not found; falling back to v6 login (v%s)", self.api_version) @@ - _LOGGER.error("TESTv%s - failed to login to v8 URL", self.api_version) + _LOGGER.debug("v8 login attempt failed (v%s)", self.api_version) @@ - _LOGGER.error("TESTv%s - returning from v8 login", self.api_version) + _LOGGER.debug("v8 login succeeded (v%s)", self.api_version) @@ - _LOGGER.error("TESTv%s - start v6 attempts", self.api_version) + _LOGGER.debug("Starting v6 login attempts (v%s)", self.api_version) @@ - _LOGGER.error( - "TESTv%s - Trying V6 POST to %s with SIMPLE multipart/form-data", - self.api_version, - self._login_urls["v6_login"], - ) + _LOGGER.debug("v6 login POST (multipart/form-data) to %s (v%s)", + self._login_urls["v6_login"], self.api_version) @@ - _LOGGER.error( - "TESTv%s - V6 simple multipart failed (%s) on %s. Error: %s", - self.api_version, - type(err).__name__, - self._login_urls["v6_login"], - err, - ) + _LOGGER.debug("v6 multipart failed (%s) on %s: %s (v%s)", + type(err).__name__, self._login_urls["v6_login"], err, self.api_version) @@ - _LOGGER.error( - "TESTv%s - autherror during extended multipart", self.api_version - ) + _LOGGER.debug("v6 login auth error (multipart) (v%s)", self.api_version) @@ - _LOGGER.error("TESTv%s - returning from simple multipart", self.api_version) + _LOGGER.debug("v6 login succeeded (multipart) (v%s)", self.api_version)Also applies to: 388-419, 295-306, 314-315
265-278: Blocker: HTTP header typo — use "Referer" (not "Referrer").This header is ignored as written. Fix to ensure servers see the origin.
- request_headers["Referrer"] = f"{self.base_url}/index.cgi" + request_headers["Referer"] = f"{self.base_url}/index.cgi"
369-377: Narrow exception scope for cookie prefetch and set v6 mode here.
- Broad suppress hides real bugs.
- Root cookie prefetch does not need auth headers.
- Set api_version=6 when entering the v6 flow, not inside _request_json.
- # Start of v6, go for cookies - _LOGGER.error("TESTv%s - Trying to get / first for cookies", self.api_version) - with contextlib.suppress(Exception): - cookieresponse = await self._request_json( - "GET", f"{self.base_url}/", authenticated=True - ) - _LOGGER.error( - "TESTv%s - Cookie response: %s", self.api_version, cookieresponse - ) + # Start of v6, prefetch cookies, switch to v6 + self.api_version = 6 + _LOGGER.debug("Prefetching cookies from / for v6 login") + try: + cookieresponse = await self._request_json( + "GET", f"{self.base_url}/", authenticated=False + ) + _LOGGER.debug("Cookie prefetch response: %s", cookieresponse) + except (aiohttp.ClientError, asyncio.TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)
207-216: Do not log CSRF or cookies; redact and downgrade to debug.Current logs leak secrets at error level. Redact values and use debug.
@@ - if self._csrf_id: # pragma: no cover - _LOGGER.error("TESTv%s - CSRF ID found %s", self.api_version, self._csrf_id) + if self._csrf_id: # pragma: no cover + _LOGGER.debug("CSRF ID present for v%s", self.api_version) headers["X-CSRF-ID"] = self._csrf_id @@ - if self._auth_cookie: # pragma: no cover - _LOGGER.error( - "TESTv%s - auth_cookie found: AIROS_%s", - self.api_version, - self._auth_cookie, - ) + if self._auth_cookie: # pragma: no cover + cookie_name = self._auth_cookie.split("=", 1)[0] + _LOGGER.debug("Using auth cookie: AIROS_%s=*** (v%s)", cookie_name, self.api_version) headers["Cookie"] = f"AIROS_{self._auth_cookie}" @@ - for set_cookie in response.headers.getall("Set-Cookie", []): - _LOGGER.error( - "TESTv%s - regular cookie handling: %s", self.api_version, set_cookie - ) + for set_cookie in response.headers.getall("Set-Cookie", []): + _LOGGER.debug("Cookie received (masked): %s", set_cookie.split("=",1)[0] + "=***") cookie.load(set_cookie) for key, morsel in cookie.items(): - _LOGGER.error( - "TESTv%s - AIROS_cookie handling: %s with %s", - self.api_version, - key, - morsel.value, - ) + _LOGGER.debug("Cookie set: %s=*** (v%s)", key, self.api_version) if key.startswith("AIROS_"): self._auth_cookie = morsel.key[6:] + "=" + morsel.value breakAlso applies to: 228-238
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airos/base.py(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
🔇 Additional comments (1)
airos/base.py (1)
72-73: LGTM on API surface changes.
- api_version default to 8
- v6_login key addition
- form_data and allow_redirects support
If you have access to v6 hardware, please confirm multipart/form-data login works end-to-end.
Also applies to: 83-84, 249-254
| try: | ||
| if url not in self._login_urls.values() and not self.connected: | ||
| if ( | ||
| url not in self._login_urls.values() | ||
| and url != f"{self.base_url}/" | ||
| and not self.connected | ||
| ): | ||
| _LOGGER.error("Not connected, login first") | ||
| raise AirOSDeviceConnectionError from None | ||
|
|
There was a problem hiding this comment.
Blocker: Auth gate and v6/root handling — wrong conditions, JSON decode, and version flip.
- Gate should only fire for authenticated calls.
- Don't raise on v6 login or root prefetch.
- Don't set api_version=6 for every non-login URL.
- Avoid JSON-decoding v6 login or root responses.
@@
- if (
- url not in self._login_urls.values()
- and url != f"{self.base_url}/"
- and not self.connected
- ):
+ if (
+ authenticated
+ and url not in self._login_urls.values()
+ and url != f"{self.base_url}/"
+ and not self.connected
+ ):
_LOGGER.error("Not connected, login first")
raise AirOSDeviceConnectionError from None
@@
- # v6 responds with a 302 redirect and empty body
- if url != self._login_urls["v6_login"]:
- self.api_version = 6
- response.raise_for_status()
+ # v6 login (302/empty) and root prefetch may not be JSON; don't raise here
+ if url not in {self._login_urls["v6_login"], f"{self.base_url}/"}:
+ response.raise_for_status()
@@
- response_text = await response.text()
- _LOGGER.error("Successfully fetched %s from %s", response_text, url)
+ response_text = await response.text()
+ _LOGGER.debug("Fetched response from %s", url)
@@
- _LOGGER.error("TESTv%s - response: %s", self.api_version, response_text)
- # V6 responds with empty body on login, not JSON
- if url.startswith(self._login_urls["v6_login"]):
- self._store_auth_data(response)
- self.connected = True
- return {}
+ _LOGGER.debug("Response body from %s: %s", url, response_text[:200])
+ # Root cookie prefetch: no JSON expected
+ if method == "GET" and url == f"{self.base_url}/":
+ return {}
+ # v6 login: often empty body/redirect; do not JSON-decode
+ if url == self._login_urls["v6_login"]:
+ return {} if not response_text.strip() else response_textAlso applies to: 308-315, 321-328
🤖 Prompt for AI Agents
In airos/base.py around lines 279-287 (and similarly at 308-315 and 321-328),
the auth/connection gate and v6/root handling are incorrect: only authenticated
calls should trigger the "not connected" error, v6 login and root prefetch
responses must not cause a raise, you must not unconditionally set api_version=6
for every non-login URL, and you must avoid JSON-decoding v6 login or root
responses. Fix by: check whether the request requires authentication before
raising AirOSDeviceConnectionError; skip the connection check and avoid raising
for v6 login and root-prefetch endpoints; only set api_version to 6 when you
have explicit evidence (e.g., response headers or a successful v6 login), not
for every non-login URL; and guard JSON decoding so you do not attempt to parse
v6 login/root responses (treat them as raw text or check content-type/version
first). Ensure these same fixes are applied at the other referenced line ranges.
| except AirOSConnectionAuthenticationError: | ||
| _LOGGER.error( | ||
| "TESTv%s - autherror during extended multipart", self.api_version | ||
| ) | ||
| raise |
There was a problem hiding this comment.
Message mismatch: says “extended multipart” inside simple attempt.
Adjust text to match the attempt type.
- _LOGGER.error(
- "TESTv%s - autherror during extended multipart", self.api_version
- )
+ _LOGGER.debug("v6 login auth error (simple multipart) (v%s)", self.api_version)🤖 Prompt for AI Agents
In airos/base.py around lines 412 to 416, the error log message incorrectly says
"autherror during extended multipart" while this except block is handling the
simple attempt; update the log string to reflect the correct attempt type (e.g.,
"autherror during simple attempt") and keep the same API version interpolation
and raise behavior.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (6)
airos/base.py (6)
432-436: Fix misleading error message.Line 434 says "extended multipart" but this except block is handling the simple multipart attempt. The message is incorrect.
Apply this diff:
except AirOSConnectionAuthenticationError: - _LOGGER.error( - "TESTv%s - autherror during extended multipart", self.api_version - ) + _LOGGER.debug("v6 simple multipart auth error (api_version=%s)", self.api_version) raise
295-301: Major: Auth gate doesn't respectauthenticatedparameter.Lines 295-299 block requests when
not self.connected, regardless of whether the caller explicitly passedauthenticated=False. This prevents legitimate unauthenticated preflight requests (like the root cookie prefetch at line 392-394).Apply this diff to only enforce the gate for authenticated requests:
- if ( - url not in self._login_urls.values() - and url != f"{self.base_url}/" - and not self.connected - ): + if ( + authenticated + and url not in self._login_urls.values() + and url != f"{self.base_url}/" + and not self.connected + ): _LOGGER.error("Not connected, login first") raise AirOSDeviceConnectionError from None
265-293: Critical: Invalid HTTP header name "Referrer" (should be "Referer").Lines 268 and 279 use
"Referrer"as an HTTP header name. The correct HTTP/1.1 header is"Referer"(with the intentional misspelling from RFC 2616). Servers ignore the"Referrer"header, which may break v6 login flows that depend on referer validation.Apply this diff:
if self.api_version == 6 and url.startswith(self._status_cgi_url): # Modified from login.cgi to index.cgi - request_headers["Referrer"] = f"{self.base_url}/index.cgi" + request_headers["Referer"] = f"{self.base_url}/index.cgi" request_headers["Accept"] = "application/json, text/javascript, */*; q=0.01" ... if url.startswith(self._login_urls["v6_login"]): - request_headers["Referrer"] = f"{self.base_url}/login.cgi" + request_headers["Referer"] = f"{self.base_url}/login.cgi" request_headers["Origin"] = self.base_url
334-341: Downgrade success logs to debug level.Lines 334 and 341 log successful responses at error level. These should use debug level for normal operational logging.
Apply this diff:
response_text = await response.text() - _LOGGER.error("Successfully fetched %s from %s", response_text, url) + _LOGGER.debug("Fetched response from %s: %s", url, response_text[:200]) # Truncate for brevity # If this is the login request, we need to store the new auth data if url in self._login_urls.values(): self._store_auth_data(response) self.connected = True - - _LOGGER.error("TESTv%s - response: %s", self.api_version, response_text)
228-238: Critical: Cookie values logged without redaction.Lines 228-238 log raw cookie headers and values at error level. Authentication cookies contain sensitive session data that must not appear in production logs.
Apply this diff to redact and downgrade log level:
for set_cookie in response.headers.getall("Set-Cookie", []): - _LOGGER.error( - "TESTv%s - regular cookie handling: %s", self.api_version, set_cookie - ) + _LOGGER.debug("Processing Set-Cookie header (api_version=%s)", self.api_version) cookie.load(set_cookie) for key, morsel in cookie.items(): - _LOGGER.error( - "TESTv%s - AIROS_cookie handling: %s with %s", - self.api_version, - key, - morsel.value, - ) + _LOGGER.debug("Found cookie: %s=*** (api_version=%s)", key, self.api_version) if key.startswith("AIROS_"): self._auth_cookie = morsel.key[6:] + "=" + morsel.value break
372-387: Use debug level for login flow logs.Lines 372, 377-380, 383, and 386 use error level for normal login flow information. Reserve error level for actual failures.
Apply this diff:
try: - _LOGGER.error("TESTv%s - Trying default v8 login URL", self.api_version) + _LOGGER.debug("Attempting v8 login (api_version=%s)", self.api_version) await self._request_json( "POST", self._login_urls["default"], json_data=payload ) except AirOSUrlNotFoundError: - _LOGGER.error( - "TESTv%s - gives URL not found, trying alternative v6 URL", - self.api_version, - ) + _LOGGER.debug("v8 endpoint not found, attempting v6 login (api_version=%s)", self.api_version) # Try next URL except AirOSConnectionSetupError as err: - _LOGGER.error("TESTv%s - failed to login to v8 URL", self.api_version) + _LOGGER.debug("v8 login failed (api_version=%s)", self.api_version) raise AirOSConnectionSetupError("Failed to login to AirOS device") from err else: - _LOGGER.error("TESTv%s - returning from v8 login", self.api_version) + _LOGGER.debug("v8 login succeeded (api_version=%s)", self.api_version) return
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
airos/base.py(8 hunks)tests/test_airos6.py(2 hunks)tests/test_airos8.py(2 hunks)tests/test_airos_request.py(6 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
🔇 Additional comments (9)
airos/base.py (1)
456-464: LGTM: Proper AJAX headers for status endpoint.Lines 458-461 correctly define headers for XMLHttpRequest-style status queries. This aligns with how browsers make AJAX requests to AirOS devices.
tests/test_airos8.py (2)
83-83: LGTM: Appropriate Content-Type for invalid JSON test.Setting
Content-Type: text/htmlcorrectly simulates a scenario where the server returns an HTML error page instead of JSON, which is the error condition this test validates.
105-105: LGTM: Consistent Content-Type for missing data test.Same pattern as line 83—properly simulates non-JSON responses for error handling tests.
tests/test_airos6.py (2)
82-82: LGTM: Proper mock setup for invalid JSON test.Setting
Content-Type: text/htmlcorrectly simulates non-JSON error responses, consistent with the test's purpose.
104-104: LGTM: Consistent Content-Type for error scenario.Matches the pattern in test_airos8.py for testing non-JSON response handling.
tests/test_airos_request.py (4)
45-63: LGTM: Proper verification of allow_redirects parameter.Lines 45 and 62 correctly add Content-Type header to mocks and verify that
allow_redirects=Trueis forwarded to the aiohttp session request. This validates the new parameter added to_request_jsonsignature.
91-91: LGTM: Consistent mock setup.Content-Type header properly set for non-JSON error test scenario.
119-119: LGTM: Appropriate for JSON decode error test.Content-Type: text/html correctly simulates a response that cannot be parsed as JSON.
142-162: LGTM: Complete parameter validation.Lines 142 and 162 properly verify all request parameters including the new
allow_redirects=Trueare correctly forwarded to the session.
airos/base.py
Outdated
| _LOGGER.error("TESTv%s - CSRF ID found %s", self.api_version, self._csrf_id) | ||
| headers["X-CSRF-ID"] = self._csrf_id | ||
|
|
||
| if self._auth_cookie: # pragma: no cover | ||
| _LOGGER.error( | ||
| "TESTv%s - auth_cookie found: AIROS_%s", | ||
| self.api_version, | ||
| self._auth_cookie, | ||
| ) | ||
| headers["Cookie"] = f"AIROS_{self._auth_cookie}" |
There was a problem hiding this comment.
Critical: Sensitive auth data logged at error level without redaction.
Lines 208 and 212-216 log CSRF tokens and authentication cookies at error level with full values exposed. This violates security best practices and can leak credentials in production logs.
Apply this diff to use debug level and redact sensitive values:
if self._csrf_id: # pragma: no cover
- _LOGGER.error("TESTv%s - CSRF ID found %s", self.api_version, self._csrf_id)
+ _LOGGER.debug("Using CSRF token (api_version=%s)", self.api_version)
headers["X-CSRF-ID"] = self._csrf_id
if self._auth_cookie: # pragma: no cover
- _LOGGER.error(
- "TESTv%s - auth_cookie found: AIROS_%s",
- self.api_version,
- self._auth_cookie,
- )
+ cookie_name = self._auth_cookie.split("=", 1)[0] if "=" in self._auth_cookie else "***"
+ _LOGGER.debug("Using auth cookie: AIROS_%s=*** (api_version=%s)", cookie_name, self.api_version)
headers["Cookie"] = f"AIROS_{self._auth_cookie}"🤖 Prompt for AI Agents
In airos/base.py around lines 208 to 217, the code logs CSRF tokens and auth
cookies at error level exposing full sensitive values; change these log calls to
use debug level and redact the sensitive values (for example mask all but the
last 4 characters or replace with a fixed placeholder like "<redacted>") before
logging, and adjust the log messages to include context without printing the raw
token/cookie (e.g., "CSRF ID found: ****1234" or "auth_cookie found:
<redacted>"); ensure the headers still set the full values, only the log output
is redacted.
| _LOGGER.error("TESTv%s - Trying with URL: %s", self.api_version, url) | ||
| async with self.session.request( | ||
| method, | ||
| url, | ||
| json=json_data, | ||
| data=form_data, | ||
| headers=request_headers, # Pass the constructed headers | ||
| allow_redirects=allow_redirects, | ||
| ) as response: | ||
| response.raise_for_status() | ||
| _LOGGER.error( | ||
| "TESTv%s - Response code: %s", self.api_version, response.status | ||
| ) | ||
| _LOGGER.error( | ||
| "TESTv%s - Response headers: %s", | ||
| self.api_version, | ||
| dict(response.headers), | ||
| ) |
There was a problem hiding this comment.
Use debug level for diagnostic logs.
Lines 310, 319-320, and 322-326 log diagnostic information at error level. These are not errors but informational messages for debugging request flow.
Apply this diff:
- _LOGGER.error("TESTv%s - Trying with URL: %s", self.api_version, url)
+ _LOGGER.debug("Requesting URL: %s (api_version=%s)", url, self.api_version)
async with self.session.request(
...
) as response:
- _LOGGER.error(
- "TESTv%s - Response code: %s", self.api_version, response.status
- )
- _LOGGER.error(
- "TESTv%s - Response headers: %s",
- self.api_version,
- dict(response.headers),
- )
+ _LOGGER.debug("Response status: %s (api_version=%s)", response.status, self.api_version)
+ _LOGGER.debug("Response headers: %s (api_version=%s)", dict(response.headers), self.api_version)🤖 Prompt for AI Agents
In airos/base.py around lines 310 to 326 the diagnostic logs use _LOGGER.error
for non-error information (attempted URL, response code, and response headers);
change those _LOGGER.error calls to _LOGGER.debug so these are logged at debug
level instead of error level, preserving the same message text and interpolation
but using debug() for each of the occurrences on lines ~310, ~319-320, and
~322-326.
| # Start of v6, go for cookies | ||
| _LOGGER.error("TESTv%s - Trying to get / first for cookies", self.api_version) | ||
| with contextlib.suppress(Exception): | ||
| cookieresponse = await self._request_json( | ||
| "GET", f"{self.base_url}/", authenticated=True | ||
| ) | ||
| _LOGGER.error( | ||
| "TESTv%s - Cookie response: %s", self.api_version, cookieresponse | ||
| ) |
There was a problem hiding this comment.
Major: Overly broad exception suppression and wrong authenticated flag.
Line 391 uses contextlib.suppress(Exception) which is too broad and hides all errors including programming errors. Line 393 passes authenticated=True for a preflight cookie request that should be unauthenticated. Lines 390 and 395-397 log at error level.
Apply this diff:
# Start of v6, go for cookies
- _LOGGER.error("TESTv%s - Trying to get / first for cookies", self.api_version)
- with contextlib.suppress(Exception):
+ _LOGGER.debug("Prefetching cookies from / for v6 (api_version=%s)", self.api_version)
+ try:
cookieresponse = await self._request_json(
- "GET", f"{self.base_url}/", authenticated=True
+ "GET", f"{self.base_url}/", authenticated=False
)
- _LOGGER.error(
- "TESTv%s - Cookie response: %s", self.api_version, cookieresponse
- )
+ _LOGGER.debug("Cookie prefetch response: %s (api_version=%s)", cookieresponse, self.api_version)
+ except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err:
+ _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)| _LOGGER.error("TESTv%s - start v6 attempts", self.api_version) | ||
| # --- ATTEMPT B: Simple Payload (multipart/form-data) --- | ||
| try: | ||
| _LOGGER.error( | ||
| "TESTv%s - Trying V6 POST to %s with SIMPLE multipart/form-data", | ||
| self.api_version, | ||
| self._login_urls["v6_login"], | ||
| ) | ||
| await self._request_json( | ||
| "POST", | ||
| self._login_urls["v6_alternative"], | ||
| form_data=payload, | ||
| ct_form=True, | ||
| self._login_urls["v6_login"], | ||
| headers=login_headers, | ||
| form_data=v6_simple_multipart_form_data, | ||
| authenticated=True, | ||
| allow_redirects=True, | ||
| ) | ||
| except AirOSConnectionSetupError as err: | ||
| raise AirOSConnectionSetupError( | ||
| "Failed to login to default and alternate AirOS device urls" | ||
| ) from err | ||
| except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err: | ||
| _LOGGER.error( | ||
| "TESTv%s - V6 simple multipart failed (%s) on %s. Error: %s", | ||
| self.api_version, | ||
| type(err).__name__, | ||
| self._login_urls["v6_login"], | ||
| err, | ||
| ) |
There was a problem hiding this comment.
Use debug level for v6 login attempt logs.
Lines 408, 411-415, and 425-431 log normal login flow information at error level. These should be debug level.
Apply this diff:
- _LOGGER.error("TESTv%s - start v6 attempts", self.api_version)
+ _LOGGER.debug("Starting v6 login attempts (api_version=%s)", self.api_version)
# --- ATTEMPT B: Simple Payload (multipart/form-data) ---
try:
- _LOGGER.error(
- "TESTv%s - Trying V6 POST to %s with SIMPLE multipart/form-data",
- self.api_version,
- self._login_urls["v6_login"],
- )
+ _LOGGER.debug("Attempting v6 simple multipart login (api_version=%s)", self.api_version)
await self._request_json(
...
)
except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err:
- _LOGGER.error(
- "TESTv%s - V6 simple multipart failed (%s) on %s. Error: %s",
- self.api_version,
- type(err).__name__,
- self._login_urls["v6_login"],
- err,
- )
+ _LOGGER.debug(
+ "v6 simple multipart failed (%s): %s (api_version=%s)",
+ type(err).__name__,
+ err,
+ self.api_version,
+ )🤖 Prompt for AI Agents
In airos/base.py lines 408, 411-415 and 425-431, the login flow messages are
logged using _LOGGER.error even though they represent normal attempt/info-level
details; change those specific _LOGGER.error(...) calls to _LOGGER.debug(...) so
the v6 login attempt start, the "Trying V6 POST..." message, and the "V6 simple
multipart failed..." message are logged at debug level (leave other genuine
error logs untouched).
| else: | ||
| _LOGGER.error("TESTv%s - returning from simple multipart", self.api_version) | ||
| # Finalize session by visiting /index.cgi | ||
| _LOGGER.error( | ||
| "TESTv%s - Finalizing session with GET to /index.cgi", self.api_version | ||
| ) | ||
| with contextlib.suppress(Exception): | ||
| await self._request_json( | ||
| "GET", | ||
| f"{self.base_url}/index.cgi", | ||
| headers={ | ||
| "Referer": f"{self.base_url}/login.cgi", | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", | ||
| }, | ||
| authenticated=True, | ||
| allow_redirects=True, | ||
| ) | ||
| return # Success |
There was a problem hiding this comment.
Major: Overly broad exception suppression in finalization.
Line 443 uses contextlib.suppress(Exception) which is too broad. Lines 438-441 log at error level.
Apply this diff:
else:
- _LOGGER.error("TESTv%s - returning from simple multipart", self.api_version)
+ _LOGGER.debug("v6 simple multipart succeeded (api_version=%s)", self.api_version)
# Finalize session by visiting /index.cgi
- _LOGGER.error(
- "TESTv%s - Finalizing session with GET to /index.cgi", self.api_version
- )
- with contextlib.suppress(Exception):
+ _LOGGER.debug("Finalizing v6 session (api_version=%s)", self.api_version)
+ try:
await self._request_json(
"GET",
f"{self.base_url}/index.cgi",
headers={
"Referer": f"{self.base_url}/login.cgi",
"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36",
},
authenticated=True,
allow_redirects=True,
)
+ except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err:
+ _LOGGER.debug("Session finalization failed (non-fatal): %s", err)
return # Success🤖 Prompt for AI Agents
In airos/base.py around lines 437 to 454, the finalization block currently
suppresses all Exceptions with contextlib.suppress(Exception) which is too
broad; replace it with a narrower try/except that only handles expected
network/HTTP errors (for example aiohttp.ClientError, asyncio.TimeoutError, and
any JSON/parse errors your _request_json may raise), log the caught exception
details using _LOGGER.error with contextual message (include self.api_version
and the exception), and re-raise any unexpected exceptions so they aren’t
silently swallowed; keep the GET to /index.cgi and the final return on success
unchanged.
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
airos/base.py (1)
506-516: Fix content-type header mismatch in update_check(force=True).At line 514,
json_data={"force": True}is sent withct_form=True, which sets the header toapplication/x-www-form-urlencoded. This mismatches the JSON body. The force=False branch (line 521) correctly usesct_json=Truefor JSON data.Change line 514 from
ct_form=Truetoct_json=True.
♻️ Duplicate comments (10)
airos/base.py (10)
315-338: Use debug for diagnostics; keep timestamping but reduce noise.These are normal-flow logs.
- if self.api_version == 6 and url.startswith(self._status_cgi_url): - _LOGGER.error( - "TESTv%s - adding timestamp to status url!", self.api_version - ) + if self.api_version == 6 and url.startswith(self._status_cgi_url): + _LOGGER.debug("Adding timestamp to status url (v%s)", self.api_version) timestamp = int(time.time() * 1000) url = f"{self._status_cgi_url}?_={timestamp}" - _LOGGER.error("TESTv%s - Trying with URL: %s", self.api_version, url) + _LOGGER.debug("Requesting URL: %s (api_version=%s)", url, self.api_version) ... - _LOGGER.error( - "TESTv%s - Response code: %s", self.api_version, response.status - ) - _LOGGER.error( - "TESTv%s - Response headers: %s", - self.api_version, - dict(response.headers), - ) + _LOGGER.debug("Response status: %s (api_version=%s)", response.status, self.api_version) + _LOGGER.debug("Response headers: %s (api_version=%s)", dict(response.headers), self.api_version)
428-456: Downgrade v6 attempt logs to debug and fix message mismatch.These aren’t errors; “extended multipart” text is wrong in simple attempt.
- _LOGGER.error("TESTv%s - start v6 attempts", self.api_version) + _LOGGER.debug("Starting v6 login attempts (api_version=%s)", self.api_version) # --- ATTEMPT B: Simple Payload (multipart/form-data) --- try: - _LOGGER.error( - "TESTv%s - Trying V6 POST to %s with SIMPLE multipart/form-data", - self.api_version, - self._login_urls["v6_login"], - ) + _LOGGER.debug("Attempting v6 simple multipart login (api_version=%s)", self.api_version) ... - except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err: - _LOGGER.error( - "TESTv%s - V6 simple multipart failed (%s) on %s. Error: %s", - self.api_version, - type(err).__name__, - self._login_urls["v6_login"], - err, - ) + except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err: + _LOGGER.debug("v6 simple multipart failed (%s): %s (api_version=%s)", type(err).__name__, err, self.api_version) except AirOSConnectionAuthenticationError: - _LOGGER.error( - "TESTv%s - autherror during extended multipart", self.api_version - ) + _LOGGER.debug("v6 login auth error (simple multipart) (api_version=%s)", self.api_version) raise
380-399: Downgrade login flow logs to debug.Normal-control messages shouldn’t be error-level.
- _LOGGER.error("TESTv%s - Trying default v8 login URL", self.api_version) + _LOGGER.debug("Attempting v8 login (api_version=%s)", self.api_version) ... - _LOGGER.error( - "TESTv%s - gives URL not found, trying alternative v6 URL", - self.api_version, - ) + _LOGGER.debug("v8 login not found, trying v6 login (api_version=%s)", self.api_version) ... - _LOGGER.error("TESTv%s - failed to login to v8 URL", self.api_version) + _LOGGER.debug("v8 login failed (api_version=%s)", self.api_version) ... - _LOGGER.error("TESTv%s - returning from v8 login", self.api_version) + _LOGGER.debug("v8 login succeeded (api_version=%s)", self.api_version)
291-305: Header typo breaks referer; use “Referer”, not “Referrer”.“Referrer” is ignored by servers.
- request_headers["Referrer"] = f"{self.base_url}/login.cgi" + request_headers["Referer"] = f"{self.base_url}/login.cgi"
401-417: Cookie prefetch: don’t suppress all exceptions; and it must be unauthenticated.Broad suppression hides real bugs;
authenticated=Trueblocks before login.- _LOGGER.error( - "TESTv%s - Trying to get /index.cgi first for cookies", self.api_version - ) - with contextlib.suppress(Exception): + _LOGGER.debug("Prefetching cookies from /index.cgi (api_version=%s)", self.api_version) + try: cookieresponse = await self._request_json( "GET", f"{self.base_url}/index.cgi", - authenticated=True, + authenticated=False, headers={ "Referer": f"{self.base_url}/login.cgi", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", }, ) - _LOGGER.error( - "TESTv%s - Cookie response: %s", self.api_version, cookieresponse - ) + _LOGGER.debug("Cookie prefetch response: %s (api_version=%s)", cookieresponse, self.api_version) + except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)
458-474: Finalization: avoid broad suppression; use narrow try/except and debug logs.Don’t hide unexpected errors.
- else: - _LOGGER.error("TESTv%s - returning from simple multipart", self.api_version) - # Finalize session by visiting /index.cgi - _LOGGER.error( - "TESTv%s - Finalizing session with GET to /index.cgi", self.api_version - ) - with contextlib.suppress(Exception): + else: + _LOGGER.debug("v6 simple multipart succeeded (api_version=%s)", self.api_version) + _LOGGER.debug("Finalizing v6 session (api_version=%s)", self.api_version) + try: await self._request_json( "GET", f"{self.base_url}/index.cgi", headers={ "Referer": f"{self.base_url}/login.cgi", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", }, authenticated=True, allow_redirects=True, ) + except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Session finalization failed (non-fatal): %s", err) return # Success
207-218: Stop logging sensitive tokens/cookies; redact and downgrade to debug.CSRF and auth cookie values are logged at error level. This leaks secrets.
Apply:
- if self._csrf_id: # pragma: no cover - _LOGGER.error("TESTv%s - CSRF ID found %s", self.api_version, self._csrf_id) + if self._csrf_id: # pragma: no cover + _LOGGER.debug("Using CSRF token (api_version=%s)", self.api_version) headers["X-CSRF-ID"] = self._csrf_id - if self._auth_cookie: # pragma: no cover - _LOGGER.error( - "TESTv%s - auth_cookie found: AIROS_%s", - self.api_version, - self._auth_cookie, - ) + if self._auth_cookie: # pragma: no cover + cookie_name = self._auth_cookie.split("=", 1)[0] if "=" in self._auth_cookie else "***" + _LOGGER.debug("Using auth cookie: AIROS_%s=*** (api_version=%s)", cookie_name, self.api_version) headers["Cookie"] = f"AIROS_{self._auth_cookie}"
340-360: Fix inverted v6 login check, avoid duplicate auth handling, and don’t JSON-decode v6 login.Currently sets api_version=6 for non-login URLs and stores auth twice.
- # v6 responds with a 302 redirect and empty body - if not url.startswith(self._login_urls["v6_login"]): - self.api_version = 6 - response.raise_for_status() + # v6 login may 302/empty; other URLs should enforce status + if url.startswith(self._login_urls["v6_login"]): + self.api_version = 6 + # don't raise_for_status() on v6 login + else: + response.raise_for_status() response_text = await response.text() - _LOGGER.error("Successfully fetched %s from %s", response_text, url) + _LOGGER.debug("Fetched response from %s", url) # If this is the login request, we need to store the new auth data if url in self._login_urls.values(): self._store_auth_data(response) self.connected = True - _LOGGER.error("TESTv%s - response: %s", self.api_version, response_text) - # V6 responds with empty body on login, not JSON - if url.startswith(self._login_urls["v6_login"]): - self._store_auth_data(response) - self.connected = True - return {} + _LOGGER.debug("Response body (first 200): %s", response_text[:200]) + # v6 login often empty/non-JSON + if url.startswith(self._login_urls["v6_login"]): + return {} if not response_text.strip() else response_text return json.loads(response_text)
306-314: Blocker: Auth gate must respectauthenticated; currently breaks pre-login flows.Only gate requests that require auth; allow root/unauthenticated prefetch.
- if ( - url not in self._login_urls.values() - and url != f"{self.base_url}/" - and not self.connected - ): + if ( + authenticated + and url not in self._login_urls.values() + and url != f"{self.base_url}/" + and not self.connected + ): _LOGGER.error("Not connected, login first") raise AirOSDeviceConnectionError from None
228-239: Redact Set-Cookie logging; avoid printing values.Do not emit raw Set-Cookie headers or cookie values.
- for set_cookie in response.headers.getall("Set-Cookie", []): - _LOGGER.error( - "TESTv%s - regular cookie handling: %s", self.api_version, set_cookie - ) + for set_cookie in response.headers.getall("Set-Cookie", []): + _LOGGER.debug("Cookie header received (api_version=%s)", self.api_version) cookie.load(set_cookie) - for key, morsel in cookie.items(): - _LOGGER.error( - "TESTv%s - AIROS_cookie handling: %s with %s", - self.api_version, - key, - morsel.value, - ) + for key, morsel in cookie.items(): + _LOGGER.debug("Cookie set: %s=*** (api_version=%s)", key, self.api_version) if key.startswith("AIROS_"): self._auth_cookie = morsel.key[6:] + "=" + morsel.value break
🧹 Nitpick comments (3)
airos/base.py (3)
265-305: Trim UA header spoofing; fix Host fallback and language.
- Hard-coded Host fallback “192.168.1.142” is brittle.
Accept-Language: plis surprising; prefer neutral/en-US or omit.- Minimal headers are usually enough (Accept, X-Requested-With, Referer).
- request_headers["Host"] = ( - urlparse(self.base_url).hostname or "192.168.1.142" - ) + host = urlparse(self.base_url).hostname or "" + if host: + request_headers["Host"] = host - request_headers["Accept-Language"] = "pl" + # request_headers["Accept-Language"] = "en-US,en;q=0.9"Would you like me to reduce these headers to the minimum set validated against v6 hardware?
419-423: FormData reuse: safe here, but be cautious across retries.It’s created once and used in a single POST, so OK. If you add retries/alternate URLs, rebuild FormData per attempt.
Also applies to: 436-443
419-474: Add tests for v6 multipart login path.Verification confirms no tests exist for the v6 multipart login code (airos/base.py lines 419-474). The single existing form_data test handles plain dicts, not aiohttp.FormData objects. Tests should verify FormData construction with fields "uri", "username", and "password", and assert multipart request is sent to /login.cgi.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airos/base.py(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
🔇 Additional comments (2)
airos/base.py (2)
478-484: Status headers: LGTM.Minimal, aligned with v6/XHR expectations.
249-254: Plumbing for multipart support: LGTM.
form_dataandallow_redirectsthreading look correct.Also applies to: 327-330
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (8)
airos/base.py (8)
291-291: CRITICAL: HTTP header typo breaks Referer header.Line 291 uses
"Referrer"which is incorrect. The HTTP header is spelled"Referer"(note the misspelling is part of the standard). This header will be ignored by servers, potentially breaking v6 login.Apply this diff:
- request_headers["Referrer"] = f"{self.base_url}/login.cgi" + request_headers["Referer"] = f"{self.base_url}/login.cgi"
475-492: MAJOR: Overly broad exception suppression in session finalization.Lines 481-491 use
contextlib.suppress(Exception)which is too broad. Also, lines 476-480 log at error level.Apply this diff:
else: - _LOGGER.error("TESTv%s - returning from simple multipart", self.api_version) + _LOGGER.debug("v6 simple multipart succeeded (api_version=%s)", self.api_version) # Finalize session by visiting /index.cgi - _LOGGER.error( - "TESTv%s - Finalizing session with GET to /index.cgi", self.api_version - ) - with contextlib.suppress(Exception): + _LOGGER.debug("Finalizing v6 session (api_version=%s)", self.api_version) + try: await self._request_json( "GET", f"{self.base_url}/index.cgi", headers={ "Referer": f"{self.base_url}/login.cgi", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", }, authenticated=True, allow_redirects=True, ) + except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Session finalization failed (non-fatal): %s", err) return # Success
444-474: MINOR: Use debug level and fix message mismatch.Lines 447-451, 463-469, and 471-473 log normal login flow at error level. Line 472 says "extended multipart" but this is the simple multipart attempt block.
Apply this diff:
- _LOGGER.error("TESTv%s - start v6 attempts", self.api_version) + _LOGGER.debug("Starting v6 login attempts (api_version=%s)", self.api_version) # --- ATTEMPT B: Simple Payload (multipart/form-data) --- try: - _LOGGER.error( - "TESTv%s - Trying V6 POST to %s with SIMPLE multipart/form-data", - self.api_version, - self._login_urls["v6_login"], - ) + _LOGGER.debug("Attempting v6 simple multipart login (api_version=%s)", self.api_version) await self._request_json( ... ) except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err: - _LOGGER.error( - "TESTv%s - V6 simple multipart failed (%s) on %s. Error: %s", - self.api_version, - type(err).__name__, - self._login_urls["v6_login"], - err, - ) + _LOGGER.debug( + "v6 simple multipart failed (%s): %s (api_version=%s)", + type(err).__name__, + err, + self.api_version, + ) except AirOSConnectionAuthenticationError: - _LOGGER.error( - "TESTv%s - autherror during extended multipart", self.api_version - ) + _LOGGER.debug("v6 login auth error (simple multipart) (api_version=%s)", self.api_version) raise
208-217: CRITICAL: Sensitive authentication data logged at error level without redaction.Lines 208-209 and 212-217 log CSRF tokens and authentication cookies at error level with full values exposed. This is a security risk that can leak credentials in production logs.
Apply this diff to use debug level and redact sensitive values:
if self._csrf_id: # pragma: no cover - _LOGGER.error("TESTv%s - CSRF ID found %s", self.api_version, self._csrf_id) + _LOGGER.debug("Using CSRF token (api_version=%s)", self.api_version) headers["X-CSRF-ID"] = self._csrf_id if self._auth_cookie: # pragma: no cover - _LOGGER.error( - "TESTv%s - auth_cookie found: AIROS_%s", - self.api_version, - self._auth_cookie, - ) + cookie_name = self._auth_cookie.split("=", 1)[0] if "=" in self._auth_cookie else "***" + _LOGGER.debug("Using auth cookie: AIROS_%s=*** (api_version=%s)", cookie_name, self.api_version) headers["Cookie"] = f"AIROS_{self._auth_cookie}"
322-342: MINOR: Use debug level for diagnostic request/response logs.Lines 322, 331-341 log diagnostic information (URL, response status, headers, history) at error level. These are not errors but informational messages for debugging.
Apply this diff:
- _LOGGER.error("TESTv%s - Trying with URL: %s", self.api_version, url) + _LOGGER.debug("Requesting URL: %s (api_version=%s)", url, self.api_version) async with self.session.request( ... ) as response: - _LOGGER.error( - "TESTv%s - Response code: %s", self.api_version, response.status - ) - _LOGGER.error( - "TESTv%s - Response headers: %s", - self.api_version, - dict(response.headers), - ) - _LOGGER.error( - "TESTv%s - Response history: %s", self.api_version, response.history - ) + _LOGGER.debug("Response status: %s (api_version=%s)", response.status, self.api_version) + _LOGGER.debug("Response headers: %s (api_version=%s)", dict(response.headers), self.api_version) + _LOGGER.debug("Response history: %s (api_version=%s)", response.history, self.api_version)
307-313: MAJOR: Auth gate should check authenticated flag.The auth gate at lines 307-311 raises an error for any non-login, non-root URL when not connected, regardless of whether the request requires authentication. This can block legitimate unauthenticated requests.
Apply this diff to only gate authenticated requests:
if ( + authenticated + and url not in self._login_urls.values() - url not in self._login_urls.values() and url != f"{self.base_url}/" and not self.connected ):
343-368: CRITICAL: Inverted v6 login check and duplicate auth handling.Line 344 uses
if not url.startswith(...)which is backwards—it setsapi_version=6for all URLs EXCEPT the v6 login endpoint. Lines 365-368 duplicate the auth storage already handled at lines 359-361.Apply this diff to fix the logic:
- # v6 responds with a 302 redirect and empty body - if not url.startswith(self._login_urls["v6_login"]): - self.api_version = 6 - response.raise_for_status() + # v6 login may respond with a 302 redirect; other endpoints should raise + if url.startswith(self._login_urls["v6_login"]): + self.api_version = 6 + # Don't raise_for_status on v6 login (302 is expected) + else: + response.raise_for_status() response_text = await response.text() - _LOGGER.error("Successfully fetched %s from %s", response_text, url) + _LOGGER.debug("Fetched response from %s", url) if not response_text.strip(): - _LOGGER.error( - "TESTv%s - Empty response from %s despite %s", - self.api_version, - url, - response.status, - ) + _LOGGER.debug( + "Empty response from %s (status=%s, api_version=%s)", + url, + response.status, + self.api_version, + ) # If this is the login request, we need to store the new auth data if url in self._login_urls.values(): self._store_auth_data(response) self.connected = True - - _LOGGER.error("TESTv%s - response: %s", self.api_version, response_text) - # V6 responds with empty body on login, not JSON - if url.startswith(self._login_urls["v6_login"]): - self._store_auth_data(response) - self.connected = True return {} return json.loads(response_text)
411-427: MAJOR: Overly broad exception suppression in cookie prefetch.Lines 415-427 use
contextlib.suppress(Exception)which is too broad and will hide programming errors, network issues, and other failures. This makes debugging difficult.Apply this diff to catch specific exceptions:
# Start of v6, go for cookies - _LOGGER.error( - "TESTv%s - Trying to get /index.cgi first for cookies", self.api_version - ) - with contextlib.suppress(Exception): + _LOGGER.debug("Prefetching cookies from /index.cgi (api_version=%s)", self.api_version) + try: cookieresponse = await self._request_json( "GET", f"{self.base_url}/index.cgi", authenticated=True, headers={ "Referer": f"{self.base_url}/login.cgi", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", }, ) - _LOGGER.error( - "TESTv%s - Cookie response: %s", self.api_version, cookieresponse - ) + _LOGGER.debug("Cookie prefetch response: %s (api_version=%s)", cookieresponse, self.api_version) + except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airos/base.py(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
| if self.api_version == 6 and url.startswith(self._status_cgi_url): | ||
| # Ensure all HAR-matching headers are present | ||
| request_headers["Accept"] = "application/json, text/javascript, */*; q=0.01" | ||
| request_headers["Accept-Encoding"] = "gzip, deflate, br, zstd" | ||
| request_headers["Accept-Language"] = "pl" | ||
| request_headers["Cache-Control"] = "no-cache" | ||
| request_headers["Connection"] = "keep-alive" | ||
| request_headers["Host"] = ( | ||
| urlparse(self.base_url).hostname or "192.168.1.142" | ||
| ) | ||
| request_headers["Pragma"] = "no-cache" | ||
| request_headers["Referer"] = f"{self.base_url}/index.cgi" | ||
| request_headers["Sec-Fetch-Dest"] = "empty" | ||
| request_headers["Sec-Fetch-Mode"] = "cors" | ||
| request_headers["Sec-Fetch-Site"] = "same-origin" | ||
| request_headers["User-Agent"] = ( | ||
| "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36" | ||
| ) | ||
| request_headers["X-Requested-With"] = "XMLHttpRequest" | ||
| request_headers["sec-ch-ua"] = ( | ||
| '"Google Chrome";v="141", "Not?A_Brand";v="8", "Chromium";v="141"' | ||
| ) | ||
| request_headers["sec-ch-ua-mobile"] = "?0" | ||
| request_headers["sec-ch-ua-platform"] = '"Windows"' |
There was a problem hiding this comment.
Minor: Hardcoded IP fallback in Host header.
Line 274 uses a hardcoded IP "192.168.1.142" as a fallback for the Host header. This appears to be a development artifact and should use a generic default or the actual parsed hostname.
Apply this diff:
request_headers["Host"] = (
- urlparse(self.base_url).hostname or "192.168.1.142"
+ urlparse(self.base_url).hostname or "device.local"
)🤖 Prompt for AI Agents
In airos/base.py around lines 266 to 289, the Host header fallback currently
uses a hardcoded IP ("192.168.1.142"); replace this development artifact with a
generic default by using the parsed hostname from urlparse(self.base_url) and
falling back to a neutral value such as "localhost" (or raise/log an error if
hostname is unexpectedly missing) so the Host header isn't tied to a specific
dev IP.
| _LOGGER.error("TESTv%s - Trying default v8 login URL", self.api_version) | ||
| await self._request_json( | ||
| "POST", self._login_urls["default"], json_data=payload | ||
| ) | ||
| except AirOSUrlNotFoundError: | ||
| pass # Try next URL | ||
| _LOGGER.error( | ||
| "TESTv%s - gives URL not found, trying alternative v6 URL", | ||
| self.api_version, | ||
| ) | ||
| # Try next URL | ||
| except AirOSConnectionSetupError as err: | ||
| _LOGGER.error("TESTv%s - failed to login to v8 URL", self.api_version) | ||
| raise AirOSConnectionSetupError("Failed to login to AirOS device") from err | ||
| else: | ||
| _LOGGER.error("TESTv%s - returning from v8 login", self.api_version) |
There was a problem hiding this comment.
MINOR: Use debug level for login flow logs.
Lines 394, 399-402, 405, and 408 log normal login flow information at error level. These should be debug level.
Apply this diff:
- _LOGGER.error("TESTv%s - Trying default v8 login URL", self.api_version)
+ _LOGGER.debug("Attempting v8 login (api_version=%s)", self.api_version)
await self._request_json(
"POST", self._login_urls["default"], json_data=payload
)
except AirOSUrlNotFoundError:
- _LOGGER.error(
- "TESTv%s - gives URL not found, trying alternative v6 URL",
- self.api_version,
- )
+ _LOGGER.debug("v8 login not found, attempting v6 login (api_version=%s)", self.api_version)
# Try next URL
except AirOSConnectionSetupError as err:
- _LOGGER.error("TESTv%s - failed to login to v8 URL", self.api_version)
+ _LOGGER.debug("v8 login failed (api_version=%s)", self.api_version)
raise AirOSConnectionSetupError("Failed to login to AirOS device") from err
else:
- _LOGGER.error("TESTv%s - returning from v8 login", self.api_version)
+ _LOGGER.debug("v8 login succeeded (api_version=%s)", self.api_version)
return🤖 Prompt for AI Agents
In airos/base.py around lines 394 to 408 the normal login flow messages are
logged at error level; change those _LOGGER.error calls to _LOGGER.debug so
routine flow (trying default v8 URL, v6 fallback notice, failed-to-login message
that is part of flow tracing, and returning-from-v8-login) are logged at debug
level instead of error; keep the messages and exception raise behavior intact,
only replace the logging calls' level to debug.
| _LOGGER.debug( | ||
| "TESTv%s !!!REDACT THIS!!!! Form payload: %s", | ||
| self.api_version, | ||
| v6_simple_multipart_form_data(), | ||
| ) |
There was a problem hiding this comment.
CRITICAL: Form payload with password logged.
Lines 434-438 log the form payload at debug level. Even though it says "REDACT THIS", the code still executes v6_simple_multipart_form_data() which will expose the password in logs.
Apply this diff to remove the password logging:
- _LOGGER.debug(
- "TESTv%s !!!REDACT THIS!!!! Form payload: %s",
- self.api_version,
- v6_simple_multipart_form_data(),
- )
+ _LOGGER.debug("Prepared v6 login form data (api_version=%s)", self.api_version)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _LOGGER.debug( | |
| "TESTv%s !!!REDACT THIS!!!! Form payload: %s", | |
| self.api_version, | |
| v6_simple_multipart_form_data(), | |
| ) | |
| _LOGGER.debug("Prepared v6 login form data (api_version=%s)", self.api_version) |
🤖 Prompt for AI Agents
In airos/base.py around lines 434 to 438 the debug log calls
v6_simple_multipart_form_data() which constructs and exposes the password;
remove the sensitive call from the logging path so the password is never
computed or logged. Replace the log with a non-sensitive message (e.g., log that
the form payload was redacted or omitted) or, if you must log payload structure,
compute a sanitized copy that strips or masks the password before logging;
ensure the original v6_simple_multipart_form_data() is not invoked as part of
the log statement.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (12)
airos/base.py (12)
267-291: Replace hardcoded IP with generic default.Line 276 uses a hardcoded development IP
"192.168.1.142"as the Host header fallback. This should use a generic default.Apply this diff:
request_headers["Host"] = ( - urlparse(self.base_url).hostname or "192.168.1.142" + urlparse(self.base_url).hostname or "device.local" )Based on learnings.
511-515: Log message mismatch: says "extended" in simple attempt.Line 513 logs "autherror during extended multipart" but this except block handles the simple multipart attempt, not extended.
Apply this diff:
except AirOSConnectionAuthenticationError: - _LOGGER.error( - "TESTv%s - autherror during extended multipart", self.api_version - ) + _LOGGER.debug("v6 login auth error (simple multipart) (api_version=%s)", self.api_version) raiseBased on learnings.
228-243: CRITICAL: Redact sensitive cookie values in logs.Lines 230-232 and 235-240 log full cookie values at error level. Cookie contents are sensitive session data and must not be exposed in logs.
Apply this diff to use debug level and redact values:
for set_cookie in response.headers.getall("Set-Cookie", []): - _LOGGER.error( - "TESTv%s - regular cookie handling: %s", self.api_version, set_cookie - ) + _LOGGER.debug("Processing Set-Cookie header (api_version=%s)", self.api_version) cookie.load(set_cookie) for key, morsel in cookie.items(): - _LOGGER.error( - "TESTv%s - AIROS_cookie handling: %s with %s", - self.api_version, - key, - morsel.value, - ) + _LOGGER.debug("Found cookie: %s=*** (api_version=%s)", key, self.api_version) if key.startswith("AIROS_"): self._auth_cookie = morsel.key[6:] + "=" + morsel.value breakBased on learnings.
475-479: CRITICAL: Form payload with password logged.Lines 475-479 log the form payload at debug level. Even with the "REDACT THIS" comment, line 478 executes
v6_simple_multipart_form_data()which exposes the password in logs.Apply this diff to remove password logging:
- _LOGGER.debug( - "TESTv%s !!!REDACT THIS!!!! Form payload: %s", - self.api_version, - v6_simple_multipart_form_data(), - ) + _LOGGER.debug("Prepared v6 login form data (api_version=%s)", self.api_version)Based on learnings.
422-438: Use debug level for login flow logs.Lines 423, 428-431, 434, and 437 log normal login attempt flow at error level. These are informational debug messages.
Apply this diff:
try: - _LOGGER.error("TESTv%s - Trying default v8 login URL", self.api_version) + _LOGGER.debug("Attempting v8 login (api_version=%s)", self.api_version) await self._request_json( "POST", self._login_urls["default"], json_data=payload ) except AirOSUrlNotFoundError: - _LOGGER.error( - "TESTv%s - gives URL not found, trying alternative v6 URL", - self.api_version, - ) + _LOGGER.debug("v8 login not found, attempting v6 login (api_version=%s)", self.api_version) # Try next URL except AirOSConnectionSetupError as err: - _LOGGER.error("TESTv%s - failed to login to v8 URL", self.api_version) + _LOGGER.debug("v8 login failed (api_version=%s)", self.api_version) raise AirOSConnectionSetupError("Failed to login to AirOS device") from err else: - _LOGGER.error("TESTv%s - returning from v8 login", self.api_version) + _LOGGER.debug("v8 login succeeded (api_version=%s)", self.api_version) returnBased on learnings.
485-510: Use debug level for v6 login attempt logs.Lines 485, 488-492, and 504-510 log normal v6 login attempt flow at error level. These are informational debug messages.
Apply this diff:
- _LOGGER.error("TESTv%s - start v6 attempts", self.api_version) + _LOGGER.debug("Starting v6 login attempts (api_version=%s)", self.api_version) # --- ATTEMPT B: Simple Payload (multipart/form-data) --- try: - _LOGGER.error( - "TESTv%s - Trying V6 POST to %s with SIMPLE multipart/form-data", - self.api_version, - self._login_urls["v6_login"], - ) + _LOGGER.debug("Attempting v6 simple multipart login (api_version=%s)", self.api_version) await self._request_json( ... ) except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err: - _LOGGER.error( - "TESTv%s - V6 simple multipart failed (%s) on %s. Error: %s", - self.api_version, - type(err).__name__, - self._login_urls["v6_login"], - err, - ) + _LOGGER.debug("v6 simple multipart failed (%s): %s (api_version=%s)", + type(err).__name__, err, self.api_version)Based on learnings.
516-533: Overly broad exception suppression in session finalization.Lines 522-532 use
contextlib.suppress(Exception)which is too broad and will hide all errors including programming errors. Lines 517-521 log at error level.Apply this diff:
else: - _LOGGER.error("TESTv%s - returning from simple multipart", self.api_version) + _LOGGER.debug("v6 simple multipart succeeded (api_version=%s)", self.api_version) # Finalize session by visiting /index.cgi - _LOGGER.error( - "TESTv%s - Finalizing session with GET to /index.cgi", self.api_version - ) - with contextlib.suppress(Exception): + _LOGGER.debug("Finalizing v6 session (api_version=%s)", self.api_version) + try: await self._request_json( "GET", f"{self.base_url}/index.cgi", headers={ "Referer": f"{self.base_url}/login.cgi", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", }, authenticated=True, allow_redirects=True, ) + except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Session finalization failed (non-fatal): %s", err) return # SuccessBased on learnings.
208-220: CRITICAL: Redact sensitive auth data in logs.Lines 209 and 213-217 log full CSRF tokens and authentication cookies at error level. This exposes sensitive credentials in production logs and violates security best practices.
Apply this diff to use debug level and redact sensitive values:
if self._csrf_id: # pragma: no cover - _LOGGER.error("TESTv%s - CSRF ID found %s", self.api_version, self._csrf_id) + _LOGGER.debug("Using CSRF token (api_version=%s)", self.api_version) headers["X-CSRF-ID"] = self._csrf_id if self._auth_cookie: # pragma: no cover - _LOGGER.error( - "TESTv%s - auth_cookie found: AIROS_%s", - self.api_version, - self._auth_cookie, - ) - # headers["Cookie"] = f"AIROS_{self._auth_cookie}" - headers["Cookie"] = self._auth_cookie + cookie_name = self._auth_cookie.split("=", 1)[0] if "=" in self._auth_cookie else "session" + _LOGGER.debug("Using auth cookie: %s=*** (api_version=%s)", cookie_name, self.api_version) + headers["Cookie"] = self._auth_cookieBased on learnings.
309-316: Auth gate blocks legitimate unauthenticated requests.The connection check doesn't respect the
authenticatedparameter. It blocks all requests to non-login URLs when not connected, even when the caller explicitly passesauthenticated=False(e.g., cookie prefetch requests).Apply this diff to only enforce the gate for authenticated requests:
- if ( - url not in self._login_urls.values() - and url != f"{self.base_url}/" - and not self.connected - ): + if ( + authenticated + and url not in self._login_urls.values() + and url != f"{self.base_url}/" + and not self.connected + ): _LOGGER.error("Not connected, login first") raise AirOSDeviceConnectionError from NoneBased on learnings.
350-369: CRITICAL: Inverted v6 login check sets wrong API version.Line 351 uses
if not url.startswith(self._login_urls["v6_login"])which is backwards. This setsapi_version=6for ALL URLs EXCEPT the v6 login endpoint, then raises for those URLs. The logic should identify v6 login responses and handle them specially.Apply this diff:
- # v6 responds with a 302 redirect and empty body - if not url.startswith(self._login_urls["v6_login"]): - self.api_version = 6 - response.raise_for_status() + # v6 login may respond with a 302 redirect; other endpoints should raise + if url.startswith(self._login_urls["v6_login"]): + self.api_version = 6 + # Don't raise_for_status on v6 login (302 is expected) + else: + response.raise_for_status() response_text = await response.text() - _LOGGER.error("Successfully fetched %s from %s", response_text, url) - if not response_text.strip(): - _LOGGER.error( - "TESTv%s - Empty response from %s despite %s", - self.api_version, - url, - response.status, - ) + _LOGGER.debug("Fetched response from %s (api_version=%s)", url, self.api_version) + if not response_text.strip(): + _LOGGER.debug("Empty response from %s with status %s (api_version=%s)", + url, response.status, self.api_version)Based on learnings.
292-307: CRITICAL: Fix HTTP header typo - use "Referer" not "Referrer".Line 293 sets the header
"Referrer"which is incorrect and will be ignored by servers. The correct HTTP header name is"Referer"(note the misspelling is part of the HTTP standard).Apply this diff:
if url.startswith(self._login_urls["v6_login"]): - request_headers["Referrer"] = f"{self.base_url}/login.cgi" + request_headers["Referer"] = f"{self.base_url}/login.cgi" request_headers["Origin"] = self.base_urlBased on learnings.
440-469: Overly broad exception suppression and questionable auth flag.Lines 444-468 use
contextlib.suppress(Exception)which is too broad and will hide programming errors, network issues, and other failures that should be logged. Additionally, line 448 passesauthenticated=Truefor what appears to be a preflight cookie request.Apply this diff to narrow exception handling and use debug logging:
# Start of v6, go for cookies - _LOGGER.error( - "TESTv%s - Trying to get /index.cgi first for cookies", self.api_version - ) - with contextlib.suppress(Exception): + _LOGGER.debug("Prefetching cookies from /index.cgi (api_version=%s)", self.api_version) + try: cookieresponse = await self._request_json( "GET", f"{self.base_url}/index.cgi", - authenticated=True, + authenticated=False, headers={ "Referer": f"{self.base_url}/login.cgi", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", }, ) - _LOGGER.error( - "TESTv%s - Cookie response: %s", self.api_version, cookieresponse - ) + _LOGGER.debug("Cookie prefetch response: %s (api_version=%s)", cookieresponse, self.api_version) if isinstance(cookieresponse, aiohttp.ClientResponse): _LOGGER.debug( "TESTv%s - Finalization redirect chain: %s", self.api_version, cookieresponse.history, ) else: _LOGGER.debug( "TESTv%s - Finalization response is not a ClientResponse: %s", self.api_version, type(cookieresponse), ) + except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)Based on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airos/base.py(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Check commit
🔇 Additional comments (1)
airos/base.py (1)
535-563: Status method implementation looks good.The addition of explicit
status_headersand passing them to_request_jsonis correct. Error handling for deserialization with appropriate redaction is well-implemented.
| if self.api_version == 6 and url.startswith(self._status_cgi_url): | ||
| _LOGGER.error( | ||
| "TESTv%s - adding timestamp to status url!", self.api_version | ||
| ) | ||
| timestamp = int(time.time() * 1000) | ||
| url = f"{self._status_cgi_url}?_={timestamp}" | ||
|
|
||
| _LOGGER.error("TESTv%s - Trying with URL: %s", self.api_version, url) | ||
| async with self.session.request( | ||
| method, | ||
| url, | ||
| json=json_data, | ||
| data=form_data, | ||
| headers=request_headers, # Pass the constructed headers | ||
| allow_redirects=allow_redirects, | ||
| ) as response: | ||
| response.raise_for_status() | ||
| _LOGGER.error( | ||
| "TESTv%s - Response code: %s", self.api_version, response.status | ||
| ) | ||
| _LOGGER.error( | ||
| "TESTv%s - Response headers: %s", | ||
| self.api_version, | ||
| dict(response.headers), | ||
| ) | ||
| _LOGGER.error( | ||
| "TESTv%s - Response history: %s", self.api_version, response.history | ||
| ) | ||
| _LOGGER.error( | ||
| "TESTv%s - Session cookies: %s", | ||
| self.api_version, | ||
| self.session.cookie_jar.filter_cookies(URL(url)), | ||
| ) |
There was a problem hiding this comment.
Use debug level for diagnostic logs.
Lines 318-320, 324, and 333-348 log diagnostic information (timestamp addition, request URL, response status/headers/history/cookies) at error level. These are informational debug messages, not errors.
Apply this diff:
if self.api_version == 6 and url.startswith(self._status_cgi_url):
- _LOGGER.error(
- "TESTv%s - adding timestamp to status url!", self.api_version
- )
+ _LOGGER.debug("Adding timestamp to status URL (api_version=%s)", self.api_version)
timestamp = int(time.time() * 1000)
url = f"{self._status_cgi_url}?_={timestamp}"
- _LOGGER.error("TESTv%s - Trying with URL: %s", self.api_version, url)
+ _LOGGER.debug("Requesting URL: %s (api_version=%s)", url, self.api_version)
async with self.session.request(
...
) as response:
- _LOGGER.error(
- "TESTv%s - Response code: %s", self.api_version, response.status
- )
- _LOGGER.error(
- "TESTv%s - Response headers: %s",
- self.api_version,
- dict(response.headers),
- )
- _LOGGER.error(
- "TESTv%s - Response history: %s", self.api_version, response.history
- )
- _LOGGER.error(
- "TESTv%s - Session cookies: %s",
- self.api_version,
- self.session.cookie_jar.filter_cookies(URL(url)),
- )
+ _LOGGER.debug("Response status: %s (api_version=%s)", response.status, self.api_version)
+ _LOGGER.debug("Response headers: %s (api_version=%s)", dict(response.headers), self.api_version)
+ _LOGGER.debug("Response history: %s (api_version=%s)", response.history, self.api_version)
+ _LOGGER.debug("Session cookies: %s (api_version=%s)",
+ self.session.cookie_jar.filter_cookies(URL(url)), self.api_version)Based on learnings.
🤖 Prompt for AI Agents
In airos/base.py around lines 317 to 348, the diagnostic logs that currently use
_LOGGER.error (timestamp addition, request URL, response status, response
headers, response history, and session cookies) should be changed to use
_LOGGER.debug instead; replace those _LOGGER.error calls with _LOGGER.debug so
these informational messages are logged at debug level, preserving the message
text and interpolation arguments, and leave any actual error logging untouched.
| _LOGGER.error("TESTv%s - response: %s", self.api_version, response_text) | ||
|
|
||
| location = response.headers.get("Location") | ||
| if location and isinstance(location, str) and location.startswith("/"): | ||
| _LOGGER.error( | ||
| "TESTv%s - Following redirect to: %s", | ||
| self.api_version, | ||
| location, | ||
| ) | ||
| await self._request_json( | ||
| "GET", | ||
| f"{self.base_url}{location}", | ||
| headers={ | ||
| "Referer": self._login_urls["v6_login"], | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", | ||
| }, | ||
| authenticated=True, | ||
| allow_redirects=False, | ||
| ) | ||
| else: | ||
| _LOGGER.error( | ||
| "TESTv%s - no location header found to follow in response to %s", | ||
| self.api_version, | ||
| url, | ||
| ) | ||
| # V6 responds with empty body on login, not JSON | ||
| if url.startswith(self._login_urls["v6_login"]): | ||
| return {} | ||
|
|
There was a problem hiding this comment.
Use debug level for redirect and response logging.
Lines 370, 374-378, 390-394 log normal redirect-following flow at error level. These are informational debug messages.
Apply this diff:
- _LOGGER.error("TESTv%s - response: %s", self.api_version, response_text)
+ _LOGGER.debug("Response body (api_version=%s): %s", self.api_version, response_text[:200])
location = response.headers.get("Location")
if location and isinstance(location, str) and location.startswith("/"):
- _LOGGER.error(
- "TESTv%s - Following redirect to: %s",
- self.api_version,
- location,
- )
+ _LOGGER.debug("Following redirect to: %s (api_version=%s)", location, self.api_version)
await self._request_json(
...
)
else:
- _LOGGER.error(
- "TESTv%s - no location header found to follow in response to %s",
- self.api_version,
- url,
- )
+ _LOGGER.debug("No location header in response to %s (api_version=%s)", url, self.api_version)Based on learnings.
🤖 Prompt for AI Agents
In airos/base.py around lines 370 to 398, the code logs normal
redirect-following and response details at error level; change those
informational logs to use _LOGGER.debug instead of _LOGGER.error for the
response_text log, the "Following redirect to" block, and the "no location
header found" message so these become debug-level messages; keep behavior and
message text the same (just swap the logging method), ensuring
formatting/arguments remain unchanged and no other control flow is modified.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
airos/base.py (6)
281-283: Hardcoded development IP in Host header fallback.The fallback IP
"192.168.1.142"appears to be a development artifact and should use a generic default.Apply this diff:
request_headers["Host"] = ( - urlparse(self.base_url).hostname or "192.168.1.142" + urlparse(self.base_url).hostname or "device.local" )
213-214: Critical: CSRF token logged at error level without redaction.The CSRF token is a sensitive authentication credential and should not be logged at error level with its full value exposed.
Apply this diff:
if self._csrf_id: # pragma: no cover - _LOGGER.error("TESTv%s - CSRF ID found %s", self.api_version, self._csrf_id) + _LOGGER.debug("Using CSRF token (api_version=%s)", self.api_version) headers["X-CSRF-ID"] = self._csrf_id
481-485: Critical: Password exposed in debug log.The code logs the form payload which contains the password in plain text.
Apply this diff:
- _LOGGER.debug( - "TESTv%s !!!REDACT THIS!!!! Form payload: %s", - self.api_version, - v6_simple_multipart_form_data(), - ) + _LOGGER.debug("Prepared v6 login form data (api_version=%s)", self.api_version)
356-369: Critical: Inverted v6 login check sets wrong API version.Line 357 uses
if not url.startswith(...)which setsapi_version=6for all URLs EXCEPT the v6 login endpoint. This is backwards.Apply this diff:
- # v6 responds with a 302 redirect and empty body - if not url.startswith(self._login_urls["v6_login"]): - self.api_version = 6 - response.raise_for_status() + # v6 login may respond with a 302 redirect; other endpoints should raise + if url.startswith(self._login_urls["v6_login"]): + self.api_version = 6 + # Don't raise_for_status on v6 login (302 is expected) + else: + response.raise_for_status() response_text = await response.text() - _LOGGER.error("Successfully fetched %s from %s", response_text, url) + _LOGGER.debug("Fetched response from %s", url) if not response_text.strip(): - _LOGGER.error( - "TESTv%s - Empty response from %s despite %s", - self.api_version, - url, - response.status, - ) + _LOGGER.debug( + "Empty response from %s (status=%s, api_version=%s)", + url, + response.status, + self.api_version, + )
236-246: Critical: Sensitive cookie data logged at error level without redaction.Cookie values contain sensitive session data and should not be exposed in logs.
Apply this diff:
for set_cookie in response.headers.getall("Set-Cookie", []): - _LOGGER.error( - "TESTv%s - regular cookie handling: %s", self.api_version, set_cookie - ) + _LOGGER.debug("Processing Set-Cookie header (api_version=%s)", self.api_version) cookie.load(set_cookie) for key, morsel in cookie.items(): - _LOGGER.error( - "TESTv%s - AIROS_cookie handling: %s with %s", - self.api_version, - key, - morsel.value, - ) + _LOGGER.debug("Found cookie: %s=*** (api_version=%s)", key, self.api_version) if key.startswith("AIROS_"): self._auth_cookie = morsel.key[6:] + "=" + morsel.value break
523-539: Major: Overly broad exception suppression in session finalization.Line 528 uses
contextlib.suppress(Exception)which is too broad and will hide programming errors and other unexpected failures.Apply this diff:
else: - _LOGGER.error("TESTv%s - returning from simple multipart", self.api_version) + _LOGGER.debug("v6 simple multipart succeeded (api_version=%s)", self.api_version) # Finalize session by visiting /index.cgi - _LOGGER.error( - "TESTv%s - Finalizing session with GET to /index.cgi", self.api_version - ) - with contextlib.suppress(Exception): + _LOGGER.debug("Finalizing v6 session (api_version=%s)", self.api_version) + try: await self._request_json( "GET", f"{self.base_url}/index.cgi", headers={ "Referer": f"{self.base_url}/login.cgi", "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", }, authenticated=True, allow_redirects=True, ) + except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err: + _LOGGER.debug("Session finalization failed (non-fatal): %s", err) return # Success
🧹 Nitpick comments (5)
airos/base.py (5)
323-328: Use debug level for diagnostic log.This logs normal request flow information and should use debug level, not error.
Apply this diff:
if self.api_version == 6 and url.startswith(self._status_cgi_url): - _LOGGER.error( - "TESTv%s - adding timestamp to status url!", self.api_version - ) + _LOGGER.debug("Adding timestamp to status URL (api_version=%s)", self.api_version) timestamp = int(time.time() * 1000) url = f"{self._status_cgi_url}?_={timestamp}"Based on learnings.
330-354: Use debug level for diagnostic logs.These log normal request/response flow information and should use debug level, not error.
Apply this diff:
- _LOGGER.error("TESTv%s - Trying with URL: %s", self.api_version, url) + _LOGGER.debug("Requesting URL: %s (api_version=%s)", url, self.api_version) async with self.session.request( ... ) as response: - _LOGGER.error( - "TESTv%s - Response code: %s", self.api_version, response.status - ) - _LOGGER.error( - "TESTv%s - Response headers: %s", - self.api_version, - dict(response.headers), - ) - _LOGGER.error( - "TESTv%s - Response history: %s", self.api_version, response.history - ) - _LOGGER.error( - "TESTv%s - Session cookies: %s", - self.api_version, - self.session.cookie_jar.filter_cookies(URL(url)), - ) + _LOGGER.debug("Response status: %s (api_version=%s)", response.status, self.api_version) + _LOGGER.debug("Response headers: %s (api_version=%s)", dict(response.headers), self.api_version) + _LOGGER.debug("Response history: %s (api_version=%s)", response.history, self.api_version) + _LOGGER.debug("Session cookies: %s (api_version=%s)", + self.session.cookie_jar.filter_cookies(URL(url)), self.api_version)Based on learnings.
376-400: Use debug level for redirect flow logs.These log normal redirect-following and should use debug level, not error.
Apply this diff:
- _LOGGER.error("TESTv%s - response: %s", self.api_version, response_text) + _LOGGER.debug("Response body (api_version=%s): %s", self.api_version, response_text[:200]) location = response.headers.get("Location") if location and isinstance(location, str) and location.startswith("/"): - _LOGGER.error( - "TESTv%s - Following redirect to: %s", - self.api_version, - location, - ) + _LOGGER.debug("Following redirect to: %s (api_version=%s)", location, self.api_version) await self._request_json( ... ) else: - _LOGGER.error( - "TESTv%s - no location header found to follow in response to %s", - self.api_version, - url, - ) + _LOGGER.debug("No location header in response to %s (api_version=%s)", url, self.api_version)Based on learnings.
429-444: Use debug level for login flow logs.These log normal login attempt flow and should use debug level, not error.
Apply this diff:
- _LOGGER.error("TESTv%s - Trying default v8 login URL", self.api_version) + _LOGGER.debug("Attempting v8 login (api_version=%s)", self.api_version) await self._request_json( "POST", self._login_urls["default"], json_data=payload ) except AirOSUrlNotFoundError: - _LOGGER.error( - "TESTv%s - gives URL not found, trying alternative v6 URL", - self.api_version, - ) + _LOGGER.debug("v8 login not found, attempting v6 login (api_version=%s)", self.api_version) # Try next URL except AirOSConnectionSetupError as err: - _LOGGER.error("TESTv%s - failed to login to v8 URL", self.api_version) + _LOGGER.debug("v8 login failed (api_version=%s)", self.api_version) raise AirOSConnectionSetupError("Failed to login to AirOS device") from err else: - _LOGGER.error("TESTv%s - returning from v8 login", self.api_version) + _LOGGER.debug("v8 login succeeded (api_version=%s)", self.api_version) returnBased on learnings.
491-521: Use debug level for login attempt logs and fix message mismatch.Lines 491-516 use error level for normal login flow. Line 519 says "extended multipart" but this is the simple attempt block.
Apply this diff:
- _LOGGER.error("TESTv%s - start v6 attempts", self.api_version) + _LOGGER.debug("Starting v6 login attempts (api_version=%s)", self.api_version) # --- ATTEMPT B: Simple Payload (multipart/form-data) --- try: - _LOGGER.error( - "TESTv%s - Trying V6 POST to %s with SIMPLE multipart/form-data", - self.api_version, - self._login_urls["v6_login"], - ) + _LOGGER.debug("Attempting v6 simple multipart login (api_version=%s)", self.api_version) await self._request_json( ... ) except (AirOSUrlNotFoundError, AirOSConnectionSetupError) as err: - _LOGGER.error( - "TESTv%s - V6 simple multipart failed (%s) on %s. Error: %s", - self.api_version, - type(err).__name__, - self._login_urls["v6_login"], - err, - ) + _LOGGER.debug( + "v6 simple multipart failed (%s): %s (api_version=%s)", + type(err).__name__, + err, + self.api_version, + ) except AirOSConnectionAuthenticationError: - _LOGGER.error( - "TESTv%s - autherror during extended multipart", self.api_version - ) + _LOGGER.debug("v6 login auth error (simple multipart) (api_version=%s)", self.api_version) raiseBased on learnings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
airos/base.py(8 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
airos/base.py (2)
tests/conftest.py (1)
base_url(18-20)airos/exceptions.py (4)
AirOSDeviceConnectionError(24-25)AirOSUrlNotFoundError(44-45)AirOSConnectionSetupError(8-9)AirOSConnectionAuthenticationError(12-13)
| # self.session = session | ||
| self.session = aiohttp.ClientSession( | ||
| connector=aiohttp.TCPConnector(verify_ssl=False, force_close=True), | ||
| cookie_jar=aiohttp.CookieJar(), | ||
| ) |
There was a problem hiding this comment.
Critical: Passed session parameter ignored, causing resource leak.
The code comments out the intended use of the passed session parameter and creates a new ClientSession that is never closed. This breaks the API contract and causes resource leaks.
Apply this diff to use the passed session:
- # self.session = session
- self.session = aiohttp.ClientSession(
- connector=aiohttp.TCPConnector(verify_ssl=False, force_close=True),
- cookie_jar=aiohttp.CookieJar(),
- )
+ self.session = sessionIf you need to configure the session differently, do so at the call site where the session is created, not here.
🤖 Prompt for AI Agents
In airos/base.py around lines 71 to 75, the code ignores the passed-in session
and instantiates a new aiohttp.ClientSession (which is never closed), causing a
resource leak and breaking the API contract; change the assignment to use the
provided session (self.session = session) instead of creating a new
ClientSession here, and remove the inline ClientSession construction; if a
fallback is required, handle session creation at the call site or clearly
document/raise when session is None so ownership and closing responsibilities
remain unambiguous.
| # Start of v6, go for cookies | ||
| _LOGGER.error( | ||
| "TESTv%s - Trying to get /index.cgi first for cookies", self.api_version | ||
| ) | ||
| with contextlib.suppress(Exception): | ||
| cookieresponse = await self._request_json( | ||
| "GET", | ||
| f"{self.base_url}/index.cgi", | ||
| authenticated=True, | ||
| headers={ | ||
| "Referer": f"{self.base_url}/login.cgi", | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36", | ||
| }, | ||
| ) | ||
| _LOGGER.error( | ||
| "TESTv%s - Cookie response: %s", self.api_version, cookieresponse | ||
| ) | ||
| if isinstance(cookieresponse, aiohttp.ClientResponse): | ||
| _LOGGER.debug( | ||
| "TESTv%s - Finalization redirect chain: %s", | ||
| self.api_version, | ||
| cookieresponse.history, | ||
| ) | ||
| else: | ||
| _LOGGER.debug( | ||
| "TESTv%s - Finalization response is not a ClientResponse: %s", | ||
| self.api_version, | ||
| type(cookieresponse), | ||
| ) |
There was a problem hiding this comment.
Major: Overly broad exception suppression in cookie prefetch.
Line 450 uses contextlib.suppress(Exception) which is too broad and will hide programming errors, network issues, and other unexpected failures.
Apply this diff:
# Start of v6, go for cookies
- _LOGGER.error(
- "TESTv%s - Trying to get /index.cgi first for cookies", self.api_version
- )
- with contextlib.suppress(Exception):
+ _LOGGER.debug("Prefetching cookies from /index.cgi (api_version=%s)", self.api_version)
+ try:
cookieresponse = await self._request_json(
"GET",
f"{self.base_url}/index.cgi",
authenticated=True,
headers={
"Referer": f"{self.base_url}/login.cgi",
"User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/141.0.0.0 Safari/537.36",
},
)
- _LOGGER.error(
- "TESTv%s - Cookie response: %s", self.api_version, cookieresponse
- )
- if isinstance(cookieresponse, aiohttp.ClientResponse):
- _LOGGER.debug(
- "TESTv%s - Finalization redirect chain: %s",
- self.api_version,
- cookieresponse.history,
- )
- else:
- _LOGGER.debug(
- "TESTv%s - Finalization response is not a ClientResponse: %s",
- self.api_version,
- type(cookieresponse),
- )
+ _LOGGER.debug("Cookie prefetch response: %s (api_version=%s)", cookieresponse, self.api_version)
+ except (aiohttp.ClientError, TimeoutError, AirOSDeviceConnectionError) as err:
+ _LOGGER.debug("Cookie prefetch failed (non-fatal): %s", err)🤖 Prompt for AI Agents
In airos/base.py around lines 446 to 474, the code currently wraps the cookie
prefetch in contextlib.suppress(Exception) which is too broad; replace the
suppress with a try/except that only catches expected runtime/network errors
(e.g., aiohttp.ClientError, asyncio.TimeoutError) and logs the exception details
before continuing, and let unexpected exceptions propagate; ensure you also log
the exception type/message (use _LOGGER.exception or _LOGGER.error with
exc_info) so failures are visible instead of silently swallowed.
|
Replaced by #122 |
1 participant
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Chores