From 97521986f93348d6f48afa2a819c84d572f1faf0 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Tue, 12 Aug 2025 17:00:03 -0400 Subject: [PATCH 1/6] fix: make the function handler timeout 5 seconds --- slack_bolt/app/app.py | 4 ++- slack_bolt/app/async_app.py | 4 ++- slack_bolt/listener/async_listener.py | 4 +++ slack_bolt/listener/asyncio_runner.py | 2 +- slack_bolt/listener/custom_listener.py | 3 ++ slack_bolt/listener/listener.py | 1 + slack_bolt/listener/thread_runner.py | 4 ++- tests/scenario_tests/test_function.py | 35 ++++++++++++++++++- tests/scenario_tests_async/test_function.py | 37 +++++++++++++++++++-- 9 files changed, 87 insertions(+), 7 deletions(-) diff --git a/slack_bolt/app/app.py b/slack_bolt/app/app.py index c117740a1..51b6ef69b 100644 --- a/slack_bolt/app/app.py +++ b/slack_bolt/app/app.py @@ -946,7 +946,7 @@ def reverse_string(ack: Ack, inputs: dict, complete: Complete, fail: Fail): def __call__(*args, **kwargs): functions = self._to_listener_functions(kwargs) if kwargs else list(args) primary_matcher = builtin_matchers.function_executed(callback_id=callback_id, base_logger=self._base_logger) - return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge) + return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, 5) return __call__ @@ -1422,6 +1422,7 @@ def _register_listener( matchers: Optional[Sequence[Callable[..., bool]]], middleware: Optional[Sequence[Union[Callable, Middleware]]], auto_acknowledgement: bool = False, + acknowledgement_timeout: int = 3, ) -> Optional[Callable[..., Optional[BoltResponse]]]: value_to_return = None if not isinstance(functions, list): @@ -1452,6 +1453,7 @@ def _register_listener( matchers=listener_matchers, middleware=listener_middleware, auto_acknowledgement=auto_acknowledgement, + acknowledgement_timeout=acknowledgement_timeout, base_logger=self._base_logger, ) ) diff --git a/slack_bolt/app/async_app.py b/slack_bolt/app/async_app.py index c04326291..6ed40b2ea 100644 --- a/slack_bolt/app/async_app.py +++ b/slack_bolt/app/async_app.py @@ -976,7 +976,7 @@ def __call__(*args, **kwargs): primary_matcher = builtin_matchers.function_executed( callback_id=callback_id, base_logger=self._base_logger, asyncio=True ) - return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge) + return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, 5) return __call__ @@ -1456,6 +1456,7 @@ def _register_listener( matchers: Optional[Sequence[Callable[..., Awaitable[bool]]]], middleware: Optional[Sequence[Union[Callable, AsyncMiddleware]]], auto_acknowledgement: bool = False, + acknowledgement_timeout: int = 3, ) -> Optional[Callable[..., Awaitable[Optional[BoltResponse]]]]: value_to_return = None if not isinstance(functions, list): @@ -1491,6 +1492,7 @@ def _register_listener( matchers=listener_matchers, middleware=listener_middleware, auto_acknowledgement=auto_acknowledgement, + acknowledgement_timeout=acknowledgement_timeout, base_logger=self._base_logger, ) ) diff --git a/slack_bolt/listener/async_listener.py b/slack_bolt/listener/async_listener.py index c8758daf2..ca069b097 100644 --- a/slack_bolt/listener/async_listener.py +++ b/slack_bolt/listener/async_listener.py @@ -15,6 +15,7 @@ class AsyncListener(metaclass=ABCMeta): ack_function: Callable[..., Awaitable[BoltResponse]] lazy_functions: Sequence[Callable[..., Awaitable[None]]] auto_acknowledgement: bool + acknowledgement_timeout: int async def async_matches( self, @@ -87,6 +88,7 @@ class AsyncCustomListener(AsyncListener): matchers: Sequence[AsyncListenerMatcher] middleware: Sequence[AsyncMiddleware] auto_acknowledgement: bool + acknowledgement_timeout: int arg_names: MutableSequence[str] logger: Logger @@ -99,6 +101,7 @@ def __init__( matchers: Sequence[AsyncListenerMatcher], middleware: Sequence[AsyncMiddleware], auto_acknowledgement: bool = False, + acknowledgement_timeout: int = 3, base_logger: Optional[Logger] = None, ): self.app_name = app_name @@ -107,6 +110,7 @@ def __init__( self.matchers = matchers self.middleware = middleware self.auto_acknowledgement = auto_acknowledgement + self.acknowledgement_timeout = acknowledgement_timeout self.arg_names = get_arg_names_of_callable(ack_function) self.logger = get_bolt_app_logger(app_name, self.ack_function, base_logger) diff --git a/slack_bolt/listener/asyncio_runner.py b/slack_bolt/listener/asyncio_runner.py index 56dc29cc1..98d3bf4f8 100644 --- a/slack_bolt/listener/asyncio_runner.py +++ b/slack_bolt/listener/asyncio_runner.py @@ -149,7 +149,7 @@ async def run_ack_function_asynchronously( self._start_lazy_function(lazy_func, request) # await for the completion of ack() in the async listener execution - while ack.response is None and time.time() - starting_time <= 3: + while ack.response is None and time.time() - starting_time <= listener.acknowledgement_timeout: await asyncio.sleep(0.01) if response is None and ack.response is None: diff --git a/slack_bolt/listener/custom_listener.py b/slack_bolt/listener/custom_listener.py index b785dab6d..e2977effa 100644 --- a/slack_bolt/listener/custom_listener.py +++ b/slack_bolt/listener/custom_listener.py @@ -18,6 +18,7 @@ class CustomListener(Listener): matchers: Sequence[ListenerMatcher] middleware: Sequence[Middleware] auto_acknowledgement: bool + acknowledgement_timeout: int = 3 arg_names: MutableSequence[str] logger: Logger @@ -30,6 +31,7 @@ def __init__( matchers: Sequence[ListenerMatcher], middleware: Sequence[Middleware], auto_acknowledgement: bool = False, + acknowledgement_timeout: int = 3, base_logger: Optional[Logger] = None, ): self.app_name = app_name @@ -38,6 +40,7 @@ def __init__( self.matchers = matchers self.middleware = middleware self.auto_acknowledgement = auto_acknowledgement + self.acknowledgement_timeout = acknowledgement_timeout self.arg_names = get_arg_names_of_callable(ack_function) self.logger = get_bolt_app_logger(app_name, self.ack_function, base_logger) diff --git a/slack_bolt/listener/listener.py b/slack_bolt/listener/listener.py index d938935df..51dadae56 100644 --- a/slack_bolt/listener/listener.py +++ b/slack_bolt/listener/listener.py @@ -13,6 +13,7 @@ class Listener(metaclass=ABCMeta): ack_function: Callable[..., BoltResponse] lazy_functions: Sequence[Callable[..., None]] auto_acknowledgement: bool + acknowledgement_timeout: int = 3 def matches( self, diff --git a/slack_bolt/listener/thread_runner.py b/slack_bolt/listener/thread_runner.py index c144daf1d..8f8288cf3 100644 --- a/slack_bolt/listener/thread_runner.py +++ b/slack_bolt/listener/thread_runner.py @@ -160,7 +160,9 @@ def run_ack_function_asynchronously(): self._start_lazy_function(lazy_func, request) # await for the completion of ack() in the async listener execution - while ack.response is None and time.time() - starting_time <= 3: + print(str(starting_time)) + while ack.response is None and time.time() - starting_time <= listener.acknowledgement_timeout: + print("we are sleeping") time.sleep(0.01) if response is None and ack.response is None: diff --git a/tests/scenario_tests/test_function.py b/tests/scenario_tests/test_function.py index 00f0efba8..41290de8f 100644 --- a/tests/scenario_tests/test_function.py +++ b/tests/scenario_tests/test_function.py @@ -1,6 +1,7 @@ import json import time import pytest +from unittest.mock import Mock from slack_sdk.signature import SignatureVerifier from slack_sdk.web import WebClient @@ -51,6 +52,10 @@ def build_request_from_body(self, message_body: dict) -> BoltRequest: timestamp, body = str(int(time.time())), json.dumps(message_body) return BoltRequest(body=body, headers=self.build_headers(timestamp, body)) + def setup_time_mocks(self, *, monkeypatch: pytest.MonkeyPatch, time_mock: Mock, sleep_mock: Mock): + monkeypatch.setattr(time, "time", time_mock) + monkeypatch.setattr(time, "sleep", sleep_mock) + def test_valid_callback_id_success(self): app = App( client=self.web_client, @@ -124,7 +129,7 @@ def test_auto_acknowledge_false_with_acknowledging(self): assert response.status == 200 assert_auth_test_count(self, 1) - def test_auto_acknowledge_false_without_acknowledging(self, caplog): + def test_auto_acknowledge_false_without_acknowledging(self, caplog, monkeypatch): app = App( client=self.web_client, signing_secret=self.signing_secret, @@ -132,12 +137,40 @@ def test_auto_acknowledge_false_without_acknowledging(self, caplog): app.function("reverse", auto_acknowledge=False)(just_no_ack) request = self.build_request_from_body(function_body) + self.setup_time_mocks( + monkeypatch=monkeypatch, + time_mock=Mock(side_effect=[current_time for current_time in range(100)]), + sleep_mock=Mock(), + ) response = app.dispatch(request) assert response.status == 404 assert_auth_test_count(self, 1) assert f"WARNING {just_no_ack.__name__} didn't call ack()" in caplog.text + def test_function_handler_timeout(self, monkeypatch): + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ) + app.function("reverse", auto_acknowledge=False)(just_no_ack) + request = self.build_request_from_body(function_body) + + sleep_mock = Mock() + self.setup_time_mocks( + monkeypatch=monkeypatch, + time_mock=Mock(side_effect=[current_time for current_time in range(100)]), + sleep_mock=sleep_mock, + ) + + response = app.dispatch(request) + + assert response.status == 404 + assert_auth_test_count(self, 1) + assert ( + sleep_mock.call_count == 5 + ), f"Expected handler to time out after calling time.sleep 5 times, but it was called {sleep_mock.call_count} times" + function_body = { "token": "verification_token", diff --git a/tests/scenario_tests_async/test_function.py b/tests/scenario_tests_async/test_function.py index a2c10950c..f76f88dd9 100644 --- a/tests/scenario_tests_async/test_function.py +++ b/tests/scenario_tests_async/test_function.py @@ -3,6 +3,7 @@ import time import pytest +from unittest.mock import AsyncMock, Mock from slack_sdk.signature import SignatureVerifier from slack_sdk.web.async_client import AsyncWebClient @@ -56,6 +57,10 @@ def build_request_from_body(self, message_body: dict) -> AsyncBoltRequest: timestamp, body = str(int(time.time())), json.dumps(message_body) return AsyncBoltRequest(body=body, headers=self.build_headers(timestamp, body)) + def setup_time_mocks(self, *, monkeypatch: pytest.MonkeyPatch, time_mock: Mock, sleep_mock: AsyncMock): + monkeypatch.setattr(time, "time", time_mock) + monkeypatch.setattr(asyncio, "sleep", sleep_mock) + @pytest.mark.asyncio async def test_mock_server_is_running(self): resp = await self.web_client.api_test() @@ -130,19 +135,47 @@ async def test_auto_acknowledge_false_with_acknowledging(self): await assert_auth_test_count_async(self, 1) @pytest.mark.asyncio - async def test_auto_acknowledge_false_without_acknowledging(self, caplog): + async def test_auto_acknowledge_false_without_acknowledging(self, caplog, monkeypatch): app = AsyncApp( client=self.web_client, signing_secret=self.signing_secret, ) app.function("reverse", auto_acknowledge=False)(just_no_ack) - request = self.build_request_from_body(function_body) + self.setup_time_mocks( + monkeypatch=monkeypatch, + time_mock=Mock(side_effect=[current_time for current_time in range(100)]), + sleep_mock=AsyncMock(), + ) response = await app.async_dispatch(request) assert response.status == 404 await assert_auth_test_count_async(self, 1) assert f"WARNING {just_no_ack.__name__} didn't call ack()" in caplog.text + @pytest.mark.asyncio + async def test_function_handler_timeout(self, monkeypatch): + app = AsyncApp( + client=self.web_client, + signing_secret=self.signing_secret, + ) + app.function("reverse", auto_acknowledge=False)(just_no_ack) + request = self.build_request_from_body(function_body) + + sleep_mock = AsyncMock() + self.setup_time_mocks( + monkeypatch=monkeypatch, + time_mock=Mock(side_effect=[current_time for current_time in range(100)]), + sleep_mock=sleep_mock, + ) + + response = await app.async_dispatch(request) + + assert response.status == 404 + await assert_auth_test_count_async(self, 1) + assert ( + sleep_mock.call_count == 5 + ), f"Expected handler to time out after calling time.sleep 5 times, but it was called {sleep_mock.call_count} times" + function_body = { "token": "verification_token", From 3cd54381bc027ee3569719873fc9e74dc858d647 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 13 Aug 2025 10:36:29 -0400 Subject: [PATCH 2/6] Update test_function.py --- tests/scenario_tests_async/test_function.py | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/tests/scenario_tests_async/test_function.py b/tests/scenario_tests_async/test_function.py index f76f88dd9..1f2749eb4 100644 --- a/tests/scenario_tests_async/test_function.py +++ b/tests/scenario_tests_async/test_function.py @@ -3,7 +3,7 @@ import time import pytest -from unittest.mock import AsyncMock, Mock +from unittest.mock import Mock, MagicMock from slack_sdk.signature import SignatureVerifier from slack_sdk.web.async_client import AsyncWebClient @@ -17,6 +17,8 @@ ) from tests.utils import remove_os_env_temporarily, restore_os_env +async def fake_sleep(seconds): + pass class TestAsyncFunction: signing_secret = "secret" @@ -57,7 +59,7 @@ def build_request_from_body(self, message_body: dict) -> AsyncBoltRequest: timestamp, body = str(int(time.time())), json.dumps(message_body) return AsyncBoltRequest(body=body, headers=self.build_headers(timestamp, body)) - def setup_time_mocks(self, *, monkeypatch: pytest.MonkeyPatch, time_mock: Mock, sleep_mock: AsyncMock): + def setup_time_mocks(self, *, monkeypatch: pytest.MonkeyPatch, time_mock: Mock, sleep_mock: MagicMock): monkeypatch.setattr(time, "time", time_mock) monkeypatch.setattr(asyncio, "sleep", sleep_mock) @@ -142,11 +144,13 @@ async def test_auto_acknowledge_false_without_acknowledging(self, caplog, monkey ) app.function("reverse", auto_acknowledge=False)(just_no_ack) request = self.build_request_from_body(function_body) + self.setup_time_mocks( monkeypatch=monkeypatch, time_mock=Mock(side_effect=[current_time for current_time in range(100)]), - sleep_mock=AsyncMock(), + sleep_mock=MagicMock(side_effect=fake_sleep), ) + response = await app.async_dispatch(request) assert response.status == 404 await assert_auth_test_count_async(self, 1) @@ -161,7 +165,7 @@ async def test_function_handler_timeout(self, monkeypatch): app.function("reverse", auto_acknowledge=False)(just_no_ack) request = self.build_request_from_body(function_body) - sleep_mock = AsyncMock() + sleep_mock = MagicMock(side_effect=fake_sleep) self.setup_time_mocks( monkeypatch=monkeypatch, time_mock=Mock(side_effect=[current_time for current_time in range(100)]), From 290017c085e11d68ce4efa8cc3ffc5727485388f Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 13 Aug 2025 17:17:23 -0400 Subject: [PATCH 3/6] Update slack_bolt/listener/thread_runner.py Co-authored-by: Eden Zimbelman --- slack_bolt/listener/thread_runner.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/slack_bolt/listener/thread_runner.py b/slack_bolt/listener/thread_runner.py index 8f8288cf3..61e8d6129 100644 --- a/slack_bolt/listener/thread_runner.py +++ b/slack_bolt/listener/thread_runner.py @@ -160,9 +160,7 @@ def run_ack_function_asynchronously(): self._start_lazy_function(lazy_func, request) # await for the completion of ack() in the async listener execution - print(str(starting_time)) while ack.response is None and time.time() - starting_time <= listener.acknowledgement_timeout: - print("we are sleeping") time.sleep(0.01) if response is None and ack.response is None: From 610d52b85aafb8fe4b86af7652866bfc8f77e86a Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 13 Aug 2025 17:18:06 -0400 Subject: [PATCH 4/6] Update slack_bolt/app/app.py Co-authored-by: Eden Zimbelman --- slack_bolt/app/app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slack_bolt/app/app.py b/slack_bolt/app/app.py index 51b6ef69b..9a9444c6c 100644 --- a/slack_bolt/app/app.py +++ b/slack_bolt/app/app.py @@ -946,7 +946,7 @@ def reverse_string(ack: Ack, inputs: dict, complete: Complete, fail: Fail): def __call__(*args, **kwargs): functions = self._to_listener_functions(kwargs) if kwargs else list(args) primary_matcher = builtin_matchers.function_executed(callback_id=callback_id, base_logger=self._base_logger) - return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, 5) + return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout=5) return __call__ From 08db9817d9d93aab31be249c49fef4915ad28857 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 13 Aug 2025 17:20:06 -0400 Subject: [PATCH 5/6] improve based on feedback --- slack_bolt/app/async_app.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/slack_bolt/app/async_app.py b/slack_bolt/app/async_app.py index 6ed40b2ea..a529f42ee 100644 --- a/slack_bolt/app/async_app.py +++ b/slack_bolt/app/async_app.py @@ -976,7 +976,7 @@ def __call__(*args, **kwargs): primary_matcher = builtin_matchers.function_executed( callback_id=callback_id, base_logger=self._base_logger, asyncio=True ) - return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, 5) + return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout=5) return __call__ From 21ab6c47461611900d95ae117530c931cc5e91eb Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Wed, 13 Aug 2025 17:24:26 -0400 Subject: [PATCH 6/6] fix linting issues --- slack_bolt/app/app.py | 4 +++- slack_bolt/app/async_app.py | 4 +++- tests/scenario_tests_async/test_function.py | 4 +++- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/slack_bolt/app/app.py b/slack_bolt/app/app.py index 9a9444c6c..86909ed18 100644 --- a/slack_bolt/app/app.py +++ b/slack_bolt/app/app.py @@ -946,7 +946,9 @@ def reverse_string(ack: Ack, inputs: dict, complete: Complete, fail: Fail): def __call__(*args, **kwargs): functions = self._to_listener_functions(kwargs) if kwargs else list(args) primary_matcher = builtin_matchers.function_executed(callback_id=callback_id, base_logger=self._base_logger) - return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout=5) + return self._register_listener( + functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout=5 + ) return __call__ diff --git a/slack_bolt/app/async_app.py b/slack_bolt/app/async_app.py index a529f42ee..294fb8b0c 100644 --- a/slack_bolt/app/async_app.py +++ b/slack_bolt/app/async_app.py @@ -976,7 +976,9 @@ def __call__(*args, **kwargs): primary_matcher = builtin_matchers.function_executed( callback_id=callback_id, base_logger=self._base_logger, asyncio=True ) - return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout=5) + return self._register_listener( + functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout=5 + ) return __call__ diff --git a/tests/scenario_tests_async/test_function.py b/tests/scenario_tests_async/test_function.py index 1f2749eb4..fc1299e55 100644 --- a/tests/scenario_tests_async/test_function.py +++ b/tests/scenario_tests_async/test_function.py @@ -17,9 +17,11 @@ ) from tests.utils import remove_os_env_temporarily, restore_os_env + async def fake_sleep(seconds): pass + class TestAsyncFunction: signing_secret = "secret" valid_token = "xoxb-valid" @@ -144,7 +146,7 @@ async def test_auto_acknowledge_false_without_acknowledging(self, caplog, monkey ) app.function("reverse", auto_acknowledge=False)(just_no_ack) request = self.build_request_from_body(function_body) - + self.setup_time_mocks( monkeypatch=monkeypatch, time_mock=Mock(side_effect=[current_time for current_time in range(100)]),