Skip to content

BrokenPipeError (?) when idle for a long time #64

Closed
Closed
BrokenPipeError (?) when idle for a long time#64

Description

@speendo
speendo
opened on Dec 15, 2015

Hi and thank you for this project!

When mpd is idle (after calling client.stop()) for a couple of hours, I cannot successfully call client.play() again. I am aware of ConnectionErrors that can occur so I catch them in my script. In this case, however, I get a BrokenPipeError (sometimes I don't get an error at all, but nevertheless mpd won't start to play reproducibly).

I wrote a test script (find it here: https://gist.github.com/speendo/0d9c1a028d045de3f7a6) to track the error down.

I ran this script on my raspberry pi with nohup python3 test_mpd.py &.

Here's a traceback of the Error (found in nohup.out):

Traceback (most recent call last):
  File "test_mpd.py", line 62, in <module>
    client.play()
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 600, in decorator
    return wrapper(self, name, args, bound_decorator(self, returnValue))
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 236, in _execute
    self._write_command(command, args)
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 263, in _write_command
    self._write_line(" ".join(parts))
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 243, in _write_line
    self._wfile.flush()
  File "/usr/lib/python3.4/socket.py", line 391, in write
    return self._sock.send(b)
BrokenPipeError: [Errno 32] Broken pipe

And here's the log:

2015-12-15 08:35:26,497 INFO: Log started
2015-12-15 08:35:26,500 INFO: Attempt to (re-)connect to mpd server
2015-12-15 08:35:26,503 INFO: Calling MPD connect('localhost', 6600, timeout=None)
2015-12-15 08:35:26,566 INFO: Finished attempt to (re-)connect to mpd server
2015-12-15 08:35:26,568 INFO: <bound method MPDClient.decorator of <mpd.MPDClient object at 0xb6a38590>>
2015-12-15 08:35:26,570 INFO: Add playlist entry
2015-12-15 08:35:26,573 DEBUG: Calling MPD clear()
2015-12-15 08:35:26,578 DEBUG: Calling MPD add('http://uwstream2.somafm.com:9016',)
2015-12-15 08:35:26,582 INFO: Start playing for the first time
2015-12-15 08:35:26,585 DEBUG: Calling MPD play()
2015-12-15 08:35:36,597 INFO: This obviously works. Switching off now for 4 hours.
2015-12-15 08:35:36,604 DEBUG: Calling MPD stop()
2015-12-15 12:35:36,725 INFO: Trying to switch on again now
2015-12-15 12:35:36,727 DEBUG: Calling MPD play()

Can you help me find the source of this problem (or even solve it)?

Activity

  1. Mic92 commented on Dec 16, 2015

    @Mic92
    Mic92
    on Dec 16, 2015
    Owner

    Duplicate of #31

  2. Mic92
    closed this as completedon Dec 16, 2015

  3. speendo commented on Dec 16, 2015

    @speendo
    speendo
    on Dec 16, 2015
    Author

    Hello Jörg!

    I'm aware of #31 and ConnectionErrors.

    Prove me wrong, but I don't see how this issue is related to that.

    My script should catch all ConnectionErrors with

    try:
    client.play()
except MPDConnectionError:
    logging.info("Lost MPD connection")
    client.connect()
    client.play()

    Please find the whole script here: https://gist.github.com/speendo/0d9c1a028d045de3f7a6

    After a couple of hours of idle time I get a (generic) BrokenPipeError which - as far as I know - is not related to the mpd-specific ConnectionError.

    I would be glad if you could clarify why this is a duplicate or reopen the issue.

    Thank you!

  4. Mic92 commented on Dec 16, 2015

    @Mic92
    Mic92
    on Dec 16, 2015
    Owner

    ok. take a look at it.

  5. Mic92
    reopened this on Dec 16, 2015

  6. speendo commented on Dec 16, 2015

    @speendo
    speendo
    on Dec 16, 2015
    Author

    Thank you!

  7. Mic92 commented on Dec 16, 2015

    @Mic92
    Mic92
    on Dec 16, 2015
    Owner

    The problem here is, that mpd does not mask any exception thrown at all, which are generated by the layers below. Sorry this was not my invention. The alternative is exception chaining (catching the error and rethrow an MPDError inheriting the original one). Python 2 does not support this. However it is possible to a custom reference in the exeption to its root cause.

  8. speendo commented on Dec 16, 2015

    @speendo
    speendo
    on Dec 16, 2015
    Author

    Okay, thank you. Took me some time but I think I understand the problem now.

    I use Python3 anyway but I understand that this is not a solution you would want to offer only for Python3 users...

    I would like to avoid catching all Exceptions and do something like

    try:
    client.play()
except Exception:
    logging.info("Some random exception occured")
    client.connect()
    client.play()

    as this is considered as bad style... Is there a way to be more specific - maybe using the custom reference?

  9. speendo commented on Dec 16, 2015

    @speendo
    speendo
    on Dec 16, 2015
    Author

    Wouldn't this be a possible solution?
    http://stackoverflow.com/questions/1350671/inner-exception-with-traceback-in-python

    [edit]
    more to find here:
    http://www.ianbicking.org/blog/2007/09/re-raising-exceptions.html

  10. speendo commented on Dec 24, 2015

    @speendo
    speendo
    on Dec 24, 2015
    Author

    Just for testing purposes, I tried to catch every exception and reconnect to the server, if I catch one. Like this:

    try:
    client.play()
except Exception:
    logging.info("Some random exception occured")
    client.connect()
    client.play()

    (find a full example here: https://gist.github.com/speendo/91de6511443e75f18fed)

    This also didn't work because the client still thinks it is connected. I get this error:

    Traceback (most recent call last):
  File "test_mpd.py", line 63, in <module>
    client.play()
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 600, in decorator
    return wrapper(self, name, args, bound_decorator(self, returnValue))
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 236, in _execute
    self._write_command(command, args)
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 263, in _write_command
    self._write_line(" ".join(parts))
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 243, in _write_line
    self._wfile.flush()
  File "/usr/lib/python3.4/socket.py", line 391, in write
    return self._sock.send(b)
BrokenPipeError: [Errno 32] Broken pipe

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test_mpd.py", line 66, in <module>
     client_connect()
  File "test_mpd.py", line 25, in client_connect
    client.connect("localhost", 6600)
  File "/usr/local/lib/python3.4/dist-packages/mpd.py", line 507, in connect
    raise ConnectionError("Already connected")
mpd.ConnectionError: Already connected

    As far as I understand this, this means I would have to close the (broken) connection first before I could reestablish the client's connection, right?

    This would be a bit akward. I am only a hobby programmer and not an expert, but from my point of view I think that python-mpd2 should catch the BrokenPipeError and do something about it - either set the connection closed or reestablish the connection.

    If the connection is not reestablished for me it would feel more natural if my script would fire a MPD.ConnectionError instead of a "generic" BrokenPipeError.

    However, do you know of a way to get the BrokenPipeError more quickly? Because every test I make takes me 4 hours which is quite a lot for someone who can only code in his free time ;-)

    By the way:

    Schöne Weihnachten!

  11. dinofizz commented on Feb 25, 2017

    @dinofizz
    dinofizz
    on Feb 25, 2017

    To repro the BrokenPipeError, assuming you have a mpd server running on localhost on port 6600:

    from mpd import MPDClient

client = MPDClient()
client.connect("localhost", 6600)
client.close()
client.ping() # first ping returns with no error, but second one...
client.ping()
Traceback (most recent call last):
  File "<input>", line 1, in <module>
  File "/home/user/.virtualenvs/app/lib/python3.6/site-packages/mpd.py", line 629, in decorator
    return wrapper(self, name, args, bound_decorator(self, returnValue))
  File "/home/user/.virtualenvs/app/lib/python3.6/site-packages/mpd.py", line 252, in _execute
    self._write_command(command, args)
  File "/home/user/.virtualenvs/app/lib/python3.6/site-packages/mpd.py", line 279, in _write_command
    self._write_line(" ".join(parts))
  File "/home/user/.virtualenvs/app/lib/python3.6/site-packages/mpd.py", line 259, in _write_line
    self._wfile.flush()
  File "/usr/lib64/python3.6/socket.py", line 604, in write
    return self._sock.send(b)
BrokenPipeError: [Errno 32] Broken pipe

    As stated above, one cannot call a client.connect(...) to remedy the situation as the client._sock object exists, and mpd thus believes it is still connected.

    To handle this case I am catching BrokenPipeError and then calling a client._reset(), which sets the _sock object to None. From here I can attempt to reconnect using client.connect(...)

  12. TomTom0815 commented on Mar 14, 2017

    @TomTom0815
    TomTom0815
    on Mar 14, 2017

    Hi.

    I'm running into this issue as well. After some hours, the connection get dropped and get a socket errors all over the place. It only happens after some hours.

    Sad story: I build a device to play music for my little daughter (4 years) - it worked great for many months until I switched to python-mpd2. She goes to bed and listens to music and as soon she wakes up she like to hear her favorite audio book. So, now she wakes me up at 5am and I need to reboot the device. :(

    I reworked the connection/re-connection and expectation handling many times (takes one day turn-around for each test) but always ended up with a new exception or fail.
    I think the merge request #65 from Speendo solves the issue. Protect the write() and flush() to the sockets, catch socket.error and do a reset() on the socket will solve the issues with the re-connect when I discover that the mpd command didn’t succeeded. Right now, the re-connect fails with “Already connected” or more socket errors.

    I’m going to test this merge request locally in the next day. But could you please revive the discussions around this merge request and drive that forward. I would really appreciate a fix for this.

  13. speendo commented on Mar 14, 2017

    @speendo
    speendo
    on Mar 14, 2017
    Author

    Hello TomTom0815

    I use this library for my Project FlushFM - a webradio on my toilet that starts playing every time the light goes on.

    It's working fine for a more than a year now, so chances are that my pull request will work for you too.

    I wish you (and your daughter) the best :)

  14. quantenschaum commented on Sep 23, 2017

    @quantenschaum
    quantenschaum
    on Sep 23, 2017

    I had the same problem and solved/worked around it by calling _reset() on BrokenPipeError and then reconnect.

    I think disconnect() should be modified to be able to deal with dead, already closed sockets correctly.

  15. quantenschaum commented on Sep 23, 2017

    @quantenschaum
    quantenschaum
    on Sep 23, 2017

    Maybe something like:

        def disconnect(self):
        logger.info("Calling MPD disconnect()")
        try:
            if (self._rfile is not None
                    and not isinstance(self._rfile, _NotConnected)):
                self._rfile.close()
            if (self._wfile is not None
                    and not isinstance(self._wfile, _NotConnected)):
                self._wfile.close()
            if self._sock is not None:
                self._sock.close()
        finally:
            self._reset()

    So it will still raise its exception, but afterwards the mpd instance is able to reconnect.

  16. quantenschaum commented on Sep 24, 2017

    @quantenschaum
    quantenschaum
    on Sep 24, 2017

    I find it quite astonishing, that the library does not handle the BrokenPipeError correctly, because it is something very common, when dealing with sockets. The default connection_timeout of mpd is 60 seconds. This means, after 60s mpd closes the connection, when no data was sent. No need to wait for 4 hours. It's 100% reproducible. Connect, wait more than 60s, and send something. You get a BrokenPipeError and an unusable MPDClient.

    The problem is, that disconnect tries to close _wfile. This forces a flush of its buffers, which fails, because the socket was closed on the other end. The raised exception prevents _sock.close() and _reset to be called and leaves a borked MPDClient.

    I worked around this deficiency by wrapping MPDClient like this

    from mpd import MPDClient
from mpd import ConnectionError as MPDConnectionError

class MPDClientWrapper(object):
    def __init__(self, *args, **kwargs):
        self.__dict__['_mpd'] = MPDClient(*args, **kwargs)

    def __getattr__(self, name):
        a = self._mpd.__getattribute__(name)
        if not callable(a): return a

        def b(*args, **kwargs):
            try:
                return a(*args, **kwargs)
            except (MPDConnectionError, ConnectionError) as e:
                cargs, ckwargs = self.__dict__['_connect_args']
                self.connect(*cargs, **ckwargs)
                return a(*args, **kwargs)

        return b

    def __setattr__(self, name, value):
        self._mpd.__setattr__(name, value)

    def connect(self, *args, **kwargs):
        self.__dict__['_connect_args'] = args, kwargs
        self.disconnect()
        self._mpd.connect(*args, **kwargs)

    def disconnect(self):
        try:
            self._mpd.close()
            self._mpd.disconnect()
        except (MPDConnectionError, ConnectionError) as e:
            pass
        finally:
            self._mpd._reset()
    1. This makes disconnect never raise a ConnectionError and leaves MPDClient in a clean state.
    2. It allows to connect to another server without the need to disconnect explicitly.
    3. It automatically reconnects, if the connection died for what ever reason
    4. You can easily extend this to and custom methods, i.e. readd volume

  17. speendo commented on Sep 24, 2017

    @speendo
    speendo
    on Sep 24, 2017
    Author

    Thank you for looking at it!

    Did you also see my merge request? At least for me it work very robust and it tries to keep the logic of the original library.

  18. quantenschaum commented on Sep 24, 2017

    @quantenschaum
    quantenschaum
    on Sep 24, 2017

    Hi @speendo, I saw it. It might work for your case, but it does not fix the broken disconnect method and the python version handling seems overly complicated.

    Let's assume the connection is broken and disconnect is called. Then we end up with a borked MPDClient as before.

    In my opinion, disconnect has to be changed as I proposed (or in a similar way). Then a broken connection is indicated by an exception and one can recover from this state by disconnecting and connecting again. Clean, straight forward and it works in either case. One may discuss that a unification of mpd.ConnectionError and ConnectionError makes sense, so that there is only one type of exception to be caught, which indicates a problem with the underlying connection.

  19. quantenschaum commented on Oct 20, 2017

    @quantenschaum
    quantenschaum
    on Oct 20, 2017

    Nothing?

  20. speendo commented on Oct 20, 2017

    @speendo
    speendo
    on Oct 20, 2017
    Author

    Well, @quantenschaum, as far as I get it, I think you simply follow a different approach. While my solution keeps the logic of the original library (not questioning if it makes sense or not), you solve the problem by changing its behavior.

    When using (my version of) python-mpd2, I do also use a wrapper, because I want my mpd client to stay connected. But this wrapper is implemented in my project not in the library.

    Nevertheless, your approach also solves the issue and for that I find it much better than the current state of this library.

    ... however, I guess you wanted @Mic92 to answer your comment, not me...

  21. quantenschaum commented on Oct 21, 2017

    @quantenschaum
    quantenschaum
    on Oct 21, 2017

    Yes, of course, the current behavior is leaving a broken MPDClient when the remote end closes the socket. This is a bug and needs to be fixed = changed. One might make the auto reconnect feature optional, but fixing the broken disconnect method is essential.

  22. Mic92 commented on Feb 20, 2018

    @Mic92
    Mic92
    on Feb 20, 2018
    Owner

    merged in #92

  23. Mic92
    closed this as completedon Feb 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions