Skip to content

Commit fa4988f

Browse files
seratchseratch
authored
Fix #378 by adding middleware error handlers (#401)
* Fix #378 by adding middleware error handlers * Delete the internals that are no longer used
1 parent 149c354 commit fa4988f

15 files changed

+412
-317
lines changed

‎slack_bolt/app/app.py‎

Lines changed: 90 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,10 @@
6262
CustomMiddleware,
6363
)
6464
from slack_bolt.middleware.message_listener_matches import MessageListenerMatches
65+
from slack_bolt.middleware.middleware_error_handler import (
66+
DefaultMiddlewareErrorHandler,
67+
CustomMiddlewareErrorHandler,
68+
)
6569
from slack_bolt.middleware.url_verification import UrlVerification
6670
from slack_bolt.oauth import OAuthFlow
6771
from slack_bolt.oauth.internals import select_consistent_installation_store
@@ -309,6 +313,9 @@ def message_hello(message, say):
309313
executor=listener_executor,
310314
),
311315
)
316+
self._middleware_error_handler = DefaultMiddlewareErrorHandler(
317+
logger=self._framework_logger,
318+
)
312319

313320
self._init_middleware_list_done = False
314321
self._init_middleware_list(
@@ -448,84 +455,99 @@ def dispatch(self, req: BoltRequest) -> BoltResponse:
448455
def middleware_next():
449456
middleware_state["next_called"] = True
450457

451-
for middleware in self._middleware_list:
452-
middleware_state["next_called"] = False
453-
if self._framework_logger.level <= logging.DEBUG:
454-
self._framework_logger.debug(debug_applying_middleware(middleware.name))
455-
resp = middleware.process(req=req, resp=resp, next=middleware_next)
456-
if not middleware_state["next_called"]:
457-
if resp is None:
458-
# next() method was not called without providing the response to return to Slack
459-
# This should not be an intentional handling in usual use cases.
460-
resp = BoltResponse(
461-
status=404, body={"error": "no next() calls in middleware"}
458+
try:
459+
for middleware in self._middleware_list:
460+
middleware_state["next_called"] = False
461+
if self._framework_logger.level <= logging.DEBUG:
462+
self._framework_logger.debug(
463+
debug_applying_middleware(middleware.name)
462464
)
463-
if self._raise_error_for_unhandled_request is True:
464-
self._listener_runner.listener_error_handler.handle(
465-
error=BoltUnhandledRequestError(
465+
resp = middleware.process(req=req, resp=resp, next=middleware_next)
466+
if not middleware_state["next_called"]:
467+
if resp is None:
468+
# next() method was not called without providing the response to return to Slack
469+
# This should not be an intentional handling in usual use cases.
470+
resp = BoltResponse(
471+
status=404, body={"error": "no next() calls in middleware"}
472+
)
473+
if self._raise_error_for_unhandled_request is True:
474+
self._listener_runner.listener_error_handler.handle(
475+
error=BoltUnhandledRequestError(
476+
request=req,
477+
current_response=resp,
478+
last_global_middleware_name=middleware.name,
479+
),
466480
request=req,
467-
current_response=resp,
468-
last_global_middleware_name=middleware.name,
469-
),
470-
request=req,
471-
response=resp,
481+
response=resp,
482+
)
483+
return resp
484+
self._framework_logger.warning(
485+
warning_unhandled_by_global_middleware(middleware.name, req)
472486
)
473487
return resp
474-
self._framework_logger.warning(
475-
warning_unhandled_by_global_middleware(middleware.name, req)
476-
)
477488
return resp
478-
return resp
479-
480-
for listener in self._listeners:
481-
listener_name = get_name_for_callable(listener.ack_function)
482-
self._framework_logger.debug(debug_checking_listener(listener_name))
483-
if listener.matches(req=req, resp=resp):
484-
# run all the middleware attached to this listener first
485-
middleware_resp, next_was_not_called = listener.run_middleware(
486-
req=req, resp=resp
487-
)
488-
if next_was_not_called:
489-
if middleware_resp is not None:
490-
if self._framework_logger.level <= logging.DEBUG:
491-
debug_message = debug_return_listener_middleware_response(
492-
listener_name,
493-
middleware_resp.status,
494-
middleware_resp.body,
495-
starting_time,
496-
)
497-
self._framework_logger.debug(debug_message)
498-
return middleware_resp
499-
# The last listener middleware didn't call next() method.
500-
# This means the listener is not for this incoming request.
501-
continue
502489

503-
if middleware_resp is not None:
504-
resp = middleware_resp
490+
for listener in self._listeners:
491+
listener_name = get_name_for_callable(listener.ack_function)
492+
self._framework_logger.debug(debug_checking_listener(listener_name))
493+
if listener.matches(req=req, resp=resp):
494+
# run all the middleware attached to this listener first
495+
middleware_resp, next_was_not_called = listener.run_middleware(
496+
req=req, resp=resp
497+
)
498+
if next_was_not_called:
499+
if middleware_resp is not None:
500+
if self._framework_logger.level <= logging.DEBUG:
501+
debug_message = (
502+
debug_return_listener_middleware_response(
503+
listener_name,
504+
middleware_resp.status,
505+
middleware_resp.body,
506+
starting_time,
507+
)
508+
)
509+
self._framework_logger.debug(debug_message)
510+
return middleware_resp
511+
# The last listener middleware didn't call next() method.
512+
# This means the listener is not for this incoming request.
513+
continue
505514

506-
self._framework_logger.debug(debug_running_listener(listener_name))
507-
listener_response: Optional[BoltResponse] = self._listener_runner.run(
515+
if middleware_resp is not None:
516+
resp = middleware_resp
517+
518+
self._framework_logger.debug(debug_running_listener(listener_name))
519+
listener_response: Optional[
520+
BoltResponse
521+
] = self._listener_runner.run(
522+
request=req,
523+
response=resp,
524+
listener_name=listener_name,
525+
listener=listener,
526+
)
527+
if listener_response is not None:
528+
return listener_response
529+
530+
if resp is None:
531+
resp = BoltResponse(status=404, body={"error": "unhandled request"})
532+
if self._raise_error_for_unhandled_request is True:
533+
self._listener_runner.listener_error_handler.handle(
534+
error=BoltUnhandledRequestError(
535+
request=req,
536+
current_response=resp,
537+
),
508538
request=req,
509539
response=resp,
510-
listener_name=listener_name,
511-
listener=listener,
512540
)
513-
if listener_response is not None:
514-
return listener_response
515-
516-
if resp is None:
517-
resp = BoltResponse(status=404, body={"error": "unhandled request"})
518-
if self._raise_error_for_unhandled_request is True:
519-
self._listener_runner.listener_error_handler.handle(
520-
error=BoltUnhandledRequestError(
521-
request=req,
522-
current_response=resp,
523-
),
541+
return resp
542+
return self._handle_unmatched_requests(req, resp)
543+
except Exception as error:
544+
resp = BoltResponse(status=500, body="")
545+
self._middleware_error_handler.handle(
546+
error=error,
524547
request=req,
525548
response=resp,
526549
)
527550
return resp
528-
return self._handle_unmatched_requests(req, resp)
529551

530552
def _handle_unmatched_requests(
531553
self, req: BoltRequest, resp: BoltResponse
@@ -664,6 +686,10 @@ def custom_error_handler(error, body, logger):
664686
logger=self._framework_logger,
665687
func=func,
666688
)
689+
self._middleware_error_handler = CustomMiddlewareErrorHandler(
690+
logger=self._framework_logger,
691+
func=func,
692+
)
667693
return func
668694

669695
# -------------------------

‎slack_bolt/app/async_app.py‎

Lines changed: 93 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,10 @@
1212
AsyncDefaultListenerCompletionHandler,
1313
)
1414
from slack_bolt.listener.asyncio_runner import AsyncioListenerRunner
15+
from slack_bolt.middleware.async_middleware_error_handler import (
16+
AsyncCustomMiddlewareErrorHandler,
17+
AsyncDefaultMiddlewareErrorHandler,
18+
)
1519
from slack_bolt.middleware.message_listener_matches.async_message_listener_matches import (
1620
AsyncMessageListenerMatches,
1721
)
@@ -334,6 +338,9 @@ async def message_hello(message, say): # async function
334338
logger=self._framework_logger,
335339
),
336340
)
341+
self._async_middleware_error_handler = AsyncDefaultMiddlewareErrorHandler(
342+
logger=self._framework_logger,
343+
)
337344

338345
self._init_middleware_list_done = False
339346
self._init_async_middleware_list(
@@ -499,89 +506,101 @@ async def async_dispatch(self, req: AsyncBoltRequest) -> BoltResponse:
499506
async def async_middleware_next():
500507
middleware_state["next_called"] = True
501508

502-
for middleware in self._async_middleware_list:
503-
middleware_state["next_called"] = False
504-
if self._framework_logger.level <= logging.DEBUG:
505-
self._framework_logger.debug(f"Applying {middleware.name}")
506-
resp = await middleware.async_process(
507-
req=req, resp=resp, next=async_middleware_next
508-
)
509-
if not middleware_state["next_called"]:
510-
if resp is None:
511-
# next() method was not called without providing the response to return to Slack
512-
# This should not be an intentional handling in usual use cases.
513-
resp = BoltResponse(
514-
status=404, body={"error": "no next() calls in middleware"}
515-
)
516-
if self._raise_error_for_unhandled_request is True:
517-
await self._async_listener_runner.listener_error_handler.handle(
518-
error=BoltUnhandledRequestError(
509+
try:
510+
for middleware in self._async_middleware_list:
511+
middleware_state["next_called"] = False
512+
if self._framework_logger.level <= logging.DEBUG:
513+
self._framework_logger.debug(f"Applying {middleware.name}")
514+
resp = await middleware.async_process(
515+
req=req, resp=resp, next=async_middleware_next
516+
)
517+
if not middleware_state["next_called"]:
518+
if resp is None:
519+
# next() method was not called without providing the response to return to Slack
520+
# This should not be an intentional handling in usual use cases.
521+
resp = BoltResponse(
522+
status=404, body={"error": "no next() calls in middleware"}
523+
)
524+
if self._raise_error_for_unhandled_request is True:
525+
await self._async_listener_runner.listener_error_handler.handle(
526+
error=BoltUnhandledRequestError(
527+
request=req,
528+
current_response=resp,
529+
last_global_middleware_name=middleware.name,
530+
),
519531
request=req,
520-
current_response=resp,
521-
last_global_middleware_name=middleware.name,
522-
),
523-
request=req,
524-
response=resp,
532+
response=resp,
533+
)
534+
return resp
535+
self._framework_logger.warning(
536+
warning_unhandled_by_global_middleware(middleware.name, req)
525537
)
526538
return resp
527-
self._framework_logger.warning(
528-
warning_unhandled_by_global_middleware(middleware.name, req)
529-
)
530539
return resp
531-
return resp
532540

533-
for listener in self._async_listeners:
534-
listener_name = get_name_for_callable(listener.ack_function)
535-
self._framework_logger.debug(debug_checking_listener(listener_name))
536-
if await listener.async_matches(req=req, resp=resp):
537-
# run all the middleware attached to this listener first
538-
(
539-
middleware_resp,
540-
next_was_not_called,
541-
) = await listener.run_async_middleware(req=req, resp=resp)
542-
if next_was_not_called:
541+
for listener in self._async_listeners:
542+
listener_name = get_name_for_callable(listener.ack_function)
543+
self._framework_logger.debug(debug_checking_listener(listener_name))
544+
if await listener.async_matches(req=req, resp=resp):
545+
# run all the middleware attached to this listener first
546+
(
547+
middleware_resp,
548+
next_was_not_called,
549+
) = await listener.run_async_middleware(req=req, resp=resp)
550+
if next_was_not_called:
551+
if middleware_resp is not None:
552+
if self._framework_logger.level <= logging.DEBUG:
553+
debug_message = (
554+
debug_return_listener_middleware_response(
555+
listener_name,
556+
middleware_resp.status,
557+
middleware_resp.body,
558+
starting_time,
559+
)
560+
)
561+
self._framework_logger.debug(debug_message)
562+
return middleware_resp
563+
# The last listener middleware didn't call next() method.
564+
# This means the listener is not for this incoming request.
565+
continue
566+
543567
if middleware_resp is not None:
544-
if self._framework_logger.level <= logging.DEBUG:
545-
debug_message = debug_return_listener_middleware_response(
546-
listener_name,
547-
middleware_resp.status,
548-
middleware_resp.body,
549-
starting_time,
550-
)
551-
self._framework_logger.debug(debug_message)
552-
return middleware_resp
553-
# The last listener middleware didn't call next() method.
554-
# This means the listener is not for this incoming request.
555-
continue
556-
557-
if middleware_resp is not None:
558-
resp = middleware_resp
559-
560-
self._framework_logger.debug(debug_running_listener(listener_name))
561-
listener_response: Optional[
562-
BoltResponse
563-
] = await self._async_listener_runner.run(
568+
resp = middleware_resp
569+
570+
self._framework_logger.debug(debug_running_listener(listener_name))
571+
listener_response: Optional[
572+
BoltResponse
573+
] = await self._async_listener_runner.run(
574+
request=req,
575+
response=resp,
576+
listener_name=listener_name,
577+
listener=listener,
578+
)
579+
if listener_response is not None:
580+
return listener_response
581+
582+
if resp is None:
583+
resp = BoltResponse(status=404, body={"error": "unhandled request"})
584+
if self._raise_error_for_unhandled_request is True:
585+
await self._async_listener_runner.listener_error_handler.handle(
586+
error=BoltUnhandledRequestError(
587+
request=req,
588+
current_response=resp,
589+
),
564590
request=req,
565591
response=resp,
566-
listener_name=listener_name,
567-
listener=listener,
568592
)
569-
if listener_response is not None:
570-
return listener_response
571-
572-
if resp is None:
573-
resp = BoltResponse(status=404, body={"error": "unhandled request"})
574-
if self._raise_error_for_unhandled_request is True:
575-
await self._async_listener_runner.listener_error_handler.handle(
576-
error=BoltUnhandledRequestError(
577-
request=req,
578-
current_response=resp,
579-
),
593+
return resp
594+
return self._handle_unmatched_requests(req, resp)
595+
596+
except Exception as error:
597+
resp = BoltResponse(status=500, body="")
598+
await self._async_middleware_error_handler.handle(
599+
error=error,
580600
request=req,
581601
response=resp,
582602
)
583603
return resp
584-
return self._handle_unmatched_requests(req, resp)
585604

586605
def _handle_unmatched_requests(
587606
self, req: AsyncBoltRequest, resp: BoltResponse
@@ -729,6 +748,10 @@ async def custom_error_handler(error, body, logger):
729748
func=func,
730749
)
731750
)
751+
self._async_middleware_error_handler = AsyncCustomMiddlewareErrorHandler(
752+
logger=self._framework_logger,
753+
func=func,
754+
)
732755
return func
733756

734757
# -------------------------

0 commit comments

Comments
 (0)