workers.base: Log 500 error message #1637
Conversation
wking
force-pushed
the
log-exception-error-message
branch
from
72b6ee7
to
a8c35ff
Compare
|
I've expanded the scope a bit after discovering the wonders of
|
wking
force-pushed
the
log-exception-error-message
branch
from
a8c35ff
to
9c9a4c1
Compare
|
Tests for the lines I've touched may be more trouble than they're worth. I've worked up one for a broken worker_exit hook to show what they look like, but left the others untested for now. Let me now if the “coverage has worsened” errors are acceptable in this case, or if I need to push through and add enough similar tests to cover all of the log instances. |
wking
force-pushed
the
log-exception-error-message
branch
from
9c9a4c1
to
2935604
Compare
The argument is documented in [1], and we've been using it in some cases since 410bcfa (Fix statsd logging to work on Python 3, 2015-04-20, benoitc#1010). This commit goes through all of our log calls and makes sure we're using it consistently. For more examples of using exc_info instead of direct traceback calls, see 0acfb55 (Let logging module handle traceback printing, 2016-02-05, benoitc#1201). The traceback call I'm removing snuck in later with f4842a5 (Log exception occurring during worker exit, 2016-05-04, benoitc#1254). I'm removing the "Error processing SSL request" debug messages, because those will be handled by the subsequent handle_error call. We've had them, for reasons that I'm not clear on, since 1198cc2 (handle ssl connections, 2012-12-14). I've restored logging in the URI-less Worker.handle_error 500 case, since that was broken in 79011d0 (Include request URL in error message, 2015-07-08, benoitc#1071) but not restored in 1ccebab (prevent crash when reporting an error, 2016-03-22, benoitc#1231). Tests for the lines I've touched may be more trouble than they're worth. I've worked up one for a broken worker_exit hook to show what they look like, but left the others untested for now. [1]: https://docs.python.org/2/library/logging.html#logging.Logger.debug
|
Hi. Your pull request can help fix my issue, check it out: #1660 Will you PR fix issues related to SSLError exceptions? |
|
this is need q more carefull review, exposing traceback in logs can expose some private data. I will comment the changes asap. |
|
Also, I don't think we need to add |
|
stalled but also this houd be revisited by having proper parsing of the log to remove any sensitive data. Maybe also that can be directly handled in the app when needed. |
This makes it a bit easier to debug than just logging the fact that there was an exception. Also restore logging in the URI-less case, since that was broken in 79011d0 (#1071) but not restored in 1ccebab (#1231).