dispatchrun / dispatch-py

Python package to develop applications with Dispatch.
https://pypi.org/project/dispatch-py/
Apache License 2.0
56 stars 3 forks source link

Pickling of Python exceptions with constructors taking more than one argument is broken #84

Open achille-roussel opened 8 months ago

achille-roussel commented 8 months ago

This is an error that manifests with this error:

cls = <class 'dispatch.proto.Error'>
proto = type: "HTTPStatusError"
message: "test"
value: "\200\004\225\206\000\000\000\000\000\000\000\214\005httpx\224\214\017H...econstruct_httpx_error\n    raise HTTPStatusError(\"test\", request=res, response=res)\nhttpx.HTTPStatusError: test\n"

    @classmethod
    def _from_proto(cls, proto: error_pb.Error) -> Error:
>       value = pickle.loads(proto.value) if proto.value else None
E       TypeError: HTTPStatusError.__init__() missing 2 required keyword-only arguments: 'request' and 'response'

src/dispatch/proto.py:369: TypeError

This problem is common in Python, subclasses of Exception that have constructors with more than one argument cannot be unpickled.

@chriso and I looked into creating a wrapper that would allow universally pickling values, this is how far we've got:

class Wrapper:

    def __init__(self, ex, wrappers = {}):
        self.ex = ex
        self._wrappers = wrappers
        self._wrappers[id(ex)] = self

    def __getstate__(self):
        if not hasattr(self.ex, '__dict__'):
            return type(self.ex), self.ex
        cls, state = type(self.ex), self.ex.__dict__
        state = { k: self.wrap(v) for k, v in state.items() }
        return cls, state

    def __setstate__(self, args):
        cls, state = args
        self.ex = cls.__new__(cls)
        if hasattr(self.ex, '__dict__'):
            state = { k: v.ex for k, v in state.items() }
            self.ex.__dict__.update(state)
        else:
            self.ex = state

    def wrap(self, obj):
        objid = id(obj)

        if objid in self._wrappers:
            return self._wrappers[objid]

        wrapped = Wrapper(obj, self._wrappers)
        self._wrappers[objid] = wrapped
        return wrapped

One issue with the approach is that it doesn't handle the class state that's in __slots__, we need to go a little further to make it work.

achille-roussel commented 8 months ago

The specific issue is tracked upstream in https://github.com/encode/httpx/discussions/1987

achille-roussel commented 8 months ago

It looks like someone took the initiative to address the problem in httpx https://github.com/encode/httpx/pull/3108