Make HttpStatusError and RequestError pickleable

[hoodmane]

trafficstars

If an error has keyword-only arguments, it needs this extra __reduce__ method to ensure that unpickling doesn't raise TypeError: missing (n) required keyword-only argument(s).

Checklist

• [x] I understand that this PR may be closed in case there was no previous discussion. (This doesn't apply to typos!)
• [x] I've added a test for each change that was introduced, and I tried as much as possible to make a single atomic change.
• [x] I've updated the documentation accordingly.

[lovelydinosaur]

Worth adding a CHANGELOG entry for this.

[hoodmane]

Okay updated changelog.

[camillol]

The root issue is that BaseException has a custom reduce that can't handle subclasses with required keyword-only arguments. Overriding reduce is the right fix (barring a fix in Python), but I think it only needs to be in HttpStatusError. RequestError can already be pickled because its keyword argument has a default.

More importantly, I'd like to see this merged!

[lovelydinosaur]

More importantly, I'd like to see this merged!

Okay, I'll rephrase my reservations and let's see if we can get this unblocked.

I'd prefer to accept a __getstate__/__setstate__ implementation to handle the pickling.

See https://docs.python.org/3/library/pickle.html#object.setstate

Although powerful, implementing reduce() directly in your classes is error prone. For this reason, class designers should use the high-level interface (i.e., getnewargs_ex(), getstate() and setstate()) whenever possible.

[hoodmane]

I see, I didn't understand from what you said previously that this PR was waiting on me. I'll look into it.

[hoodmane]

No it is not possible to implement this with __getstate__ and __setstate__. object.__reduce__ is responsible for calling __getstate__, but BaseException.__reduce__ does not call __getstate__ -- all it does is directly return a tuple:

static PyObject *
BaseException_reduce(PyBaseExceptionObject *self, PyObject *Py_UNUSED(ignored))
{
    if (self->args && self->dict)
        return PyTuple_Pack(3, Py_TYPE(self), self->args, self->dict);
    else
        return PyTuple_Pack(2, Py_TYPE(self), self->args);
}

So in order to fix the behavior we'll have to override BaseException.__reduce__.

The ordinary object.__reduce__ is implemented here, it just calls _common_reduce directly above which just calls reduce_newobj which finally implements the actual logic. On line 6266 reduce_newobj calls a function named object_getstate. object_getstate looks up __getstate__ and calls it if present, if absent it falls back to object_getstate_default.

Anyways none of this happens if __reduce__ is overridden and doesn't call super().__reduce__, such as when inheriting from BaseException.

[hoodmane]

As @camillol said:

The root issue is that BaseException has a custom __reduce__ that can't handle subclasses with required keyword-only arguments. Overriding reduce is the right fix (barring a fix in Python)