Regression in Python 3.5 xmlrpc.client, raises RemoteDisconnected seemingly randomly.

[JelteF]

BPO	26402
Nosy	@vadmium, @JelteF
Files	xmlrpc-remote-disconnected.patch xmlrpc-remote-disconnected.v2.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2016-02-25.13:06:56.549>
created_at = <Date 2016-02-21.16:03:36.525>
labels = ['type-bug', 'library']
title = 'Regression in Python 3.5 xmlrpc.client, raises RemoteDisconnected seemingly randomly.'
updated_at = <Date 2016-02-25.17:15:02.889>
user = 'https://github.com/JelteF'

bugs.python.org fields:

activity = <Date 2016-02-25.17:15:02.889>
actor = 'JelteF'
assignee = 'none'
closed = True
closed_date = <Date 2016-02-25.13:06:56.549>
closer = 'martin.panter'
components = ['Library (Lib)']
creation = <Date 2016-02-21.16:03:36.525>
creator = 'JelteF'
dependencies = []
files = ['41987', '42001']
hgrepos = []
issue_num = 26402
keywords = ['patch', '3.5regression']
message_count = 10.0
messages = ['260617', '260619', '260620', '260621', '260641', '260642', '260660', '260853', '260856', '260869']
nosy_count = 4.0
nosy_names = ['SilentGhost', 'python-dev', 'martin.panter', 'JelteF']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue26402'
versions = ['Python 3.5', 'Python 3.6']

[JelteF]

I've been developing an application which uses fuse as in interface to an xmlrpc API. I developed it with python 3.4 and it worked fine. When I used python 3.5 it started randomly raising a specific error when requesting info using the xmlrpc API.

The error in question is the RemoteDisconnected error which was added in 3.5. An example stacktrace is:

Traceback (most recent call last):
  File "/home/jelte/fun/easyfuse/easyfuse/utils.py", line 20, in _convert_error_to_fuse_error
    yield
  File "/home/jelte/fun/easyfuse/easyfuse/filesystem.py", line 276, in children
    self.refresh_children()
  File "dokuwikifuse.py", line 139, in refresh_children
    pages = dw.pages.list(self.full_path, depth=self.full_depth + 2)
  File "/home/jelte/fun/dokuwikifuse/venv/src/dokuwiki-master/dokuwiki.py", line 102, in list
    return self._dokuwiki.send('dokuwiki.getPagelist', namespace, options)
  File "/home/jelte/fun/dokuwikifuse/venv/src/dokuwiki-master/dokuwiki.py", line 55, in send
    return method(*args)
  File "/usr/lib64/python3.5/xmlrpc/client.py", line 1091, in __call__
    return self.__send(self.__name, args)
  File "/usr/lib64/python3.5/xmlrpc/client.py", line 1431, in __request
    verbose=self.__verbose
  File "/usr/lib64/python3.5/xmlrpc/client.py", line 1133, in request
    return self.single_request(host, handler, request_body, verbose)
  File "/usr/lib64/python3.5/xmlrpc/client.py", line 1146, in single_request
    resp = http_conn.getresponse()
  File "/usr/lib64/python3.5/http/client.py", line 1174, in getresponse
    response.begin()
  File "/usr/lib64/python3.5/http/client.py", line 282, in begin
    version, status, reason = self._read_status()
  File "/usr/lib64/python3.5/http/client.py", line 251, in _read_status
    raise RemoteDisconnected("Remote end closed connection without"
http.client.RemoteDisconnected: Remote end closed connection without response

The program in question can be found here: https://github.com/JelteF/dokuwikifuse
The bug can be initiated by calling running the program as described in the README and doing a couple of file system operations wich do requests. These are things like ls wiki/* or calling head wiki/*.doku.

[JelteF]

A short look through the stacktrace actually seemed to have gotten me to the issue. It is in the xmlrpc.client library in this piece of code:

         for i in (0, 1):
             try:
                 return self.single_request(host, handler, request_body, verbose)
             except OSError as e:
                 if i or e.errno not in (errno.ECONNRESET, errno.ECONNABORTED,
                                         errno.EPIPE):
                     raise
             except http.client.RemoteDisconnected:
                 if i:
                     raise

As can be seen http.client.RemoteDisconnected error is caught after the OSError one. However http.client.RemoteDisconnected is a subclass of OSError since it's a subclass of ConnectionResetError: https://docs.python.org/3/library/http.client.html#http.client.RemoteDisconnected

A simple fix which seems to work for me is switching the two except branches as I did in the attached patch.

[SilentGhost]

The code in question[0] was introduced in bpo-3566: perhaps Martin could comment on existing implementation.

[0] https://hg.python.org/cpython/rev/eba80326ba53

[JelteF]

From what I can see that change simply replaces the old BadStatusLine exception with the new RemoteDisconnected one. But since BadStatusLine is not a subclass of OSError the correct code path would be taken. Currently the path in execpt RemoteDisconnected is simply a no-op as the exception will be caught first as an OSError.

[vadmium]

Thankyou Jelte for finding this. We made RemoteDisconnected inherit from ConnectionResetError (and therefore OSError) thinking it would help with compatibility, but in this case that has backfired. Your patch looks like a valid fix.

I would like to figure out a concise way to produce the fault so I can add a test case and play with the code. Would I be able to use Python’s XML-RPC server, or would I have to make a custom server that dropped the connection?

Also I suspect both exception handlers in the XML-RPC client could be combined as “except ConnectionError”, but I want to be sure before making that change.

[JelteF]

I don't know much about the server as I only used the client. So I don't know if it is possible to test with Python's XML-RPC server.

As for your proposed improvement, it seems that should work. Keep in mind though that that would also catch the ConnectionRefusedError, which is currently raised on the first try. I'm not entirely sure when this is raised, but this might be reasonable behaviour as refusal should probably not be a one time thing. So if that is the case the new except should probably look like this:

except ConnectionError:
    if i or isinstance(ConnectionRefusedError):
        raise

This does definitely look cleaner than the two different except blocks IMHO. It also makes it clear why the new class is a subclass of ConnectionError.

[vadmium]

Here is patch with a test case. I kept Jelte’s original fix as it is, because I want to be conservative in the code changes.

Xmlrpc.server only uses HTTP 1.0, without any support of keep-alive connections, so I used http.server instead.

[python-dev]

New changeset d668b5595534 by Martin Panter in branch '3.5':
Issue bpo-26402: Fix XML-RPC client retrying after server disconnection
https://hg.python.org/cpython/rev/d668b5595534

New changeset 70bf0d764939 by Martin Panter in branch 'default':
Issue bpo-26402: Merge XML-RPC client fix from 3.5
https://hg.python.org/cpython/rev/70bf0d764939

[vadmium]

Thanks for reporting this Jelte

[JelteF]

No problem, I'm glad to have contributed something to the language I use the most.