Skip to content

Commit a81bade

Browse files
feat: surface acknowledgement_timeout on the handler
1 parent 974816e commit a81bade

File tree

5 files changed

+81
-6
lines changed

5 files changed

+81
-6
lines changed

slack_bolt/app/app.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@
5959
info_default_oauth_settings_loaded,
6060
error_installation_store_required_for_builtin_listeners,
6161
warning_unhandled_by_global_middleware,
62+
warning_ack_timeout_has_no_effect,
6263
)
6364
from slack_bolt.middleware import (
6465
Middleware,
@@ -912,6 +913,7 @@ def function(
912913
matchers: Optional[Sequence[Callable[..., bool]]] = None,
913914
middleware: Optional[Sequence[Union[Callable, Middleware]]] = None,
914915
auto_acknowledge: bool = True,
916+
acknowledgement_timeout: int = 3,
915917
) -> Callable[..., Optional[Callable[..., Optional[BoltResponse]]]]:
916918
"""Registers a new Function listener.
917919
This method can be used as either a decorator or a method.
@@ -940,14 +942,18 @@ def reverse_string(ack: Ack, inputs: dict, complete: Complete, fail: Fail):
940942
Only when all the middleware call `next()` method, the listener function can be invoked.
941943
"""
942944

945+
if auto_acknowledge is True:
946+
if acknowledgement_timeout != 3:
947+
self._framework_logger.warning(warning_ack_timeout_has_no_effect(callback_id, acknowledgement_timeout))
948+
943949
matchers = list(matchers) if matchers else []
944950
middleware = list(middleware) if middleware else []
945951

946952
def __call__(*args, **kwargs):
947953
functions = self._to_listener_functions(kwargs) if kwargs else list(args)
948954
primary_matcher = builtin_matchers.function_executed(callback_id=callback_id, base_logger=self._base_logger)
949955
return self._register_listener(
950-
functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout=5
956+
functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout
951957
)
952958

953959
return __call__

slack_bolt/app/async_app.py

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
info_default_oauth_settings_loaded,
6969
error_installation_store_required_for_builtin_listeners,
7070
warning_unhandled_by_global_middleware,
71+
warning_ack_timeout_has_no_effect,
7172
)
7273
from slack_bolt.lazy_listener.asyncio_runner import AsyncioLazyListenerRunner
7374
from slack_bolt.listener.async_listener import AsyncListener, AsyncCustomListener
@@ -940,6 +941,7 @@ def function(
940941
matchers: Optional[Sequence[Callable[..., Awaitable[bool]]]] = None,
941942
middleware: Optional[Sequence[Union[Callable, AsyncMiddleware]]] = None,
942943
auto_acknowledge: bool = True,
944+
acknowledgement_timeout: int = 3,
943945
) -> Callable[..., Optional[Callable[..., Awaitable[BoltResponse]]]]:
944946
"""Registers a new Function listener.
945947
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
967969
middleware: A list of lister middleware functions.
968970
Only when all the middleware call `next()` method, the listener function can be invoked.
969971
"""
972+
if auto_acknowledge is True:
973+
if acknowledgement_timeout != 3:
974+
self._framework_logger.warning(warning_ack_timeout_has_no_effect(callback_id, acknowledgement_timeout))
970975

971976
matchers = list(matchers) if matchers else []
972977
middleware = list(middleware) if middleware else []
@@ -977,7 +982,7 @@ def __call__(*args, **kwargs):
977982
callback_id=callback_id, base_logger=self._base_logger, asyncio=True
978983
)
979984
return self._register_listener(
980-
functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout=5
985+
functions, primary_matcher, matchers, middleware, auto_acknowledge, acknowledgement_timeout
981986
)
982987

983988
return __call__

slack_bolt/logger/messages.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
from re import Pattern
12
import time
23
from typing import Union, Dict, Any, Optional
34

@@ -331,6 +332,14 @@ def warning_skip_uncommon_arg_name(arg_name: str) -> str:
331332
)
332333

333334

335+
def warning_ack_timeout_has_no_effect(identifier: Union[str, Pattern], acknowledgement_timeout: int) -> str:
336+
handler_example = f'@app.function("{identifier}")' if isinstance(identifier, str) else f"@app.function({identifier})"
337+
return (
338+
f"On {handler_example}, as `auto_acknowledge` is `True`, "
339+
f"`acknowledgement_timeout={acknowledgement_timeout}` you gave will be unused"
340+
)
341+
342+
334343
# -------------------------------
335344
# Info
336345
# -------------------------------

tests/scenario_tests/test_function.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import json
2+
import re
23
import time
34
import pytest
45
from unittest.mock import Mock
@@ -149,11 +150,12 @@ def test_auto_acknowledge_false_without_acknowledging(self, caplog, monkeypatch)
149150
assert f"WARNING {just_no_ack.__name__} didn't call ack()" in caplog.text
150151

151152
def test_function_handler_timeout(self, monkeypatch):
153+
timeout = 5
152154
app = App(
153155
client=self.web_client,
154156
signing_secret=self.signing_secret,
155157
)
156-
app.function("reverse", auto_acknowledge=False)(just_no_ack)
158+
app.function("reverse", auto_acknowledge=False, acknowledgement_timeout=timeout)(just_no_ack)
157159
request = self.build_request_from_body(function_body)
158160

159161
sleep_mock = Mock()
@@ -168,9 +170,34 @@ def test_function_handler_timeout(self, monkeypatch):
168170
assert response.status == 404
169171
assert_auth_test_count(self, 1)
170172
assert (
171-
sleep_mock.call_count == 5
173+
sleep_mock.call_count == timeout
172174
), f"Expected handler to time out after calling time.sleep 5 times, but it was called {sleep_mock.call_count} times"
173175

176+
def test_warning_when_timeout_improperly_set(self, caplog):
177+
app = App(
178+
client=self.web_client,
179+
signing_secret=self.signing_secret,
180+
)
181+
app.function("reverse")(just_no_ack)
182+
assert "WARNING" not in caplog.text
183+
184+
timeout_argument_name = "acknowledgement_timeout"
185+
kwargs = {timeout_argument_name: 5}
186+
187+
callback_id = "reverse1"
188+
app.function(callback_id, **kwargs)(just_no_ack)
189+
assert (
190+
f'WARNING On @app.function("{callback_id}"), as `auto_acknowledge` is `True`, `{timeout_argument_name}={kwargs[timeout_argument_name]}` you gave will be unused'
191+
in caplog.text
192+
)
193+
194+
callback_id = re.compile(r"hello \w+")
195+
app.function(callback_id, **kwargs)(just_no_ack)
196+
assert (
197+
f"WARNING On @app.function({callback_id}), as `auto_acknowledge` is `True`, `{timeout_argument_name}={kwargs[timeout_argument_name]}` you gave will be unused"
198+
in caplog.text
199+
)
200+
174201

175202
function_body = {
176203
"token": "verification_token",

tests/scenario_tests_async/test_function.py

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import asyncio
22
import json
3+
import re
34
import time
45

56
import pytest
@@ -160,11 +161,12 @@ async def test_auto_acknowledge_false_without_acknowledging(self, caplog, monkey
160161

161162
@pytest.mark.asyncio
162163
async def test_function_handler_timeout(self, monkeypatch):
164+
timeout = 5
163165
app = AsyncApp(
164166
client=self.web_client,
165167
signing_secret=self.signing_secret,
166168
)
167-
app.function("reverse", auto_acknowledge=False)(just_no_ack)
169+
app.function("reverse", auto_acknowledge=False, acknowledgement_timeout=timeout)(just_no_ack)
168170
request = self.build_request_from_body(function_body)
169171

170172
sleep_mock = MagicMock(side_effect=fake_sleep)
@@ -179,9 +181,35 @@ async def test_function_handler_timeout(self, monkeypatch):
179181
assert response.status == 404
180182
await assert_auth_test_count_async(self, 1)
181183
assert (
182-
sleep_mock.call_count == 5
184+
sleep_mock.call_count == timeout
183185
), f"Expected handler to time out after calling time.sleep 5 times, but it was called {sleep_mock.call_count} times"
184186

187+
@pytest.mark.asyncio
188+
async def test_warning_when_timeout_improperly_set(self, caplog):
189+
app = AsyncApp(
190+
client=self.web_client,
191+
signing_secret=self.signing_secret,
192+
)
193+
app.function("reverse")(just_no_ack)
194+
assert "WARNING" not in caplog.text
195+
196+
timeout_argument_name = "acknowledgement_timeout"
197+
kwargs = {timeout_argument_name: 5}
198+
199+
callback_id = "reverse1"
200+
app.function(callback_id, **kwargs)(just_no_ack)
201+
assert (
202+
f'WARNING On @app.function("{callback_id}"), as `auto_acknowledge` is `True`, `{timeout_argument_name}={kwargs[timeout_argument_name]}` you gave will be unused'
203+
in caplog.text
204+
)
205+
206+
callback_id = re.compile(r"hello \w+")
207+
app.function(callback_id, **kwargs)(just_no_ack)
208+
assert (
209+
f"WARNING On @app.function({callback_id}), as `auto_acknowledge` is `True`, `{timeout_argument_name}={kwargs[timeout_argument_name]}` you gave will be unused"
210+
in caplog.text
211+
)
212+
185213

186214
function_body = {
187215
"token": "verification_token",

0 commit comments

Comments
 (0)