Fix slackapi#709 Enable developers to inject non-bolt arguments to li…

…stener function args
seratch · Aug 25, 2022 · 24ff0da · 24ff0da
1 parent aee0fe0
commit 24ff0da
Show file tree

Hide file tree

Showing 7 changed files with 47 additions and 14 deletions.
diff --git a/slack_bolt/app/app.py b/slack_bolt/app/app.py
@@ -3,6 +3,7 @@
 import logging
 import os
 import time
+import warnings
 from concurrent.futures import Executor
 from concurrent.futures.thread import ThreadPoolExecutor
 from http.server import SimpleHTTPRequestHandler, HTTPServer
@@ -79,6 +80,7 @@
     get_boot_message,
     get_name_for_callable,
 )
+from slack_bolt.warning import BoltCodeWarning
 from slack_bolt.workflows.step import WorkflowStep, WorkflowStepMiddleware
 from slack_bolt.workflows.step.step import WorkflowStepBuilder
 
@@ -491,7 +493,9 @@ def middleware_next():
                                 response=resp,
                             )
                             return resp
-                        self._framework_logger.warning(warning_unhandled_by_global_middleware(middleware.name, req))
+                        # In most cases, this could be a coding error, but it can be intentional for some reason
+                        # So we use BoltCodeWarning for it
+                        warnings.warn(warning_unhandled_by_global_middleware(middleware.name, req), BoltCodeWarning)
                         return resp
                     return resp
 
@@ -552,7 +556,8 @@ def middleware_next():
             return resp
 
     def _handle_unmatched_requests(self, req: BoltRequest, resp: BoltResponse) -> BoltResponse:
-        self._framework_logger.warning(warning_unhandled_request(req))
+        # This warning is helpful in coding phase, but for production operations
+        warnings.warn(warning_unhandled_request(req), BoltCodeWarning)
         return resp
 
     # -------------------------

diff --git a/slack_bolt/app/async_app.py b/slack_bolt/app/async_app.py
@@ -2,6 +2,7 @@
 import logging
 import os
 import time
+import warnings
 from typing import Optional, List, Union, Callable, Pattern, Dict, Awaitable, Sequence
 
 from aiohttp import web
@@ -24,6 +25,7 @@
 )
 from slack_bolt.oauth.async_internals import select_consistent_installation_store
 from slack_bolt.util.utils import get_name_for_callable
+from slack_bolt.warning import BoltCodeWarning
 from slack_bolt.workflows.step.async_step import (
     AsyncWorkflowStep,
     AsyncWorkflowStepBuilder,
@@ -517,7 +519,9 @@ async def async_middleware_next():
                                 response=resp,
                             )
                             return resp
-                        self._framework_logger.warning(warning_unhandled_by_global_middleware(middleware.name, req))
+                        # In most cases, this could be a coding error, but it can be intentional for some reason
+                        # So we use BoltCodeWarning for it
+                        warnings.warn(warning_unhandled_by_global_middleware(middleware.name, req), BoltCodeWarning)
                         return resp
                     return resp
 
@@ -582,7 +586,8 @@ async def async_middleware_next():
             return resp
 
     def _handle_unmatched_requests(self, req: AsyncBoltRequest, resp: BoltResponse) -> BoltResponse:
-        self._framework_logger.warning(warning_unhandled_request(req))
+        # This warning is helpful in coding phase, but for production operations
+        warnings.warn(warning_unhandled_request(req), BoltCodeWarning)
         return resp
 
     # -------------------------

diff --git a/slack_bolt/authorization/async_authorize.py b/slack_bolt/authorization/async_authorize.py
@@ -1,3 +1,4 @@
+import warnings
 from logging import Logger
 from typing import Optional, Callable, Awaitable, Dict, Any
 
@@ -14,6 +15,7 @@
 from slack_bolt.context.async_context import AsyncBoltContext
 from slack_bolt.error import BoltError
 from slack_bolt.util.utils import get_arg_names_of_callable
+from slack_bolt.warning import BoltCodeWarning
 
 
 class AsyncAuthorize:
@@ -77,8 +79,9 @@ async def __call__(
             found_arg_names = kwargs.keys()
             for name in self.arg_names:
                 if name not in found_arg_names:
-                    self.logger.warning(f"{name} is not a valid argument")
-                    kwargs[name] = None
+                    warnings.warn(
+                        f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning
+                    )
 
             auth_result: Optional[AuthorizeResult] = await self.func(**kwargs)
             if auth_result is None:

diff --git a/slack_bolt/authorization/authorize.py b/slack_bolt/authorization/authorize.py
@@ -1,3 +1,4 @@
+import warnings
 from logging import Logger
 from typing import Optional, Callable, Dict, Any
 
@@ -13,6 +14,7 @@
 from slack_bolt.context.context import BoltContext
 from slack_bolt.error import BoltError
 from slack_bolt.util.utils import get_arg_names_of_callable
+from slack_bolt.warning import BoltCodeWarning
 
 
 class Authorize:
@@ -81,8 +83,9 @@ def __call__(
             found_arg_names = kwargs.keys()
             for name in self.arg_names:
                 if name not in found_arg_names:
-                    self.logger.warning(f"{name} is not a valid argument")
-                    kwargs[name] = None
+                    warnings.warn(
+                        f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning
+                    )
 
             auth_result = self.func(**kwargs)
             if auth_result is None:

diff --git a/slack_bolt/kwargs_injection/async_utils.py b/slack_bolt/kwargs_injection/async_utils.py
@@ -1,6 +1,7 @@
 # pytype: skip-file
 import inspect
 import logging
+import warnings
 from typing import Callable, Dict, Optional, Any, Sequence
 
 from slack_bolt.request.async_request import AsyncBoltRequest
@@ -17,6 +18,7 @@
     to_step,
 )
 from ..logger.messages import warning_skip_uncommon_arg_name
+from ..warning import BoltCodeWarning
 
 
 def build_async_required_kwargs(
@@ -85,7 +87,8 @@ def build_async_required_kwargs(
             required_arg_names.pop(0)
         elif first_arg_name not in all_available_args.keys():
             if this_func is None:
-                logger.warning(warning_skip_uncommon_arg_name(first_arg_name))
+                # Actually, it's rare to see this warning
+                warnings.warn(warning_skip_uncommon_arg_name(first_arg_name), BoltCodeWarning)
                 required_arg_names.pop(0)
             elif inspect.ismethod(this_func):
                 # We are sure that we should skip manipulating this arg
@@ -98,9 +101,9 @@ def build_async_required_kwargs(
             if isinstance(request, AsyncBoltRequest):
                 kwargs[name] = AsyncArgs(**all_available_args)
             else:
+                # We don't use BolCodeWarning here because something unexpected (e.g., bolt-python bug) may be the cause
                 logger.warning(f"Unknown Request object type detected ({type(request)})")
 
         if name not in found_arg_names:
-            logger.warning(f"{name} is not a valid argument")
-            kwargs[name] = None
+            warnings.warn(f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning)
     return kwargs
diff --git a/slack_bolt/kwargs_injection/utils.py b/slack_bolt/kwargs_injection/utils.py
@@ -1,6 +1,7 @@
 # pytype: skip-file
 import inspect
 import logging
+import warnings
 from typing import Callable, Dict, Optional, Any, Sequence
 
 from slack_bolt.request import BoltRequest
@@ -17,6 +18,7 @@
     to_step,
 )
 from ..logger.messages import warning_skip_uncommon_arg_name
+from ..warning import BoltCodeWarning
 
 
 def build_required_kwargs(
@@ -85,7 +87,8 @@ def build_required_kwargs(
             required_arg_names.pop(0)
         elif first_arg_name not in all_available_args.keys():
             if this_func is None:
-                logger.warning(warning_skip_uncommon_arg_name(first_arg_name))
+                # Actually, it's rare to see this warning
+                warnings.warn(warning_skip_uncommon_arg_name(first_arg_name), BoltCodeWarning)
                 required_arg_names.pop(0)
             elif inspect.ismethod(this_func):
                 # We are sure that we should skip manipulating this arg
@@ -98,9 +101,9 @@ def build_required_kwargs(
             if isinstance(request, BoltRequest):
                 kwargs[name] = Args(**all_available_args)
             else:
+                # We don't use BolCodeWarning here because something unexpected (e.g., bolt-python bug) may be the cause
                 logger.warning(f"Unknown Request object type detected ({type(request)})")
 
         if name not in found_arg_names:
-            logger.warning(f"{name} is not a valid argument")
-            kwargs[name] = None
+            warnings.warn(f"{name} may not be a valid argument name, which bolt-python cannot handle", BoltCodeWarning)
     return kwargs
diff --git a/slack_bolt/warning/__init__.py b/slack_bolt/warning/__init__.py
@@ -0,0 +1,11 @@
+"""Warnings by Bolt for Python
+
+Developers can ignore the following warning categories as necessary.
+see also: https://docs.python.org/3/library/warnings.html#default-warning-filter
+"""
+
+
+class BoltCodeWarning(UserWarning):
+    """Warning to help developers notice their coding errors.
+    This warning should not be used for informing configuration errors in the App/AsyncApp constructor.
+    """