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

Persistent MPDClient #31

Open
Qrtn opened this issue Dec 31, 2013 · 27 comments
Open

Persistent MPDClient #31

Qrtn opened this issue Dec 31, 2013 · 27 comments

Comments

@Qrtn
Copy link

Qrtn commented Dec 31, 2013

I'm writing a web-based front end to MPD. I instantiate and connect MPDClient when my program starts. At first calling commands works fine, but after some time of inactivity,

  File "/home/ztang/music/music.py", line 19, in queue
    client.add(path)
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 588, in decorator
    return wrapper(self, name, args, bound_decorator(self, returnValue))
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 229, in _execute
    return retval()
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 583, in decorator
    return function(self, *args, **kwargs)
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 352, in _fetch_nothing
    line = self._read_line()
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 260, in _read_line
    raise ConnectionError("Connection lost while reading line")
mpd.ConnectionError: Connection lost while reading line

A further attempt to connect:

  File "/home/ztang/music/music.py", line 32, in queuedir
    client.add(path)
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 588, in decorator
    return wrapper(self, name, args, bound_decorator(self, returnValue))
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 227, in _execute
    self._write_command(command, args)
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 252, in _write_command
    self._write_line(" ".join(parts))
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 233, in _write_line
    self._wfile.write("%s\n" % line)
  File "/home/ztang/music/venv/lib/python3.3/site-packages/mpd.py", line 75, in _dummy
    raise ConnectionError("Not connected")
mpd.ConnectionError: Not connected

Is there a way to avoid the ConnectionError? Will I have to catch the exception and reconnect, or connect and disconnect every time I issue a command?

@msmucr
Copy link

msmucr commented Dec 31, 2013

I was thinking about similar issue in my script, which is using mpd module. It is matter of mdp daemon, which disconnect every client afrter some time of inactivity, default period there is one minute and could be configured in mpd.conf via connection_timeout variable. Mpd module throws that exception when trying to write to closed socket after that disconnection. Another invoke of connect method should be enough to issue next commands to mpd daemon.
I came to three ways to overcome this:

  • set timeout to long period at server - not good solution.
  • use ping method of MPDClient instance to keep connection alive (lets say spawn another thread in your Python application and run endless loop with ping and timeout shorter than default timeout) - also not so clean solution, because it doesn't cover other situations like restart of mpd daemon or other reasons for closed socket.
  • handle that ConnectionError exception using try statement which wraps invoking of MPDClient methods

I've used last way and individually handled that exceptions in my python script, but thinking about future better solution for that.. create proxy object for MPDClient, which will reconnect automatically after throwing that exception.
Similarly like there: http://www.arngarden.com/2013/04/29/handling-mongodb-autoreconnect-exceptions-in-python-using-a-proxy/, but i haven't did that yet.

Generally i think, that including that auto reconnect functionality directly into python-mpd2 will be very useful.

@Qrtn
Copy link
Author

Qrtn commented Jan 1, 2014

On Tue, Dec 31, 2013 at 5:58 PM, msmucr notifications@github.com wrote:

I was thinking about similar issue in my script, which is using mpd
module. It is matter of mdp daemon, which disconnect every client afrter
some time of inactivity, default period there is one minute and could be
configured in mpd.conf via connection_timeout variable. Mpd module throws
that exception when trying to write to closed socket after that
disconnection. Another invoke of connect method should be enough to issue
next commands to mpd daemon.

Ah, so it's mpd that disconnects the client! I will probably handle the
ConnectionError as well. It is a little annoying having to try-except for
every call, so I agree, auto reconnect would be nice. Thanks msmucr.`

@Mic92
Copy link
Owner

Mic92 commented Jan 1, 2014

In a different library I use/have committed to, the client tries to connect before every send, if no connection is available and returns an error in case of failure after this action. A different solution might be something like a ConnectionError handler, because library user might have different strategies how to recover from an error. Maybe it could even be a combination of both.

@msmucr
Copy link

msmucr commented Jan 2, 2014

Hello Jörg,

i've checked mpd module guts very briefly and thought little bit about its internal exception handling. Currently it is raised mostly at place, where module reads response from server and checking newline at its end. Simple reconnection in possible internal handler wouldn't be enough for recovery at that moment, it would be necessary to repeat last command after successful reconnection and parse response again. So it needed to store all connection details and last commands with arguments to some internal property of MPDClient instance. I haven't tried password protected server yet, but it will also maybe needs another send of password.
Variant, you've mentioned with preventive connection before any command will be without that command repeat, but maybe include some overhead.
And external wrapper (like proxy object) at level of application, which use library, wouldn't require any further internal modifications.
What do will be your preferred solution?

@dbrgn
Copy link
Contributor

dbrgn commented Jan 31, 2014

I'm having the same troubles here. We're moving to MPD with Orochi (see dbrgn/orochi#45) and handling this issue in the python-mpd2 library would be the better way in my opinion than to handle it all over our code with try-except blocks.

@Qrtn
Copy link
Author

Qrtn commented Feb 1, 2014

You could connect and disconnect every time you send a command.

from contextlib import contextmanager
import mpd

HOST, PORT = '127.0.0.1', 6600
client = mpd.MPDClient()

@contextmanager
def connection():
    try:
        client.connect(HOST, PORT)
        yield
    finally:
        client.close()
        client.disconnect()

def queue(path):
    with connection():
        try:
            client.add(path)
        except mpd.CommandError:
            return 'invalid path'

    return 'queued'

@dbrgn
Copy link
Contributor

dbrgn commented Feb 2, 2014

Ah, that would actually be a great workaround in my case. Still, support for persistent connections would be even better.

@Mic92
Copy link
Owner

Mic92 commented Feb 2, 2014

As msmucr said, I could add a callback hander in _read_line(), which an application can overwrite. But just resending the last command can be dangerous, because it is unknown in which state the connection broke (it might be that the command was executed, but the connection broke during the response). This will not replace the try-catch statements around all mpd commands. But it will remove some boiler code, because the program does not need to recover from each mpd command. About the persistence connection issue: this could be solved by sending a ping command from time to time. The problem is, that this would be require some kind of event loop or timer, which is not available in every python script.

@ways
Copy link

ways commented May 8, 2014

I'm having the same problem using mopidy instead of mpd.

Traceback (most recent call last):
File "/root/RadiOS/RadiOS.py", line 442, in
while True:
File "/root/RadiOS/RadiOS.py", line 311, in Compare
str (nowPlaying) + " or ioChannel " + str (ioChannel[0]) )
File "/root/RadiOS/RadiOS.py", line 150, in StopMPD
c.clear ()
File "/usr/local/lib/python2.7/dist-packages/mpd.py", line 583, in decorator
return wrapper(self, name, args, bound_decorator(self, returnValue))
File "/usr/local/lib/python2.7/dist-packages/mpd.py", line 229, in _execute
return retval()
File "/usr/local/lib/python2.7/dist-packages/mpd.py", line 578, in decorator
return function(self, _args, *_kwargs)
File "/usr/local/lib/python2.7/dist-packages/mpd.py", line 352, in _fetch_nothing
line = self._read_line()
File "/usr/local/lib/python2.7/dist-packages/mpd.py", line 260, in _read_line
raise ConnectionError("Connection lost while reading line")
mpd.ConnectionError: Connection lost while reading line

@handsomegui
Copy link

So... it there any update & solution for this issue?

@ways
Copy link

ways commented Jun 8, 2014

@handsomegui My solution was to do a ping at a regular interval. See line 307 at https://github.com/ways/RadiOS/blob/master/RadiOS.py.

Works for me.

@schamp
Copy link

schamp commented Dec 17, 2015

I solved this problem by creating a client class that tries to ping before each command, and if it fails, reestablishes the connection. By pinging first, we avoid bad side-effects from interrupted commands. It uses the 'commands' command to determine which commands are available, and then intercepts any available command function (except for ping, which it uses internally).

I've been using it for a few days, and it seems stable. I hope you find it useful.

edit moved to its own repo, voila: https://github.com/schamp/PersistentMPDClient/

@Mic92
Copy link
Owner

Mic92 commented Feb 20, 2018

We have now two backends to handle network events better:
asyncio and twisted

@s-kostyuk
Copy link

Maybe you'll find the following information interesting:

jws85 pushed a commit to jws85/mpdnotify that referenced this issue Dec 2, 2018
MPD runs on a fairly short client timeout[1], so you actually
do *need* to handle disconnects reliably as a matter of course.

Since we have the Go library 'watcher' running, and it doesn't seem to
be affected by timeouts (probably handles it internally) we can have
the watcher connection open until an interesting event happens, at
which we actually connect to MPD and fetch the data associated with
the event.

As such, I've actually been able to remove parallelism, since it's not
needed.  A much simpler and more robust design.

[1] Mic92/python-mpd2#31 (comment)
@Galicarnax
Copy link

Galicarnax commented Oct 4, 2020

Are there other reasons, apart from default timeout, that MPD server may break connection? I'm trying to use python-mpd2 for custom playback statistics, by setting timestamp stickers like last_played for songs. I need to track if a song has been played for, say, 30 seconds, and if so, I put the timestamp with sticker_set. For that I use threading.Timer, and in its callback function I set the stickers. It just always fails due to connection error, even if the client communicated with the server just a few seconds prior to that. When testing manually without using timer, setting stickers works fine. Then I found that any command (e.g., currentsong) fails with the connection error if invoked via Timer's callback. Even if I use schamp's version linked above (PersistentMPDClient). Here is the traceback:

Traceback (most recent call last):
  File "mpdpbs.py", line 317, in <module>
    MPDPlays(2).run()
  File "mpdpbs.py", line 311, in run
    events = self.mpd.idle()
  File "mpdpbs.py", line 65, in idle
    return self.client.idle()
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 389, in mpd_command
    return wrapper(self, name, args, callback)
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 482, in _execute
    return retval()
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 376, in command_callback
    res = function(self, self._read_lines())
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 621, in _parse_idle
    ret = self._wrap_iterator(self._parse_list(lines))
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 569, in _wrap_iterator
    return list(iterator)
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 281, in _parse_list
    for key, value in self._parse_pairs(lines):
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 220, in _parse_pairs
    for line in lines:
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 547, in _read_lines
    line = self._read_line()
  File "/usr/lib/python3.8/site-packages/mpd/base.py", line 532, in _read_line
    raise ConnectionError("Connection lost while reading line")
mpd.base.ConnectionError: Connection lost while reading line

@quantenschaum
Copy link

@Galicarnax
Copy link

Galicarnax commented Oct 5, 2020

Basically, I try to do the same trick as in the linked code, with forced disconnect/connect on mpd.ConnectionError. The difference is that I didn't use the _reset() method. I now introduced this part as well, doesn't seem to make difference. The problem happens only when communication with MPD server happens in Timer's callback function.

@Galicarnax
Copy link

Galicarnax commented Oct 5, 2020

For now I've solved the problem by not relying on python-mpd2 in the callback function. I still use python-mpd2 to listen for player events to determine when a new track starts, etc., but in the callback function I invoke mpc sticker ... get ... via subprocess. This works without problems. Still, would be nice to solve the issue with losing connection in python-mpd2.

@quantenschaum
Copy link

Just guessing here, but python-mpd2 is not threadsafe! You must call it from one thread only.

@Mic92
Copy link
Owner

Mic92 commented Oct 20, 2020

It is not thread safe. You would need to use a lock around it.

@quantenschaum
Copy link

Yes, locking is one possibility. I used a dedicated thread for calling mpd methods. Instead of locking and calling mpd directly from different threads, I queued calls to mpd in a queue which then get actually executed in the dedicated thread. This way you get predictable return times (for the queuing) and mpd is operated single threaded.

@Stephanowicz
Copy link

Stephanowicz commented Dec 19, 2020

Hi,
may someone help me to understand where I can catch this exception
mpd.base.ConnectionError: Connection lost while reading line
as I have several nested try/catch blocks - but they don't catch it...
as it says mpd.base.ConnectionError - does this mean it won't be caught by except ConnectionError:?

Cheers, Stephan

EDIT:
As a workaround, I found that I can catch it with

    except:
      if "ConnectionError" in str(exc_info()[0]):

@quantenschaum
Copy link

except ConnectionError: maybe?

@Stephanowicz
Copy link

except ConnectionError: maybe?

Hi, no it doesn't catch it ...
This exception seems to be diffrent, as it is risen after the connection has already been established and then gets disconnected.
mpd.base.ConnectionError

(To catch the very first ConnectionError when trying to connect is no problem with 'except ConnectionError:'

@quantenschaum
Copy link

quantenschaum commented Dec 21, 2020

This may actually not be part of this issue, but people may find it useful anyways. Could you @Stephanowicz please post a minimal example to reproduce this?

@Stephanowicz
Copy link

This may actually not part of this issue but people may find it useful anyways. Could you @Stephanowicz please post a minimal example to reproduce this?

Hmm - if You look in the other posts with error messages in this thread You'll see that they all have this mpd.base.ConnectionError

If You already have the lib running it's quite simple to reproduce by killing mpd :D

...I'll see if I find time to make a simple example with different try/catches

@Stephanowicz
Copy link

Stephanowicz commented Dec 21, 2020

Ok, here's a script for testing
Be aware, that mpd will be stopped during the test
I'm running this on moOde audioplayer - so I can execute sudo systemctl in the script running as a normal user - maybe You have to run it as root in case this doesn't work for You

At first, mpd wil be stopped and then the script tries to connect - this will raise the default ConnectionError
then, after connected, a few status queries will be run - then mpd will be stopped - this will raise the mpd.base.ConnectionError - the error, I don't know how to catch 'regularily

Cheers, Stephan

#!/usr/bin/python3
# -*- encoding: utf-8 -*-
from time import sleep
from sys import exit
from sys import exc_info
from mpd import MPDClient
import os

def mpdConnect():
  connected = False
  iConnTry=0   

  while not connected:
    try:
     client.connect("localhost", 6600)  # connect to localhost:6600
     connected=True
     return True

    except ConnectionError:
     if iConnTry == 0: print("except ConnectionError caught")
     print("trying to connect".ljust(iConnTry+1,".").ljust(20)[:20])
     iConnTry +=1
     if iConnTry > 7:
       os.system('sudo systemctl start mpd')
     if iConnTry > 10:
      return connected
     sleep(1)
     pass

def getMpdStats():
  connected = True
  i=0
  while connected:
    try:
        status=client.status()
        state=str(status['state'])
        print(state + " " + str(i))
        i+=1
        if i > 3:
          os.system('sudo systemctl stop mpd')
        sleep(1)
    except (ConnectionError):
      print("except ConnectionError caught")
      return
    except:
      if "ConnectionError" in str(exc_info()[0]):
        print(str(exc_info()[0]))
        sleep(2)
        return
      else:
       raise  



     
print("Warning! This script is testing a connection disruption to mpd and therefor will stop the mpd process!")

if not input("continue? (y/n): ").lower().strip()[:1] == "y": exit(1)

client = MPDClient()               # create client object
client.timeout = 10                # network timeout in seconds (floats allowed), default: None
client.idletimeout = None          # timeout for fetching the result of the idle command is handled seperately, default: None

print("stopping mpd and then trying to connect...")
os.system('sudo systemctl stop mpd')
connected=mpdConnect()
if not connected:
  print("cannot connect to mpd...?")
  exit(1)
print("connected")
sleep(0.5)
print("running a few status queries and then will stop mpd")
getMpdStats()

print("starting mpd...")
os.system('sudo systemctl start mpd')
print("bye...")

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

No branches or pull requests