X Tutup
Skip to content

Commit dd855e1

Browse files
authored
Fix slackapi#31 by adding app.error handler (slackapi#43)
1 parent 059d375 commit dd855e1

File tree

14 files changed

+592
-37
lines changed

14 files changed

+592
-37
lines changed

.travis.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,4 +20,4 @@ script:
2020
# run all tests just in case
2121
- travis_retry python setup.py test
2222
# Run pytype only for Python 3.8
23-
- if [ ${TRAVIS_PYTHON_VERSION:0:3} == "3.8" ]; then pip install -e ".[adapter]"; pytype slack_bolt/; fi
23+
- if [ ${TRAVIS_PYTHON_VERSION:0:3} == "3.8" ]; then pip install -e ".[adapter]" && pytype slack_bolt/; fi

samples/app.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,12 @@ def event_test(payload, say, logger):
2626
say("What's up?")
2727

2828

29+
@app.error
30+
def global_error_handler(error, payload, logger):
31+
logger.exception(error)
32+
logger.info(payload)
33+
34+
2935
if __name__ == "__main__":
3036
app.start(3000)
3137

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
#!/bin/bash
2+
# Run all the tests or a single test
3+
# all: ./scripts/install_all_and_run_tests.sh
4+
# single: ./scripts/install_all_and_run_tests.sh tests/scenario_tests/test_app.py
5+
6+
script_dir=`dirname $0`
7+
cd ${script_dir}/..
8+
9+
test_target="$1"
10+
11+
if [[ $test_target != "" ]]
12+
then
13+
pip install -e ".[testing]" && \
14+
black slack_bolt/ tests/ && \
15+
pytest $1
16+
else
17+
pip install -e ".[testing]" && \
18+
black slack_bolt/ tests/ && \
19+
pytest && \
20+
pytype slack_bolt/
21+
fi

scripts/run_pytype.sh

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
#!/bin/bash
2+
# ./scripts/run_pytype.sh
3+
4+
script_dir=$(dirname $0)
5+
cd ${script_dir}/..
6+
pip install -e ".[adapter]" && pytype slack_bolt/

scripts/run_tests.sh

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@ test_target="$1"
1010

1111
if [[ $test_target != "" ]]
1212
then
13-
pip install -e ".[testing]" && \
14-
black slack_bolt/ tests/ && \
13+
black slack_bolt/ tests/ && \
1514
pytest $1
1615
else
17-
pip install -e ".[testing]" && \
18-
black slack_bolt/ tests/ && \
16+
black slack_bolt/ tests/ && \
1917
pytest && \
2018
pytype slack_bolt/
2119
fi

slack_bolt/app/app.py

Lines changed: 59 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@
1515
from slack_bolt.error import BoltError
1616
from slack_bolt.listener.custom_listener import CustomListener
1717
from slack_bolt.listener.listener import Listener
18+
from slack_bolt.listener.listener_error_handler import (
19+
ListenerErrorHandler,
20+
DefaultListenerErrorHandler,
21+
CustomListenerErrorHandler,
22+
)
1823
from slack_bolt.listener_matcher import CustomListenerMatcher
1924
from slack_bolt.listener_matcher import builtins as builtin_matchers
2025
from slack_bolt.listener_matcher.listener_matcher import ListenerMatcher
@@ -180,6 +185,9 @@ def __init__(
180185
self._middleware_list: List[Union[Callable, Middleware]] = []
181186
self._listeners: List[Listener] = []
182187
self._listener_executor = ThreadPoolExecutor(max_workers=5) # TODO: shutdown
188+
self._listener_error_handler = DefaultListenerErrorHandler(
189+
logger=self._framework_logger
190+
)
183191
self._process_before_response = process_before_response
184192

185193
self._init_middleware_list_done = False
@@ -238,6 +246,10 @@ def installation_store(self) -> Optional[InstallationStore]:
238246
def oauth_state_store(self) -> Optional[OAuthStateStore]:
239247
return self._oauth_state_store
240248

249+
@property
250+
def listener_error_handler(self) -> ListenerErrorHandler:
251+
return self._listener_error_handler
252+
241253
# -------------------------
242254
# standalone server
243255

@@ -301,13 +313,24 @@ def run_listener(
301313
ack = request.context.ack
302314
starting_time = time.time()
303315
if self._process_before_response:
304-
returned_value = listener.run_ack_function(
305-
request=request, response=response
306-
)
307-
if isinstance(returned_value, BoltResponse):
308-
response = returned_value
309-
if ack.response is None and listener.auto_acknowledgement:
310-
ack() # automatic ack() call if the call is not yet done
316+
try:
317+
returned_value = listener.run_ack_function(
318+
request=request, response=response
319+
)
320+
if isinstance(returned_value, BoltResponse):
321+
response = returned_value
322+
if ack.response is None and listener.auto_acknowledgement:
323+
ack() # automatic ack() call if the call is not yet done
324+
except Exception as e:
325+
# The default response status code is 500 in this case.
326+
# You can customize this by passing your own error handler.
327+
if response is None:
328+
response = BoltResponse(status=500)
329+
response.status = 500
330+
self._listener_error_handler.handle(
331+
error=e, request=request, response=response,
332+
)
333+
ack.response = response
311334

312335
if response is not None:
313336
self._debug_log_completion(starting_time, response)
@@ -318,13 +341,22 @@ def run_listener(
318341
else:
319342
# start the listener function asynchronously
320343
def run_ack_function_asynchronously():
344+
nonlocal ack, request, response
321345
try:
322346
listener.run_ack_function(request=request, response=response)
323347
except Exception as e:
324-
# TODO: error handler
325-
self._framework_logger.exception(
326-
f"Failed to run listener function (error: {e})"
348+
# The default response status code is 500 in this case.
349+
# You can customize this by passing your own error handler.
350+
if response is None:
351+
response = BoltResponse(status=500)
352+
response.status = 500
353+
if ack.response is not None: # already acknowledged
354+
response = None
355+
356+
self._listener_error_handler.handle(
357+
error=e, request=request, response=response,
327358
)
359+
ack.response = response
328360

329361
self._listener_executor.submit(run_ack_function_asynchronously)
330362

@@ -336,12 +368,17 @@ def run_ack_function_asynchronously():
336368
while ack.response is None and time.time() - starting_time <= 3:
337369
time.sleep(0.01)
338370

371+
if response is None and ack.response is None:
372+
self._framework_logger.warning(f"{listener_name} didn't call ack()")
373+
return None
374+
339375
if response is None and ack.response is not None:
340376
response = ack.response
341377
self._debug_log_completion(starting_time, response)
342378
return response
343-
else:
344-
self._framework_logger.warning(f"{listener_name} didn't call ack()")
379+
380+
if response is not None:
381+
return response
345382

346383
# None for both means no ack() in the listener
347384
return None
@@ -367,6 +404,16 @@ def middleware(self, *args):
367404
CustomMiddleware(app_name=self.name, func=func)
368405
)
369406

407+
# -------------------------
408+
# global error handler
409+
410+
def error(self, *args):
411+
if len(args) > 0:
412+
func = args[0]
413+
self._listener_error_handler = CustomListenerErrorHandler(
414+
logger=self._framework_logger, func=func,
415+
)
416+
370417
# -------------------------
371418
# events
372419

slack_bolt/app/async_app.py

Lines changed: 62 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,14 @@
1515
from slack_sdk.oauth.state_store.async_state_store import AsyncOAuthStateStore
1616
from slack_sdk.web.async_client import AsyncWebClient
1717

18+
from slack_bolt.context.ack.async_ack import AsyncAck
1819
from slack_bolt.error import BoltError
1920
from slack_bolt.listener.async_listener import AsyncListener, AsyncCustomListener
21+
from slack_bolt.listener.async_listener_error_handler import (
22+
AsyncDefaultListenerErrorHandler,
23+
AsyncListenerErrorHandler,
24+
AsyncCustomListenerErrorHandler,
25+
)
2026
from slack_bolt.listener_matcher import builtins as builtin_matchers
2127
from slack_bolt.listener_matcher.async_listener_matcher import (
2228
AsyncListenerMatcher,
@@ -199,6 +205,9 @@ def __init__(
199205

200206
self._async_middleware_list: List[Union[Callable, AsyncMiddleware]] = []
201207
self._async_listeners: List[AsyncListener] = []
208+
self._async_listener_error_handler = AsyncDefaultListenerErrorHandler(
209+
logger=self._framework_logger
210+
)
202211
self._process_before_response = process_before_response
203212

204213
self._init_middleware_list_done = False
@@ -256,6 +265,10 @@ def installation_store(self) -> Optional[AsyncInstallationStore]:
256265
def oauth_state_store(self) -> Optional[AsyncOAuthStateStore]:
257266
return self._async_oauth_state_store
258267

268+
@property
269+
def listener_error_handler(self) -> AsyncListenerErrorHandler:
270+
return self._async_listener_error_handler
271+
259272
# -------------------------
260273
# standalone server
261274

@@ -325,13 +338,24 @@ async def run_async_listener(
325338
ack = request.context.ack
326339
starting_time = time.time()
327340
if self._process_before_response:
328-
returned_value = await async_listener.run_ack_function(
329-
request=request, response=response
330-
)
331-
if isinstance(returned_value, BoltResponse):
332-
response = returned_value
333-
if ack.response is None and async_listener.auto_acknowledgement:
334-
await ack() # automatic ack() call if the call is not yet done
341+
try:
342+
returned_value = await async_listener.run_ack_function(
343+
request=request, response=response
344+
)
345+
if isinstance(returned_value, BoltResponse):
346+
response = returned_value
347+
if ack.response is None and async_listener.auto_acknowledgement:
348+
await ack() # automatic ack() call if the call is not yet done
349+
except Exception as e:
350+
# The default response status code is 500 in this case.
351+
# You can customize this by passing your own error handler.
352+
if response is None:
353+
response = BoltResponse(status=500)
354+
response.status = 500
355+
await self._async_listener_error_handler.handle(
356+
error=e, request=request, response=response,
357+
)
358+
ack.response = response
335359

336360
if response is not None:
337361
self._debug_log_completion(starting_time, response)
@@ -347,33 +371,46 @@ async def run_async_listener(
347371
# start the listener function asynchronously
348372
# NOTE: intentionally
349373
async def run_ack_function_asynchronously(
350-
request: AsyncBoltRequest, response: BoltResponse,
374+
ack: AsyncAck, request: AsyncBoltRequest, response: BoltResponse,
351375
):
352376
try:
353377
await async_listener.run_ack_function(
354378
request=request, response=response
355379
)
356380
except Exception as e:
357-
# TODO: error handler
358-
self._framework_logger.exception(
359-
f"Failed to run listener function (error: {e})"
381+
# The default response status code is 500 in this case.
382+
# You can customize this by passing your own error handler.
383+
if response is None:
384+
response = BoltResponse(status=500)
385+
response.status = 500
386+
if ack.response is not None: # already acknowledged
387+
response = None
388+
389+
await self._async_listener_error_handler.handle(
390+
error=e, request=request, response=response,
360391
)
392+
ack.response = response
361393

362394
_f: Future = asyncio.ensure_future(
363-
run_ack_function_asynchronously(request, response)
395+
run_ack_function_asynchronously(ack, request, response)
364396
)
365397
self._framework_logger.debug(f"Async listener: {listener_name} started..")
366398

367399
# await for the completion of ack() in the async listener execution
368400
while ack.response is None and time.time() - starting_time <= 3:
369401
await asyncio.sleep(0.01)
370402

371-
if ack.response is not None:
403+
if response is None and ack.response is None:
404+
self._framework_logger.warning(f"{listener_name} didn't call ack()")
405+
return None
406+
407+
if response is None and ack.response is not None:
372408
response = ack.response
373409
self._debug_log_completion(starting_time, response)
374410
return response
375-
else:
376-
self._framework_logger.warning(f"{listener_name} didn't call ack()")
411+
412+
if response is not None:
413+
return response
377414

378415
# None for both means no ack() in the listener
379416
return None
@@ -399,6 +436,16 @@ def middleware(self, *args):
399436
AsyncCustomMiddleware(app_name=self.name, func=func)
400437
)
401438

439+
# -------------------------
440+
# global error handler
441+
442+
def error(self, *args):
443+
if len(args) > 0:
444+
func = args[0]
445+
self._async_listener_error_handler = AsyncCustomListenerErrorHandler(
446+
logger=self._framework_logger, func=func,
447+
)
448+
402449
# -------------------------
403450
# events
404451

slack_bolt/context/async_context.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,18 @@
11
from typing import Optional
22

3+
from slack_sdk.web.async_client import AsyncWebClient
4+
35
from slack_bolt.context.ack.async_ack import AsyncAck
46
from slack_bolt.context.base_context import BaseContext
57
from slack_bolt.context.respond.async_respond import AsyncRespond
68
from slack_bolt.context.say.async_say import AsyncSay
79

810

911
class AsyncBoltContext(BaseContext):
12+
@property
13+
def client(self) -> Optional[AsyncWebClient]:
14+
return self.get("client", None)
15+
1016
@property
1117
def ack(self) -> AsyncAck:
1218
if "ack" not in self:

slack_bolt/context/base_context.py

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
from typing import Optional, Tuple
33

44
from slack_bolt.auth import AuthorizationResult
5-
from slack_sdk import WebClient
65

76

87
class BaseContext(dict):
@@ -18,10 +17,6 @@ def logger(self) -> Logger:
1817
def token(self) -> Optional[str]:
1918
return self.get("token", None)
2019

21-
@property
22-
def client(self) -> Optional[WebClient]:
23-
return self.get("client", None)
24-
2520
@property
2621
def enterprise_id(self) -> Optional[str]:
2722
return self.get("enterprise_id", None)

slack_bolt/context/context.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,19 @@
11
# pytype: skip-file
22
from typing import Optional
33

4+
from slack_sdk import WebClient
5+
46
from slack_bolt.context.ack import Ack
57
from slack_bolt.context.base_context import BaseContext
68
from slack_bolt.context.respond import Respond
79
from slack_bolt.context.say import Say
810

911

1012
class BoltContext(BaseContext):
13+
@property
14+
def client(self) -> Optional[WebClient]:
15+
return self.get("client", None)
16+
1117
@property
1218
def ack(self) -> Ack:
1319
if "ack" not in self:

0 commit comments

Comments
 (0)
X Tutup