-
Notifications
You must be signed in to change notification settings - Fork 290
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible Solution for HTTP2 blocking operation #2519
Comments
Hi jleinenbach, |
Unfortunately the step 1 is still a no go for some of us, many non .com users (.de in my case) cannot go pass 2FA step during the integration process. Downgrade to the older AMP is also not working. |
This is a different issue. I am a .de user and I can go pass the 2FA step. This solution is for the HTTP2 blocking issue only. |
I just did a little bit of digging and I think the root cause is |
I tried all possible workarounds here, but only the HTTP2 blocking warning disappeared. Here are the relevant libraries and modules responsible for the reported blocking call error:
Summary of Responsible Libraries:
Primary Cause:
|
I can confirm that making the above change to
This one is now resolved:
One of my remaining 4 is from custom integration 'smartthinq_sensors':
I applied the smartthinq_sensors change to alexalogin.py and resolved another warning.
...
I also made a change in
I'm now down to two warnings:
|
I just now successfully eliminated the last two warnings!!! This fixes them...
to:
and ~line 735:
to:
|
You mean cookiejar.py? #2520 |
Yes. Your solution is much better than mine though! Thanks! |
@jleinenbach I've just noticed that with your patch:
my http2 connection fails to connect:
I removed the patch and http2 is connected and running:
|
It seems that the issue you're facing is likely due to the shift in Alexapy from using the native Python ssl to homeassistant.util.ssl. This change can cause compatibility issues, especially when Alexapy's internal processes no longer use the expected SSL handling. |
You may try this patch instead, as I’m unable to test your Alexapy SSL changes on my side.
from homeassistant.util.ssl import get_default_context Then replace this method: async def http2_connect() -> Optional[HTTP2EchoClient]:
"""Open HTTP2 Push connection.
This will only attempt one login before failing.
"""
http2: Optional[HTTP2EchoClient] = None
email = login_obj.email
try:
if login_obj.session.closed:
_LOGGER.debug(
"%s: HTTP2 creation aborted. Session is closed.",
hide_email(email),
)
return
# Get the SSL context from Home Assistant
ssl_context = get_default_context()
_LOGGER.debug("Using Home Assistant's get_default_context for SSL.")
# Extract the cafile from the Alexapy login object to maintain consistency
cafile = login_obj.session._ssl_context.get_ca_certs()[0]['path'] # Retrieve the CA certificate path from Alexapy
# Load the SSL context in a separate thread to prevent blocking the event loop
try:
await hass.async_add_executor_job(
ssl_context.load_verify_locations, cafile=cafile
)
_LOGGER.debug("SSL context loaded successfully and certificates verified.")
except Exception as e:
_LOGGER.error(f"Failed to load SSL context: {e}")
# Create HTTP2EchoClient
http2 = HTTP2EchoClient(
login_obj,
msg_callback=http2_handler,
open_callback=http2_open_handler,
close_callback=http2_close_handler,
error_callback=http2_error_handler,
loop=hass.loop,
)
_LOGGER.debug("%s: HTTP2 created: %s", hide_email(email), http2)
# Start HTTP2 connection
await http2.async_run()
except AlexapyLoginError as exception_:
_LOGGER.debug(
"%s: Login Error detected from http2: %s",
hide_email(email),
exception_,
)
hass.bus.async_fire(
"alexa_media_relogin_required",
event_data={"email": hide_email(email), "url": login_obj.url},
)
return
except BaseException as exception_: # pylint: disable=broad-except
_LOGGER.debug(
"%s: HTTP2 creation failed: %s", hide_email(email), exception_
)
return
return http2 |
I will try that.
What's your take on this approach?? |
I removed my changes in alexalogin.py and the verify warning returned:
|
That's a much better solution than my approach. This ensures that all operations related to |
Confirmed that I do need the ssl change in alexalogin.py to eliminate the error in addition to offloading the HTTP2EchoClient.
|
It's because Alexapy uses /usr/local/lib/python3.12/ssl.py and AMP uses AlexaLogin( |
I just don't comprehend the difference between Alan's approach using python's ssl vs the get_default_context in homeassistant.util. I'm thinking maybe his coding predates the existence of homeassistant.util.ssl? |
So, are you happy/comfortable with my code? It will mean yet another alexapy version bump... |
Here is my current |
Based on Alen's documentation, Alexapy was "was originally designed for alexa_media_player". However, I think this also implies that its approach has since evolved to be more general and applicable beyond just Home Assistant. |
Yes but I think his basic code is many years old now and I do not know when HA's util.ssl was added. |
I think this should work, but maybe more logging? We could also try to reconnect/retry if something fails, but this may be too much at once. try:
http2 = await hass.async_add_executor_job(
HTTP2EchoClient,
login_obj,
msg_callback,
open_callback,
close_callback,
error_callback,
loop,
)
_LOGGER.debug("%s: HTTP2 client successfully created: %s", hide_email(email), http2)
except (AlexapyConnectionError, AlexapyLoginError) as conn_error:
_LOGGER.error("Connection or login error with HTTP2Client for %s: %s", hide_email(email), conn_error)
except Exception as e:
# Using _LOGGER.exception to include the full stack trace for debugging purposes
_LOGGER.exception("Unexpected error when initializing HTTP2 client for %s: %s", hide_email(email), e) |
I now solved this problem (hopefully) without a new Please test my (non-HA-exclusive) And this is your This is the only warning I get for alexa* (huge list of almost all integrations, but without any details): |
I have found another issue: The HTTP/2 connection status isn't updated properly. I asked ChatGPT to write the following: Here's an improved version of the response, incorporating your feedback about the importance of jitter handling and focusing more on points 1 and 2: Issues and Solutions Related to the HTTP/2 Connection in
|
@danielbrunt57 |
So, not sure what I might have done wrong but with no alexa_media integration yet, I'm getting this error:
|
Here is the log sequence...
|
There's no pickle file...
and there's no entry in core.config_entries either. |
I've gone back to square one with vanilla 1.29.2 and 4.13.0. I re-added your patch:
but the blocking warning still appears referencing: |
It seems likely that the issue arises because the context wasn't offloaded properly. I'll attach both the The primary goal is to get this working without needing changes to the |
Changes might be required there but I understand the desire to keep it non-HA related. It would be nice to know from Alan though if we really need to! Will wait for your files... |
I suppose I could try a MR with my homeassistant.util.ssl import and see if he accepts it or rejects it. |
Ok, this works for me. I removed my configuration and recreated a new one - and it works. At least both together, but I tried to make it compatible, so please try the There are no more blocking call warnings, but this one:
It's because of the AlexaProxy. |
I left the config entry in place, restored I will try both files a little later... |
Oh no, I'm so sorry, that happend to you as I fixed the typos in my setup:
grep -r "in_progess_instances" /config/custom_components/alexa_media/
grep -r "in_progress_instances" /config/custom_components/alexa_media/ |
I found it in Everything is working fine now with just your
and this warning:
|
I finally found the source for this error! |
This is weird though. The WARNING is also gone! |
See PR 2541 |
@jleinenbach
So I would say that option is a no-go! |
I restarted twice and same scenario each time. Restored |
Remember
One possible solution (I haven't looked in depth) would be to just add a parameter to pass in a context (and possibly AMP side then just needs to include the appropriate HA helpers when creating So something like (not even tried this in an IDE to see what it is missing...) def __init__(
self,
url: str,
email: str,
password: str,
outputpath: Callable[[str], str],
debug: bool = False,
otp_secret: str = "",
oauth: Optional[dict[Any, Any]] = None,
uuid: Optional[str] = None,
oauth_login: bool = True,
context: Optional[SSLContext] = None,
session: Optional[aiohttp.ClientSession] = None,
) -> None:
# pylint: disable=too-many-arguments,import-outside-toplevel
"""Set up initial connection and log in."""
import ssl
import certifi
oauth = oauth or {}
self._hass_domain: str = "alexa_media"
self._prefix: str = "https://alexa."
self._url: str = url
self._email: str = email
self._password: str = password
self._session: session
self._ssl = context
if not context:
self._ssl = ssl.create_default_context(
purpose=ssl.Purpose.SERVER_AUTH, cafile=certifi.where()
) AMP side for this bit would be alexa_media_player/custom_components/alexa_media/__init__.py Lines 236 to 247 in 263a92b
becomes login_obj = AlexaLogin(
url=url,
email=email,
password=password,
outputpath=hass.config.path,
debug=account.get(CONF_DEBUG),
otp_secret=account.get(CONF_OTPSECRET, ""),
oauth=account.get(CONF_OAUTH, {}),
uuid=uuid,
oauth_login=True,
context = homeassistant.util.ssl.get_default_context(),
session = homeassistant.helpers.aiohttp_client.async_get_clientsession
) (that bit has full package names but should be done via imports. Not tested any of this but in theory that would keep ==== alexa_media_player/custom_components/alexa_media/__init__.py Lines 876 to 883 in 263a92b
|
@jamesonuk When I tried it, it became obvious that there cannot be HA dependencies in it. The merge request processing could not find In another github post (I can't find it now), @bdraco recommended changing ssl.create_default_context as a top level import as those occur at load time in the executor loop. I've tried it and it works for me so I have submitted |
Describe the bug
A blocking operation was detected in the
http2_connect()
function in thealexa_media
integration. Specifically, the functionload_verify_locations
was being called inside the event loop, which caused performance issues and blocked other tasks.To Reproduce
Expected behavior
The
http2_connect()
function should complete without blocking the event loop, allowing the system to handle multiple tasks concurrently without performance issues.System details
Debug Logs (alexa_media & alexapy)
Please refer to the following error message from the debug logs:
Additional context
I was able to fix the issue by modifying the
http2_connect()
function to offload the SSL initialization to a separate thread usinghass.async_add_executor_job
. Here is the change that fixed the issue:The Problem:
In Home Assistant, the Alexa Media integration was causing performance issues due to the SSL certificate loading (
load_verify_locations
) being called inside the event loop. This caused the event loop to block, slowing down the system.The Solution:
I adjusted the code to ensure that the SSL certificate loading happens in a separate thread, outside the event loop, allowing the system to run smoothly.
Code Changes:
hass.async_add_executor_job
:HTTP2EchoClient
remains unchanged.Here is the full function after the change:
Since making this change, the warning is no longer appearing.
The text was updated successfully, but these errors were encountered: