Mic92 / python-mpd2

Python library which provides a client interface for the Music Player Daemon.
GNU Lesser General Public License v3.0
352 stars 119 forks source link

Propagate exception to all pending commands #221

Closed 2franix closed 9 months ago

2franix commented 10 months ago

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 commented 9 months ago

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

Mic92 commented 9 months ago

@mergify queue

mergify[bot] commented 9 months ago

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at *c75f0ba7621ea5f7dcda29cf3603796af1e98b2a*
2franix commented 9 months ago

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!