From a81bade3160fa80a236b3baf9f0ee57f160fd8be Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Thu, 21 Aug 2025 12:42:19 -0400 Subject: [PATCH 1/2] feat: surface acknowledgement_timeout on the handler --- slack_bolt/app/app.py | 8 +++++- slack_bolt/app/async_app.py | 7 ++++- slack_bolt/logger/messages.py | 9 ++++++ tests/scenario_tests/test_function.py | 31 ++++++++++++++++++-- tests/scenario_tests_async/test_function.py | 32 +++++++++++++++++++-- 5 files changed, 81 insertions(+), 6 deletions(-) diff --git a/slack_bolt/app/app.py b/slack_bolt/app/app.py index 86909ed18..8fd298a10 100644 --- a/slack_bolt/app/app.py +++ b/slack_bolt/app/app.py @@ -59,6 +59,7 @@ info_default_oauth_settings_loaded, error_installation_store_required_for_builtin_listeners, warning_unhandled_by_global_middleware, + warning_ack_timeout_has_no_effect, ) from slack_bolt.middleware import ( Middleware, @@ -912,6 +913,7 @@ def function( matchers: Optional[Sequence[Callable[..., bool]]] = None, middleware: Optional[Sequence[Union[Callable, Middleware]]] = None, auto_acknowledge: bool = True, + acknowledgement_timeout: int = 3, ) -> Callable[..., Optional[Callable[..., Optional[BoltResponse]]]]: """Registers a new Function listener. This method can be used as either a decorator or a method. @@ -940,6 +942,10 @@ def reverse_string(ack: Ack, inputs: dict, complete: Complete, fail: Fail): Only when all the middleware call `next()` method, the listener function can be invoked. """ + if auto_acknowledge is True: + if acknowledgement_timeout != 3: + self._framework_logger.warning(warning_ack_timeout_has_no_effect(callback_id, acknowledgement_timeout)) + matchers = list(matchers) if matchers else [] middleware = list(middleware) if middleware else [] @@ -947,7 +953,7 @@ 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 + functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout ) return __call__ diff --git a/slack_bolt/app/async_app.py b/slack_bolt/app/async_app.py index 294fb8b0c..4532084bb 100644 --- a/slack_bolt/app/async_app.py +++ b/slack_bolt/app/async_app.py @@ -68,6 +68,7 @@ info_default_oauth_settings_loaded, error_installation_store_required_for_builtin_listeners, warning_unhandled_by_global_middleware, + warning_ack_timeout_has_no_effect, ) from slack_bolt.lazy_listener.asyncio_runner import AsyncioLazyListenerRunner from slack_bolt.listener.async_listener import AsyncListener, AsyncCustomListener @@ -940,6 +941,7 @@ def function( matchers: Optional[Sequence[Callable[..., Awaitable[bool]]]] = None, middleware: Optional[Sequence[Union[Callable, AsyncMiddleware]]] = None, auto_acknowledge: bool = True, + acknowledgement_timeout: int = 3, ) -> Callable[..., Optional[Callable[..., Awaitable[BoltResponse]]]]: """Registers a new Function listener. This method can be used as either a decorator or a method. @@ -967,6 +969,9 @@ async def reverse_string(ack: AsyncAck, inputs: dict, complete: AsyncComplete, f middleware: A list of lister middleware functions. Only when all the middleware call `next()` method, the listener function can be invoked. """ + if auto_acknowledge is True: + if acknowledgement_timeout != 3: + self._framework_logger.warning(warning_ack_timeout_has_no_effect(callback_id, acknowledgement_timeout)) matchers = list(matchers) if matchers else [] middleware = list(middleware) if middleware else [] @@ -977,7 +982,7 @@ def __call__(*args, **kwargs): 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 + functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout ) return __call__ diff --git a/slack_bolt/logger/messages.py b/slack_bolt/logger/messages.py index 3ec1fef8a..fa04bee45 100644 --- a/slack_bolt/logger/messages.py +++ b/slack_bolt/logger/messages.py @@ -1,3 +1,4 @@ +from re import Pattern import time from typing import Union, Dict, Any, Optional @@ -331,6 +332,14 @@ def warning_skip_uncommon_arg_name(arg_name: str) -> str: ) +def warning_ack_timeout_has_no_effect(identifier: Union[str, Pattern], acknowledgement_timeout: int) -> str: + handler_example = f'@app.function("{identifier}")' if isinstance(identifier, str) else f"@app.function({identifier})" + return ( + f"On {handler_example}, as `auto_acknowledge` is `True`, " + f"`acknowledgement_timeout={acknowledgement_timeout}` you gave will be unused" + ) + + # ------------------------------- # Info # ------------------------------- diff --git a/tests/scenario_tests/test_function.py b/tests/scenario_tests/test_function.py index 41290de8f..c1b7e057a 100644 --- a/tests/scenario_tests/test_function.py +++ b/tests/scenario_tests/test_function.py @@ -1,4 +1,5 @@ import json +import re import time import pytest from unittest.mock import Mock @@ -149,11 +150,12 @@ def test_auto_acknowledge_false_without_acknowledging(self, caplog, monkeypatch) assert f"WARNING {just_no_ack.__name__} didn't call ack()" in caplog.text def test_function_handler_timeout(self, monkeypatch): + timeout = 5 app = App( client=self.web_client, signing_secret=self.signing_secret, ) - app.function("reverse", auto_acknowledge=False)(just_no_ack) + app.function("reverse", auto_acknowledge=False, acknowledgement_timeout=timeout)(just_no_ack) request = self.build_request_from_body(function_body) sleep_mock = Mock() @@ -168,9 +170,34 @@ def test_function_handler_timeout(self, monkeypatch): assert response.status == 404 assert_auth_test_count(self, 1) assert ( - sleep_mock.call_count == 5 + sleep_mock.call_count == timeout ), f"Expected handler to time out after calling time.sleep 5 times, but it was called {sleep_mock.call_count} times" + def test_warning_when_timeout_improperly_set(self, caplog): + app = App( + client=self.web_client, + signing_secret=self.signing_secret, + ) + app.function("reverse")(just_no_ack) + assert "WARNING" not in caplog.text + + timeout_argument_name = "acknowledgement_timeout" + kwargs = {timeout_argument_name: 5} + + callback_id = "reverse1" + app.function(callback_id, **kwargs)(just_no_ack) + assert ( + f'WARNING On @app.function("{callback_id}"), as `auto_acknowledge` is `True`, `{timeout_argument_name}={kwargs[timeout_argument_name]}` you gave will be unused' + in caplog.text + ) + + callback_id = re.compile(r"hello \w+") + app.function(callback_id, **kwargs)(just_no_ack) + assert ( + f"WARNING On @app.function({callback_id}), as `auto_acknowledge` is `True`, `{timeout_argument_name}={kwargs[timeout_argument_name]}` you gave will be unused" + in caplog.text + ) + function_body = { "token": "verification_token", diff --git a/tests/scenario_tests_async/test_function.py b/tests/scenario_tests_async/test_function.py index fc1299e55..4af4024cf 100644 --- a/tests/scenario_tests_async/test_function.py +++ b/tests/scenario_tests_async/test_function.py @@ -1,5 +1,6 @@ import asyncio import json +import re import time import pytest @@ -160,11 +161,12 @@ async def test_auto_acknowledge_false_without_acknowledging(self, caplog, monkey @pytest.mark.asyncio async def test_function_handler_timeout(self, monkeypatch): + timeout = 5 app = AsyncApp( client=self.web_client, signing_secret=self.signing_secret, ) - app.function("reverse", auto_acknowledge=False)(just_no_ack) + app.function("reverse", auto_acknowledge=False, acknowledgement_timeout=timeout)(just_no_ack) request = self.build_request_from_body(function_body) sleep_mock = MagicMock(side_effect=fake_sleep) @@ -179,9 +181,35 @@ async def test_function_handler_timeout(self, monkeypatch): assert response.status == 404 await assert_auth_test_count_async(self, 1) assert ( - sleep_mock.call_count == 5 + sleep_mock.call_count == timeout ), f"Expected handler to time out after calling time.sleep 5 times, but it was called {sleep_mock.call_count} times" + @pytest.mark.asyncio + async def test_warning_when_timeout_improperly_set(self, caplog): + app = AsyncApp( + client=self.web_client, + signing_secret=self.signing_secret, + ) + app.function("reverse")(just_no_ack) + assert "WARNING" not in caplog.text + + timeout_argument_name = "acknowledgement_timeout" + kwargs = {timeout_argument_name: 5} + + callback_id = "reverse1" + app.function(callback_id, **kwargs)(just_no_ack) + assert ( + f'WARNING On @app.function("{callback_id}"), as `auto_acknowledge` is `True`, `{timeout_argument_name}={kwargs[timeout_argument_name]}` you gave will be unused' + in caplog.text + ) + + callback_id = re.compile(r"hello \w+") + app.function(callback_id, **kwargs)(just_no_ack) + assert ( + f"WARNING On @app.function({callback_id}), as `auto_acknowledge` is `True`, `{timeout_argument_name}={kwargs[timeout_argument_name]}` you gave will be unused" + in caplog.text + ) + function_body = { "token": "verification_token", From 259c769446a3413f07329ff88e52a95d00580eb7 Mon Sep 17 00:00:00 2001 From: William Bergamin Date: Fri, 22 Aug 2025 14:47:27 -0400 Subject: [PATCH 2/2] rename acknowledgement_timeout to ack_timeout --- slack_bolt/app/app.py | 14 ++++++-------- slack_bolt/app/async_app.py | 14 ++++++-------- slack_bolt/listener/async_listener.py | 8 ++++---- slack_bolt/listener/asyncio_runner.py | 2 +- slack_bolt/listener/custom_listener.py | 6 +++--- slack_bolt/listener/listener.py | 2 +- slack_bolt/listener/thread_runner.py | 2 +- slack_bolt/logger/messages.py | 7 ++----- tests/scenario_tests/test_function.py | 4 ++-- tests/scenario_tests_async/test_function.py | 4 ++-- 10 files changed, 28 insertions(+), 35 deletions(-) diff --git a/slack_bolt/app/app.py b/slack_bolt/app/app.py index 8fd298a10..60f20ea9e 100644 --- a/slack_bolt/app/app.py +++ b/slack_bolt/app/app.py @@ -913,7 +913,7 @@ def function( matchers: Optional[Sequence[Callable[..., bool]]] = None, middleware: Optional[Sequence[Union[Callable, Middleware]]] = None, auto_acknowledge: bool = True, - acknowledgement_timeout: int = 3, + ack_timeout: int = 3, ) -> Callable[..., Optional[Callable[..., Optional[BoltResponse]]]]: """Registers a new Function listener. This method can be used as either a decorator or a method. @@ -943,8 +943,8 @@ def reverse_string(ack: Ack, inputs: dict, complete: Complete, fail: Fail): """ if auto_acknowledge is True: - if acknowledgement_timeout != 3: - self._framework_logger.warning(warning_ack_timeout_has_no_effect(callback_id, acknowledgement_timeout)) + if ack_timeout != 3: + self._framework_logger.warning(warning_ack_timeout_has_no_effect(callback_id, ack_timeout)) matchers = list(matchers) if matchers else [] middleware = list(middleware) if middleware else [] @@ -952,9 +952,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, acknowledgement_timeout - ) + return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, ack_timeout) return __call__ @@ -1430,7 +1428,7 @@ def _register_listener( matchers: Optional[Sequence[Callable[..., bool]]], middleware: Optional[Sequence[Union[Callable, Middleware]]], auto_acknowledgement: bool = False, - acknowledgement_timeout: int = 3, + ack_timeout: int = 3, ) -> Optional[Callable[..., Optional[BoltResponse]]]: value_to_return = None if not isinstance(functions, list): @@ -1461,7 +1459,7 @@ def _register_listener( matchers=listener_matchers, middleware=listener_middleware, auto_acknowledgement=auto_acknowledgement, - acknowledgement_timeout=acknowledgement_timeout, + ack_timeout=ack_timeout, base_logger=self._base_logger, ) ) diff --git a/slack_bolt/app/async_app.py b/slack_bolt/app/async_app.py index 4532084bb..906359fcc 100644 --- a/slack_bolt/app/async_app.py +++ b/slack_bolt/app/async_app.py @@ -941,7 +941,7 @@ def function( matchers: Optional[Sequence[Callable[..., Awaitable[bool]]]] = None, middleware: Optional[Sequence[Union[Callable, AsyncMiddleware]]] = None, auto_acknowledge: bool = True, - acknowledgement_timeout: int = 3, + ack_timeout: int = 3, ) -> Callable[..., Optional[Callable[..., Awaitable[BoltResponse]]]]: """Registers a new Function listener. This method can be used as either a decorator or a method. @@ -970,8 +970,8 @@ async def reverse_string(ack: AsyncAck, inputs: dict, complete: AsyncComplete, f Only when all the middleware call `next()` method, the listener function can be invoked. """ if auto_acknowledge is True: - if acknowledgement_timeout != 3: - self._framework_logger.warning(warning_ack_timeout_has_no_effect(callback_id, acknowledgement_timeout)) + if ack_timeout != 3: + self._framework_logger.warning(warning_ack_timeout_has_no_effect(callback_id, ack_timeout)) matchers = list(matchers) if matchers else [] middleware = list(middleware) if middleware else [] @@ -981,9 +981,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, acknowledgement_timeout - ) + return self._register_listener(functions, primary_matcher, matchers, middleware, auto_acknowledge, ack_timeout) return __call__ @@ -1463,7 +1461,7 @@ def _register_listener( matchers: Optional[Sequence[Callable[..., Awaitable[bool]]]], middleware: Optional[Sequence[Union[Callable, AsyncMiddleware]]], auto_acknowledgement: bool = False, - acknowledgement_timeout: int = 3, + ack_timeout: int = 3, ) -> Optional[Callable[..., Awaitable[Optional[BoltResponse]]]]: value_to_return = None if not isinstance(functions, list): @@ -1499,7 +1497,7 @@ def _register_listener( matchers=listener_matchers, middleware=listener_middleware, auto_acknowledgement=auto_acknowledgement, - acknowledgement_timeout=acknowledgement_timeout, + ack_timeout=ack_timeout, base_logger=self._base_logger, ) ) diff --git a/slack_bolt/listener/async_listener.py b/slack_bolt/listener/async_listener.py index ca069b097..0810b91a7 100644 --- a/slack_bolt/listener/async_listener.py +++ b/slack_bolt/listener/async_listener.py @@ -15,7 +15,7 @@ class AsyncListener(metaclass=ABCMeta): ack_function: Callable[..., Awaitable[BoltResponse]] lazy_functions: Sequence[Callable[..., Awaitable[None]]] auto_acknowledgement: bool - acknowledgement_timeout: int + ack_timeout: int async def async_matches( self, @@ -88,7 +88,7 @@ class AsyncCustomListener(AsyncListener): matchers: Sequence[AsyncListenerMatcher] middleware: Sequence[AsyncMiddleware] auto_acknowledgement: bool - acknowledgement_timeout: int + ack_timeout: int arg_names: MutableSequence[str] logger: Logger @@ -101,7 +101,7 @@ def __init__( matchers: Sequence[AsyncListenerMatcher], middleware: Sequence[AsyncMiddleware], auto_acknowledgement: bool = False, - acknowledgement_timeout: int = 3, + ack_timeout: int = 3, base_logger: Optional[Logger] = None, ): self.app_name = app_name @@ -110,7 +110,7 @@ def __init__( self.matchers = matchers self.middleware = middleware self.auto_acknowledgement = auto_acknowledgement - self.acknowledgement_timeout = acknowledgement_timeout + self.ack_timeout = ack_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 98d3bf4f8..81f9e6106 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 <= listener.acknowledgement_timeout: + while ack.response is None and time.time() - starting_time <= listener.ack_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 e2977effa..2b73018db 100644 --- a/slack_bolt/listener/custom_listener.py +++ b/slack_bolt/listener/custom_listener.py @@ -18,7 +18,7 @@ class CustomListener(Listener): matchers: Sequence[ListenerMatcher] middleware: Sequence[Middleware] auto_acknowledgement: bool - acknowledgement_timeout: int = 3 + ack_timeout: int = 3 arg_names: MutableSequence[str] logger: Logger @@ -31,7 +31,7 @@ def __init__( matchers: Sequence[ListenerMatcher], middleware: Sequence[Middleware], auto_acknowledgement: bool = False, - acknowledgement_timeout: int = 3, + ack_timeout: int = 3, base_logger: Optional[Logger] = None, ): self.app_name = app_name @@ -40,7 +40,7 @@ def __init__( self.matchers = matchers self.middleware = middleware self.auto_acknowledgement = auto_acknowledgement - self.acknowledgement_timeout = acknowledgement_timeout + self.ack_timeout = ack_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 51dadae56..7685f3c7b 100644 --- a/slack_bolt/listener/listener.py +++ b/slack_bolt/listener/listener.py @@ -13,7 +13,7 @@ class Listener(metaclass=ABCMeta): ack_function: Callable[..., BoltResponse] lazy_functions: Sequence[Callable[..., None]] auto_acknowledgement: bool - acknowledgement_timeout: int = 3 + ack_timeout: int = 3 def matches( self, diff --git a/slack_bolt/listener/thread_runner.py b/slack_bolt/listener/thread_runner.py index 61e8d6129..378ca1bfa 100644 --- a/slack_bolt/listener/thread_runner.py +++ b/slack_bolt/listener/thread_runner.py @@ -160,7 +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 - while ack.response is None and time.time() - starting_time <= listener.acknowledgement_timeout: + while ack.response is None and time.time() - starting_time <= listener.ack_timeout: time.sleep(0.01) if response is None and ack.response is None: diff --git a/slack_bolt/logger/messages.py b/slack_bolt/logger/messages.py index fa04bee45..cffdc445f 100644 --- a/slack_bolt/logger/messages.py +++ b/slack_bolt/logger/messages.py @@ -332,12 +332,9 @@ def warning_skip_uncommon_arg_name(arg_name: str) -> str: ) -def warning_ack_timeout_has_no_effect(identifier: Union[str, Pattern], acknowledgement_timeout: int) -> str: +def warning_ack_timeout_has_no_effect(identifier: Union[str, Pattern], ack_timeout: int) -> str: handler_example = f'@app.function("{identifier}")' if isinstance(identifier, str) else f"@app.function({identifier})" - return ( - f"On {handler_example}, as `auto_acknowledge` is `True`, " - f"`acknowledgement_timeout={acknowledgement_timeout}` you gave will be unused" - ) + return f"On {handler_example}, as `auto_acknowledge` is `True`, " f"`ack_timeout={ack_timeout}` you gave will be unused" # ------------------------------- diff --git a/tests/scenario_tests/test_function.py b/tests/scenario_tests/test_function.py index c1b7e057a..0a2152892 100644 --- a/tests/scenario_tests/test_function.py +++ b/tests/scenario_tests/test_function.py @@ -155,7 +155,7 @@ def test_function_handler_timeout(self, monkeypatch): client=self.web_client, signing_secret=self.signing_secret, ) - app.function("reverse", auto_acknowledge=False, acknowledgement_timeout=timeout)(just_no_ack) + app.function("reverse", auto_acknowledge=False, ack_timeout=timeout)(just_no_ack) request = self.build_request_from_body(function_body) sleep_mock = Mock() @@ -181,7 +181,7 @@ def test_warning_when_timeout_improperly_set(self, caplog): app.function("reverse")(just_no_ack) assert "WARNING" not in caplog.text - timeout_argument_name = "acknowledgement_timeout" + timeout_argument_name = "ack_timeout" kwargs = {timeout_argument_name: 5} callback_id = "reverse1" diff --git a/tests/scenario_tests_async/test_function.py b/tests/scenario_tests_async/test_function.py index 4af4024cf..3f8b7a722 100644 --- a/tests/scenario_tests_async/test_function.py +++ b/tests/scenario_tests_async/test_function.py @@ -166,7 +166,7 @@ async def test_function_handler_timeout(self, monkeypatch): client=self.web_client, signing_secret=self.signing_secret, ) - app.function("reverse", auto_acknowledge=False, acknowledgement_timeout=timeout)(just_no_ack) + app.function("reverse", auto_acknowledge=False, ack_timeout=timeout)(just_no_ack) request = self.build_request_from_body(function_body) sleep_mock = MagicMock(side_effect=fake_sleep) @@ -193,7 +193,7 @@ async def test_warning_when_timeout_improperly_set(self, caplog): app.function("reverse")(just_no_ack) assert "WARNING" not in caplog.text - timeout_argument_name = "acknowledgement_timeout" + timeout_argument_name = "ack_timeout" kwargs = {timeout_argument_name: 5} callback_id = "reverse1"