jaydeethree / Home-Assistant-weatherdotcom

Home Assistant custom component/integration for Weather.com
GNU General Public License v3.0
35 stars 5 forks source link

Blocking Call to Open inside Event Loop #33

Open nima-1102 opened 3 weeks ago

nima-1102 commented 3 weeks ago

A blocking call to open was detected inside the event loop by the custom integration 'weatherdotcom'. This issue occurs in the file coordinator.py at line 232 within the custom component 'weatherdotcom'. The problem arises when attempting to load JSON data from a file, which results in the event loop being blocked.

Steps to Reproduce:

Install and configure the 'weatherdotcom' custom integration. Start the Home Assistant service. Observe the log for errors related to blocking calls within the event loop.

Expected Behavior: The custom integration should handle file operations asynchronously to avoid blocking the event loop, ensuring smooth and responsive operation.

Actual Behavior: The integration attempts to open and read a JSON file synchronously, which blocks the event loop and causes potential delays or disruptions in the Home Assistant's performance.

Error Log:

Logger: homeassistant.util.loop
Source: util/loop.py:84
First occurred: 14:24:55 (1 occurrences)
Last logged: 14:24:55

Detected blocking call to open inside the event loop by custom integration 'weatherdotcom' at custom_components/weatherdotcom/coordinator.py, line 232: tfiledata = json.load_json(f'{tfiledir}{tfilename}.json') (offender: /usr/src/homeassistant/homeassistant/util/json.py, line 78: with open(filename, mode="rb") as fdesc:), please create a bug report at https://github.com/jaydeethree/Home-Assistant-weatherdotcom/issues/
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/usr/src/homeassistant/homeassistant/__main__.py", line 223, in <module>
    sys.exit(main())
  File "/usr/src/homeassistant/homeassistant/__main__.py", line 209, in main
    exit_code = runner.run(runtime_conf)
  File "/usr/src/homeassistant/homeassistant/runner.py", line 190, in run
    return loop.run_until_complete(setup_and_run_hass(runtime_config))
  File "/usr/local/lib/python3.12/asyncio/base_events.py", line 672, in run_until_complete
    self.run_forever()
  File "/usr/local/lib/python3.12/asyncio/base_events.py", line 639, in run_forever
    self._run_once()
  File "/usr/local/lib/python3.12/asyncio/base_events.py", line 1988, in _run_once
    handle._run()
  File "/usr/local/lib/python3.12/asyncio/events.py", line 88, in _run
    self._context.run(self._callback, *self._args)
  File "/usr/src/homeassistant/homeassistant/setup.py", line 165, in async_setup_component
    result = await _async_setup_component(hass, domain, config)
  File "/usr/src/homeassistant/homeassistant/setup.py", line 447, in _async_setup_component
    await asyncio.gather(
  File "/usr/src/homeassistant/homeassistant/setup.py", line 449, in <genexpr>
    create_eager_task(
  File "/usr/src/homeassistant/homeassistant/util/async_.py", line 37, in create_eager_task
    return Task(coro, loop=loop, name=name, eager_start=True)
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 742, in async_setup_locked
    await self.async_setup(hass, integration=integration)
  File "/usr/src/homeassistant/homeassistant/config_entries.py", line 594, in async_setup
    result = await component.async_setup_entry(hass, self)
  File "/config/custom_components/weatherdotcom/__init__.py", line 48, in async_setup_entry
    weathercoordinator = WeatherUpdateCoordinator(hass, config)
  File "/config/custom_components/weatherdotcom/coordinator.py", line 84, in __init__
    self._tranfile = self.get_tran_file()
  File "/config/custom_components/weatherdotcom/coordinator.py", line 232, in get_tran_file
    tfiledata = json.load_json(f'{tfiledir}{tfilename}.json')

Possible Solution: Refactor the code to use asynchronous file I/O operations to load the JSON data, thereby preventing the event loop from being blocked.

Environment:

Core: 2024.6.2 Supervisor: 2024.06.0 Operating System: 12.3 Frontend: 20240610.0 Integration Version: 1.1.6

Additional Information: This issue impacts the performance of the Home Assistant due to the synchronous nature of the file operation within the event loop. Addressing this will improve the responsiveness and reliability of the integration.

Please let me know if further information is required to resolve this issue.

jaydeethree commented 3 weeks ago

Thanks for the report! This doesn't seem urgent to me (I see similar log messages for several other integrations, including some core integrations), so I'll plan to look at it sometime in the next week or two.

jaydeethree commented 2 weeks ago

I started looking at this to get a better understanding of what I need to change. This error happens because of how I'm loading the JSON file that contains translations, and the right way to fix this is to use Home Assistant's native translation functionality instead of the custom translation handling that's already built into the integration. I started looking at how to switch to that, but it's going to require a lot of changes to this integration and I unfortunately don't have time for that right now. Instead I'm planning to just load the JSON file asynchronously - that will fix this error and be an improvement, but I'd still like to switch to the native translation functionality someday.

jaydeethree commented 2 weeks ago

I spent some time working on this, but unfortunately I haven't been able to fix it yet. I have no experience with Python coroutines/async, and all of the code for that is from https://github.com/cytech/Home-Assistant-wundergroundpws/ which I used as the base for this integration. I tried several different ways of fixing this but all of them created more problems - for example I was able to load the translations asynchronously and make this error go away, but that caused the translations to not be loaded early enough during startup so the sensors didn't have their names set (which led to different, more serious errors).

For now this issue isn't very urgent - it logs an annoying warning, but otherwise it has no real impact. I'll hopefully revisit this at some point in the future, but for now I'm planning to put it on hold. If anyone with more async experience wants to send a PR to fix this I would appreciate it :)