8000 added logging to HTTP errors w/ url, status and response content by andymccurdy · Pull Request #77 · samgiles/slumber · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

added logging to HTTP errors w/ url, status and response content #77

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

andymccurdy
Copy link
Contributor

This change adds a "slumber" logger and uses it to emit ERROR level messages w/ the URL, HTTP Status and response content in the event of an HTTP error. This should make it easier to debug server errors on the client side.

@andymccurdy
Copy link
Contributor Author

I believe this will also satisfy #68 and #17.

@merwok
Copy link
Contributor
merwok commented Aug 21, 2013

Thanks for this! I’m not sure if error is the right level though; for production servers that could be too much logging. What about debug? /cc @hadrien @pior

pass

logger = logging.getLogger(__name__)
logger.addHandler(NullHandler())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A library should not configure logging handlers. I would revert this.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a common trick because the logging library is stupid and shoves messages to stderr if there are no handlers present.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

iow if you don't do this trick, then end users are forced to configure logging if they don't want spurious output.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If zero handlers are present, Python will yell every time the log line is hit with "No handlers could be found for logger "slumber"".

I've seen the NullHandler pattern in quite a few libs. In fact, I borrowed the above code directly out of requests.

@andymccurdy
Copy link
Contributor Author

I'm fine using whatever log level you deem appropriate. Obviously an easy change. :)

@pior
Copy link
pior commented Aug 21, 2013

@merwok I agree with the error level. There is a "slumber" logger, so it's configurable.

@merwok
Copy link
Contributor
merwok commented Aug 21, 2013

OK for the null handler.

@pior: are you saying you agree with the proposed error level?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants
0