aminghadersohi commented on code in PR #44666:
URL: https://github.com/apache/superset/pull/44666#discussion_r4190533056
##########
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:
Rechecked against the approved head: addressed by
4301016736f5fa42073c718ad631699a60544b47. The DEBUG condition requires both 404
and an unmatched URL (request.url_rule is None); a matched-view abort(404)
takes the WARNING branch with the escaped path and no traceback.
test_in_view_404_logs_a_warning_with_path_and_no_traceback locks this in.
Re-ran the error-handling file: 37 passed; changed-file pre-commit hooks and
mypy passed. No further code change needed; keeping this thread resolved.
##########
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:
Rechecked against the approved head: addressed by
4301016736f5fa42073c718ad631699a60544b47. The 4xx WARNING format is
HTTPException: %r on %r, with str(ex) and request.path, so the route remains
correlatable and both values are escaped.
test_other_4xx_log_a_warning_without_traceback asserts the path, severity,
absence of traceback, and CR/LF escaping. Re-ran the error-handling file: 37
passed; changed-file pre-commit hooks and mypy passed. No further code change
needed; keeping this thread resolved.
--
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]
