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

Propagate exception to all pending commands #221

Merged
merged 1 commit into from
Nov 26, 2023
Merged

Propagate exception to all pending commands #221

merged 1 commit into from
Nov 26, 2023

Conversation

2franix
Copy link
Contributor

@2franix 2franix commented Nov 12, 2023

Fixes #220 (see issue for details about the problem fixed here).

The fix consists in iterating over all commands still in the queue while we are running the generic exception handler, and pass each of them the exception being handled by calling their _feed_error() method.

With the fix, the output of the minimal script reproducing the issue is:

Fetching status...
Status: {'repeat': '0', 'random': '0', 'single': '0', 'consume': '0', 'partition': 'default', 'playlist': '8', 'playlistlength': '10', 'mixrampdb': '0.000000', 'state': 'stop'}
Fetching status...
Connection failure!
Done!
Exception in callback MPDClient.__idle_result(<CommandResul...ectionError()>)
handle: <Handle MPDClient.__idle_result(<CommandResul...ectionError()>)>
Traceback (most recent call last):
  File "/home/cyrille/.pyenv/versions/3.11.4/lib/python3.11/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
  File "/home/cyrille/develop/python-mpd2-2franix/mpd/asyncio.py", line 356, in __idle_result
    idle_changes = result.result()
                   ^^^^^^^^^^^^^^^
  File "/home/cyrille/develop/python-mpd2-2franix/mpd/asyncio.py", line 318, in __run
    await result._feed_from(self)
  File "/home/cyrille/tmp/testmpd.py", line 6, in rigged_feed_from
    raise ConnectionError()
ConnectionError

Pay attention to the beginning of the output as there are now the two lines we expect:

Connection failure!
Done!

showing that the client now receives the error.

The remainder of the output comes from the MPDClient.__idle_result() method hooked as "done callback" of the current future. It accesses result.result() which causes the exception to be raised again. This is most likely harmless and has nothing to do with the issue being fixed.

@2franix
Copy link
Contributor Author

2franix commented Nov 25, 2023

@Mic92 Can you please let me know what you think of this PR?

@Mic92
Copy link
Owner

Mic92 commented Nov 26, 2023

@mergify queue

Copy link
Contributor

mergify bot commented Nov 26, 2023

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at c75f0ba

@mergify mergify bot merged commit c75f0ba into Mic92:master Nov 26, 2023
2 of 3 checks passed
@2franix
Copy link
Contributor Author

2franix commented Nov 26, 2023

Thanks @Mic92!
I saw the tests are failing. That does not look related to my changes but I can take a look if you want.

Otherwise, I am waiting for a new version to be available to fix a bug in Home Assistant. So I am happy to help on anything that speeds up the process. Just let me know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pending commands hang forever if connection breaks while reading MPD's response
2 participants