ClientConnectionError exception doesn't serialize properly

[davidparks21]

Long story short

ClientConnectionError exception doesn't serialize propertly

Expected behaviour

To be able to add ClientConnectError to a queue and get it back.

Actual behaviour

AttributeError: 'str' object has no attribute 'errno'

Steps to reproduce

A similar issue is reproducible using the following code snippet:

import aiohttp
import pickle


connection_key = aiohttp.client_reqrep.ConnectionKey
ose = OSError(1, 'unittest')
cce = aiohttp.client_exceptions.ClientConnectorError(connection_key, ose)
cce_pickled = pickle.dumps(cce)
pickle.loads(cce_pickled)

Result:

Traceback (most recent call last):
  File "/opt/.pycharm_helpers/pydev/pydevd.py", line 1758, in <module>
    main()
  File "/opt/.pycharm_helpers/pydev/pydevd.py", line 1752, in main
    globals = debugger.run(setup['file'], None, None, is_module)
  File "/opt/.pycharm_helpers/pydev/pydevd.py", line 1147, in run
    pydev_imports.execfile(file, globals, locals)  # execute the script
  File "/opt/.pycharm_helpers/pydev/_pydev_imps/_pydev_execfile.py", line 18, in execfile
    exec(compile(contents+"\n", file, 'exec'), glob, loc)
  File "/project_neural_mouse/src/ARCHIVE/scratch/scratch_13.py", line 11, in <module>
  File "/usr/local/lib/python3.6/dist-packages/aiohttp/client_exceptions.py", line 133, in __init__
    super().__init__(os_error.errno, os_error.strerror)
AttributeError: 'str' object has no attribute 'errno'

Your environment

Python 3.6.8, aiohttp.version == 3.6.0

Additional detail

This issue is posted on StackOverflow here:

https://stackoverflow.com/questions/58019939/attributeerror-str-object-has-no-attribute-errno?noredirect=1#comment102443264_58019939

I placed a ClientConnectionError exception in a multiprocessing.Queue that was generated by asyncio. I did this to pass an exception generated in asyncio land back to a client in another thread/process.

This exception appears to occurred during the deserialization process reading the exception out of the queue. It looks pretty much impossible to reach otherwise.

Traceback (most recent call last):
  File "model_neural_simplified.py", line 318, in <module>
    main(**arg_parser())
  File "model_neural_simplified.py", line 314, in main
    globals()[command](**kwargs)
  File "model_neural_simplified.py", line 304, in predict
    next_neural_data, next_sample = reader.get_next_result()
  File "/project_neural_mouse/src/asyncs3/s3reader.py", line 174, in get_next_result
    result = future.result()
  File "/usr/lib/python3.6/concurrent/futures/_base.py", line 432, in result
    return self.__get_result()
  File "/usr/lib/python3.6/concurrent/futures/_base.py", line 384, in __get_result
    raise self._exception
  File "/usr/lib/python3.6/concurrent/futures/thread.py", line 56, in run
    result = self.fn(*self.args, **self.kwargs)
  File "model_neural_simplified.py", line 245, in read_sample
    f_bytes = s3f.read(read_size)
  File "/project_neural_mouse/src/asyncs3/s3reader.py", line 374, in read
    size, b = self._issue_request(S3Reader.READ, (self.url, size, self.position))
  File "/project_neural_mouse/src/asyncs3/s3reader.py", line 389, in _issue_request
    response = self.communication_channels[uuid].get()
  File "/usr/lib/python3.6/multiprocessing/queues.py", line 113, in get
    return _ForkingPickler.loads(res)
  File "/usr/local/lib/python3.6/dist-packages/aiohttp/client_exceptions.py", line 133, in __init__
    super().__init__(os_error.errno, os_error.strerror)
AttributeError: 'str' object has no attribute 'errno'

The minimally reproducible example posted above generates a similar error as the one shown here that I experienced in practice.

[asvetlov]

Pull request is welcome!

[davidparks21]

I updated the minimum reproducible test case based on some comments over at stack overflow. But in looking at it I'm not clear if this is an aiohttp bug or a pickle bug (I'm no pickle expert). I don't actually see any obvious bug in the aiohttp code. It seems like it should work based on the rules of python as far as I'm familiar.

Before pickling ClientConnectionError._os_error is a PermissionError object (a subclass of OSError), not a string. After pickling it appears that this gets converted to a string instead of back to a PermissionError.

I think the right course of action is to throw it over the fence to the pickle maintainers. Does that seem correct?

[davidparks21]

I created the issue over at bugs.python.org, we'll see what happens there.

https://bugs.python.org/issue38254

[davidparks21]

Indeed xtreak at bugs.pythong.org posted a pickle only test case that reproduces this.

import pickle

ose = OSError(1, 'unittest')

class SubOSError(OSError):

    def __init__(self, foo, os_error):
        super().__init__(os_error.errno, os_error.strerror)

cce = SubOSError(1, ose)
cce_pickled = pickle.dumps(cce)
pickle.loads(cce_pickled)


./python.exe ../backups/bpo38254.py
Traceback (most recent call last):
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/../backups/bpo38254.py", line 12, in <module>
    pickle.loads(cce_pickled)
  File "/Users/karthikeyansingaravelan/stuff/python/cpython/../backups/bpo38254.py", line 8, in __init__
    super().__init__(os_error.errno, os_error.strerror)
AttributeError: 'str' object has no attribute 'errno'

[tirkarthi]

As noted in the SO answer linked in the bpo this is regarding OSError having a __reduce__ implementation which doesn't accommodate for subclass with different constructor signature here in aiohttp and aiohttp would need to add a custom __reduce__ like the one in SO answer.

[davidparks21]

Update: The open question on stack overflow received an answer and solution to the problem. It was a subtlety of OSError which implements a custom __reduce__ function for serialization. When the signature of a descendant class changes it must also implement __reduce__ to provide custom serialization.

Ooof, that was tricky. But now that it's understood, and it's fixable in aiohttp, I've closed the bugs.pythong.org issue and I'll see about sending a pull request later this week probably.

[serhiy-storchaka]

I am already working on this issue since yesterday.

There are many exception classes with custom constructor in in aiohttp, and they are organized in complex hierarhies. There is more than one way to solve this issue. Different ways can be more appropriate in every particular case.

• Set correct args. But it affects also __str__ and __repr__.
• Implement custom __reduce__. It is not easy to make it subclass friendly.
• Restore the default general __reduce__. Needs to implement __getnewargs__ or __getstate__ to ignore internal state of BaseException.

[davidparks21]

Excellent, I'll leave it to you then. Thanks @serhiy-storchaka !