Skip to content
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

Wrap TCP request reading in async_timeout.timeout() #339

Merged
merged 2 commits into from
May 29, 2023

Conversation

MarijnS95
Copy link
Contributor

@MarijnS95 MarijnS95 commented Feb 18, 2023

Fixes: robbinjanssen/home-assistant-omnik-inverter#116

Resubmit of #207, which I cannot reopen as non-collaborator.

There have recently been reports of RuntimeError: coroutine ignored GeneratorExit which are presumably triggered by timeouts on the socket, without being raised as TimeoutError. This case seems to be mitigated by timing out the request after a default of 10 seconds which doesn't give GeneratorExit - triggered by something yet unknown - enough time to surface.

@codecov
Copy link

codecov bot commented Feb 18, 2023

Codecov Report

Patch coverage: 100.00% and no project coverage change.

Comparison is base (8d5f967) 100.00% compared to head (864111f) 100.00%.

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #339   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            6         6           
  Lines          357       362    +5     
  Branches        57        60    +3     
=========================================
+ Hits           357       362    +5     
Impacted Files Coverage Δ
omnikinverter/omnikinverter.py 100.00% <100.00%> (ø)

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@github-actions
Copy link

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days. Thank you for your contributions.

@github-actions github-actions bot added the stale There has not been activity on this issue or PR for quite some time. label Mar 21, 2023
@robbinjanssen robbinjanssen added no-stale This issue or PR is exempted from the stable bot. and removed stale There has not been activity on this issue or PR for quite some time. labels Mar 21, 2023
There have recently been [reports of `RuntimeError: coroutine ignored
GeneratorExit`] which are presumably triggered by timeouts on the
socket, without being raised as `TimeoutError`.  This case seems to be
mitigated by timing out the request after a default of 10 seconds which
doesn't give `GeneratorExit` - triggered by something yet unknown -
enough time to surface.

[reports of `RuntimeError: coroutine ignored GeneratorExit`]: robbinjanssen/home-assistant-omnik-inverter#116
@klaasnicolaas klaasnicolaas added the bugfix Fixing a bug. label May 29, 2023
@klaasnicolaas klaasnicolaas merged commit d53d8d2 into klaasnicolaas:main May 29, 2023
@MarijnS95 MarijnS95 deleted the timeout-tcp branch May 29, 2023 10:47
@github-actions github-actions bot locked and limited conversation to collaborators May 30, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bugfix Fixing a bug. no-stale This issue or PR is exempted from the stable bot.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected error fetching omnik_inverter data: coroutine ignored GeneratorExit
3 participants