SlackApiError cannot be unpickled

[youtux]

SlackApiError raises an error when trying to unpickle it.

This is particularly annoying in a Celery task since another bug in celery will cause the whole worker node to fail.

The Slack SDK version

2.9.4

Python runtime version

3.11.0

OS info

ProductName: macOS
ProductVersion: 13.0
BuildVersion: 22A380
Darwin Kernel Version 22.1.0: Sun Oct 9 20:15:09 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T6000

Steps to reproduce:

import pickle
from slack.errors import SlackApiError

e = SlackApiError("Internal server error", "<response>")

dumped = pickle.dumps(e)
pickle.loads(dumped) 

Expected result:

No error.

Actual result:

Traceback (most recent call last):
  File "/private/tmp/pickle_exc/bug.py", line 7, in <module>
    pickle.loads(dumped)
TypeError: SlackApiError.__init__() missing 1 required positional argument: 'response'

Solution

The SlackApiError could be implemented in such a way that the constructor accepts only the message, and the response defaults to None. Then the string representation can be delegated to __str__:

class SlackApiError(Exception):

    def __init__(self, message, response=None):
        super().__init__(message)
        self.response = response
    
    def __str__(self):
        return f"{self.args[0]}\nThe server responded with: {self.response}"

[seratch]

Hi @youtux, thanks for writing in!

First off, I would suggest catching the exception in your async code and generate some object or your own exception, which is serializable instead. The response object has its state for pagination (more specifically, reference to WebClient instance). So, we don't recommend trying to enable the object pickle-ready to avoid potential misbehavior.

Also, even if we decide to enhance this SDK for this use case in the future, the change won't come to slackclient v2. The reason is that, unfortunately, we are no longer actively working on the v2 SDK (except critical security fixes). It'd be appreciated if you could understand this.

[youtux]

First off, I would suggest catching the exception in your async code and generate some object or your own exception, which is serializable instead. The response object has its state for pagination (more specifically, reference to WebClient instance). So, we don't recommend trying to enable the object pickle-ready to avoid potential misbehavior.

I understand that, still it would greatly help if this exception was picklable, even if that means to lose pagination (you can remove the reference to the WebClient instance by overriding SlackApiError.__reduce_ex__) so that users that don't need to use pagination can have this behaviour fixed.

Also, even if we decide to enhance this SDK for this use case in the future, the change won't come to slackclient v2. The reason is that, unfortunately, we are no longer actively working on the v2 SDK (except critical security fixes). It'd be appreciated if you could understand this.

That wouldn't be a problem, I can upgrade to v3.

[youtux]

I can open a PR if you agree.

[github-actions]

👋 It looks like this issue has been open for 30 days with no activity. We'll mark this as stale for now, and wait 10 days for an update or for further comment before closing this issue out. If you think this issue needs to be prioritized, please comment to get the thread going again! Maintainers also review issues marked as stale on a regular basis and comment or adjust status if the issue needs to be reprioritized.

[youtux]

Still an issue

[seratch]

Hi @youtux, I've excluded this issue from the auto-triage process to repeat the same one month later. With that being said, we won't make the changes that you suggested at least in the short term (for the reason I mentioned above, plus due to your priorities as the whole team -- we're focusing on the next-generation platform project now). As I mentioned in my first rely, please consider a different approach for your use case.