Repository: sanic-org/sanic
Pull request: Noisy exceptions - force logging of all exceptions
Resolves #2241.

The purpose of this PR is to allow all exceptions to be logged, regardless of them being "quiet" or not.

Problem statement:
Noisy exceptions
```
$ sanic ... --no-quiet
```

This (plus a corresponding config) might be nice to have when debugging to ignore `quiet=True` on exceptions. I do not think it should be included in `--debug`. 

The alternative might be to have different layers of verbosity in logging. And `-vv` would log out whenever an exception is swallowed.

Hints from issue discussion:
Suggesting `--noisy-exceptions` or something else that (a) uses no negation and (b) is more specific.

I was thinking about making it such that we could have different levels of verbosity across the whole app by using different amounts of `v`s (`-vvv` > `-vv` > `-v`), but `-v` is already in use to check the module version, so I think I'll go for `-ne`/`--noisy-exceptions`. 
Verbosity levels is something I want to add.

Commit message: Added tests

Patch to review:
diff --git a/sanic/__main__.py b/sanic/__main__.py
index 027bf8793d..2ebb9de0dc 100644
--- a/sanic/__main__.py
+++ b/sanic/__main__.py
@@ -96,6 +96,11 @@ def main():
         help="number of worker processes [default 1]\n ",
     )
     parser.add_argument("-d", "--debug", dest="debug", action="store_true")
+    parser.add_argument(
+        "--noisy-exceptions",
+        dest="noisy_exceptions",
+        action="store_true",
+    )
     parser.add_argument(
         "-r",
         "--reload",
@@ -149,6 +154,7 @@ def main():
                     f"Module is not a Sanic app, it is a {app_type_name}.  "
                     f"Perhaps you meant {args.module}.app?"
                 )
+
         if args.cert is not None or args.key is not None:
             ssl: Optional[Dict[str, Any]] = {
                 "cert": args.cert,
@@ -165,6 +171,7 @@ def main():
             "debug": args.debug,
             "access_log": args.access_log,
             "ssl": ssl,
+            "noisy_exceptions": args.noisy_exceptions,
         }
         if args.auto_reload:
             kwargs["auto_reload"] = True
diff --git a/sanic/app.py b/sanic/app.py
index 0686f7ed7c..e5796b3870 100644
--- a/sanic/app.py
+++ b/sanic/app.py
@@ -890,7 +890,8 @@ async def _websocket_handler(
         try:
             await fut
         except Exception as e:
-            self.error_handler.log(request, e)
+            noisy = self.error_handler.noisy
+            self.error_handler._log(request, e, noisy)
         except (CancelledError, ConnectionClosed):
             cancelled = True
         finally:
@@ -954,6 +955,7 @@ def run(
         unix: Optional[str] = None,
         loop: None = None,
         reload_dir: Optional[Union[List[str], str]] = None,
+        noisy_exceptions: bool = False,
     ) -> None:
         """
         Run the HTTP Server and listen until keyboard interrupt or term
@@ -1036,6 +1038,7 @@ def run(
             backlog=backlog,
             register_sys_signals=register_sys_signals,
             auto_reload=auto_reload,
+            noisy_exceptions=noisy_exceptions,
         )
 
         try:
@@ -1082,6 +1085,7 @@ async def create_server(
         unix: Optional[str] = None,
         return_asyncio_server: bool = False,
         asyncio_server_kwargs: Dict[str, Any] = None,
+        noisy_exceptions: bool = False,
     ) -> Optional[AsyncioServer]:
         """
         Asynchronous version of :func:`run`.
@@ -1144,6 +1148,7 @@ async def create_server(
             protocol=protocol,
             backlog=backlog,
             run_async=return_asyncio_server,
+            noisy_exceptions=noisy_exceptions,
         )
 
         main_start = server_settings.pop("main_start", None)
@@ -1256,6 +1261,7 @@ def _helper(
         register_sys_signals=True,
         run_async=False,
         auto_reload=False,
+        noisy_exceptions=False,
     ):
         """Helper function used by `run` and `create_server`."""
 
@@ -1276,6 +1282,7 @@ def _helper(
             )
 
         self.error_handler.debug = debug
+        self.error_handler.noisy = noisy_exceptions
         self.debug = debug
 
         server_settings = {
diff --git a/sanic/handlers.py b/sanic/handlers.py
index fd718f06e4..110a7153a3 100644
--- a/sanic/handlers.py
+++ b/sanic/handlers.py
@@ -34,6 +34,7 @@ def __init__(
             Tuple[Type[BaseException], Optional[str]], Optional[RouteHandler]
         ] = {}
         self.debug = False
+        self.noisy = False
         self.fallback = fallback
         self.base = base
 
@@ -59,6 +60,21 @@ def finalize(cls, error_handler):
             )
             error_handler._lookup = error_handler._legacy_lookup
 
+        sig = signature(error_handler.log)
+        if len(sig.parameters) == 2:
+            error_logger.warning(
+                DeprecationWarning(
+                    "You are using a deprecated error handler. The log "
+                    "method should accept three positional parameters: "
+                    "(request, exception, noisy: bool). "
+                    "Until you upgrade your ErrorHandler.log, the noisy "
+                    "exceptions setting will not work properly. Beginning "
+                    "in v??.?, the legacy style log method will not "
+                    "work at all."
+                ),
+            )
+            error_handler._log = error_handler._legacy_log
+
     def _full_lookup(self, exception, route_name: Optional[str] = None):
         return self.lookup(exception, route_name)
 
@@ -180,7 +196,7 @@ def default(self, request, exception):
             :class:`Exception`
         :return:
         """
-        self.log(request, exception)
+        self._log(request, exception, self.noisy)
         return exception_response(
             request,
             exception,
@@ -189,10 +205,16 @@ def default(self, request, exception):
             fallback=self.fallback,
         )
 
+    def _full_log(self, request, exception, noisy: bool):
+        self.log(request, exception, noisy)
+
+    def _legacy_log(self, request, exception, noisy: bool):
+        self.log(request, exception)  # type: ignore
+
     @staticmethod
-    def log(request, exception):
+    def log(request, exception, noisy: bool):
         quiet = getattr(exception, "quiet", False)
-        if quiet is False:
+        if quiet is False or noisy is True:
             try:
                 url = repr(request.url)
             except AttributeError:
@@ -202,6 +224,8 @@ def log(request, exception):
                 "Exception occurred while handling uri: %s", url
             )
 
+    _log = _full_log
+
 
 class ContentRangeHandler:
     """
diff --git a/tests/test_exceptions_handler.py b/tests/test_exceptions_handler.py
index 9bedf7e67c..cfb90d6c81 100644
--- a/tests/test_exceptions_handler.py
+++ b/tests/test_exceptions_handler.py
@@ -227,3 +227,46 @@ def lookup(self, exception):
         "v22.3, the legacy style lookup method will not work at all."
     )
     assert response.status == 400
+
+
+def test_double_arg_exception_handler_log_notice(exception_handler_app, caplog):
+    class CustomErrorHandler(ErrorHandler):
+        @staticmethod
+        def log(request, exception):
+            pass
+
+    exception_handler_app.error_handler = CustomErrorHandler()
+
+    with caplog.at_level(logging.WARNING):
+        _, response = exception_handler_app.test_client.get("/1")
+
+    assert caplog.records[0].message == (
+        "You are using a deprecated error handler. The log "
+        "method should accept three positional parameters: "
+        "(request, exception, noisy: bool). "
+        "Until you upgrade your ErrorHandler.log, the noisy "
+        "exceptions setting will not work properly. Beginning "
+        "in v??.?, the legacy style log method will not "
+        "work at all."
+    )
+    assert response.status == 400
+
+
+def test_error_handler_noisy_log(caplog):
+    class CustomError(Exception):
+        quiet = True
+
+    class FakeRequest(object):
+        url = "http://127.0.0.1:8000"
+
+    with caplog.at_level(logging.ERROR):
+        ErrorHandler.log(FakeRequest(), CustomError(), False)
+
+    assert len(caplog.records) == 0
+
+    with caplog.at_level(logging.ERROR):
+        ErrorHandler.log(FakeRequest(), CustomError(), True)
+
+    assert caplog.records[0].message == (
+        "Exception occurred while handling uri: '%s'" % FakeRequest.url
+    )


Code review comments:
diff_hunk: @@ -890,7 +890,8 @@ async def _websocket_handler(
         try:
             await fut
         except Exception as e:
-            self.error_handler.log(request, e)
+            noisy = self.error_handler.noisy
+            self.error_handler._log(request, e, noisy)
path: sanic/app.py
text: @author:
Ideally we would make the `log` method an `instancemethod` instead of a `staticmethod`, but I couldn't come up with a non-breaking way of doing it, so this is the nicest way of doing it that I've been able to figure out.

@author:
I think I found a way around this 😈