aminghadersohi commented on code in PR #44666:
URL: https://github.com/apache/superset/pull/44666#discussion_r4187771447


##########
superset/views/error_handling.py:
##########
@@ -248,7 +248,15 @@ def refresh_csrf_token(ex: CSRFError) -> FlaskResponse:
 
     @app.errorhandler(HTTPException)
     def show_http_exception(ex: HTTPException) -> FlaskResponse:
-        logger.warning("HTTPException", exc_info=True)
+        status = ex.code or 500
+        if status == 404:
+            # Unmatched URLs are a client condition, and scanner traffic makes
+            # them frequent; a traceback here only adds noise.
+            logger.debug("HTTPException: 404 %r", request.path)

Review Comment:
   You were not misreading it; demoting in-view 404s was not intended. Fixed in 
4301016736f5fa42073c718ad631699a60544b47: the DEBUG branch is now `status == 
404 and request.url_rule is None`, so only unmatched URLs are demoted. A 404 
from a matched view (including the masked-denial `abort(404)` in core.py) stays 
at WARNING with the request path and no traceback. Added 
`test_in_view_404_logs_a_warning_with_path_and_no_traceback` (replaces the old 
DEBUG assertion for that path), and updated the PR description and a code 
comment to state the scope.



##########
superset/views/error_handling.py:
##########
@@ -248,7 +248,15 @@ def refresh_csrf_token(ex: CSRFError) -> FlaskResponse:
 
     @app.errorhandler(HTTPException)
     def show_http_exception(ex: HTTPException) -> FlaskResponse:
-        logger.warning("HTTPException", exc_info=True)
+        status = ex.code or 500
+        if status == 404:
+            # Unmatched URLs are a client condition, and scanner traffic makes
+            # them frequent; a traceback here only adds noise.
+            logger.debug("HTTPException: 404 %r", request.path)
+        elif status < 500:
+            logger.warning("HTTPException: %r", str(ex))

Review Comment:
   Done in 4301016736f5fa42073c718ad631699a60544b47: the 4xx WARNING record now 
includes `request.path` (`HTTPException: <desc> on <path>`), and the test 
asserts it.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to