Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add param to enable "Auto-Acknowledgment" of Events to the SocketModeClient #1299

Open
amshamah419 opened this issue Nov 14, 2022 · 1 comment
Labels
discussion M-T: An issue where more input is needed to reach a decision enhancement M-T: A feature request for new functionality socket-mode Version: 3x
Milestone

Comments

@amshamah419
Copy link

amshamah419 commented Nov 14, 2022

One issue we have noticed in our deployment of a Slack app is that when there are a burst of messages being received, the built-in queue can become too large and can cause messages to expire within the 3-5 second acknowledgment period.

For example, our app receives 200 messages within a 5 second window. They are then received and queued for the message listeners to process. If the queue gets too large, it may take longer than 3-5 seconds for a listener to dequeue and process a message. This results in the app being restricted and unable to receive new messages. This can get exponentially worse when the burst is followed up by a burst of retry messages as well.

Reproducible in:

The Slack SDK version

slack-sdk==3.19.2

Python runtime version

Python 3.10.5

OS info

ProductName: macOS
ProductVersion: 11.5.2
BuildVersion: 20G95
Darwin Kernel Version 20.6.0: Wed Jun 23 00:26:31 PDT 2021; root:xnu-7195.141.2~5/RELEASE_X86_64

Steps to reproduce:

(Share the commands to run, source code, and project settings (e.g., setup.py))

  1. Setup async socket client
  2. Append one listener
  3. Receive burst of events

Expected result:

So the thought was to set a parameter which would allow messages to be auto-acknowledged as they enter the queue. This would allow the listeners to handle the events irrespective of their processing time.

It could work something like this:

    async def enqueue_message(self, message: str):
        await self.message_queue.put(message)
        if self.auto_acknowledge_messages:
            await self.auto_acknowledge_message(raw_message=message)
        if self.logger.level <= logging.DEBUG:
            queue_size = self.message_queue.qsize()
            session_id = await self.session_id()
            self.logger.debug(f"A new message enqueued (current queue size: {queue_size}, session: {session_id})")
            
    async def auto_acknowledge_message(self, raw_message):
        if raw_message.startswith("{"):
            message = json.loads(raw_message)
            if message.envelope_id:
                response = SocketModeResponse(envelope_id=message.envelope_id)
                await self.send_socket_mode_response(response)

Actual result:

Socket connection is closed due to missing too many acknowledgements.

Requirements

For general questions/issues about Slack API platform or its server-side, could you submit questions at https://my.slack.com/help/requests/new instead. 🙇

Please read the Contributing guidelines and Code of Conduct before creating this issue or pull request. By submitting, you are agreeing to those rules.

@seratch seratch added enhancement M-T: A feature request for new functionality discussion M-T: An issue where more input is needed to reach a decision Version: 3x socket-mode labels Nov 14, 2022
@seratch seratch added this to the 3.x milestone Nov 14, 2022
@seratch
Copy link
Member

seratch commented Nov 14, 2022

Hi @amshamah419, thanks for writing in!

Indeed, this situation can arise when all of the following are true:

  • your app listeners take long to complete
  • your app uses the default concurrency setting (10)

Adding the auto-ack option is one possible solution but before adding it, could you increase the "concurrency" from 10 to 100 or even larger number? With that, your situation should be resolved.

The downside of the auto-ack option is that, when the flag is true, your app is unable to handle interactivity and slash command patterns. Therefore, we hesitate to add this option. My different idea to add this option is enabling auto-ack only for events_api payload patterns. The option name can be auto_ack_events_api_requests or something like that. With this, your situation can be resolved and we can avoid confusion around interactivity.

Either way, before working on the change, I'd suggest adjusting your concurrency first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion M-T: An issue where more input is needed to reach a decision enhancement M-T: A feature request for new functionality socket-mode Version: 3x
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants