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.

Sign up for GitHub

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
andymccurdy wants to merge 2 commits into
base: master
Choose a base branch
from
Open

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

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0bd9a0e
added logging to HTTP errors w/ url, status and response content
andymccurdy Aug 21, 2013
aa44acb
No need to annoy people w/ "No handlers" messages
andymccurdy Aug 21, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 18 additions & 2 deletions slumber/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
import posixpath
import urlparse

Expand All @@ -9,6 +10,17 @@
__all__ = ["Resource", "API"]


try: # Python 2.7+
from logging import NullHandler
except ImportError:
class NullHandler(logging.Handler):
def emit(self, record):
pass

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

@merwok merwok Aug 21, 2013

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
Copy Markdown
Collaborator

@dstufft dstufft Aug 21, 2013

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
Copy Markdown
Collaborator

@dstufft dstufft Aug 21, 2013

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
Copy Markdown
Contributor Author

@andymccurdy andymccurdy Aug 21, 2013

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.



def url_join(base, *args):
"""
Helper function to join an arbitrary number of url segments together.
Expand Down Expand Up @@ -105,9 +117,13 @@ def _request(self, method, data=None, files=None, params=None):
resp = self._store["session"].request(method, url, data=data, params=params, files=files, headers=headers)

if 400 <= resp.status_code <= 499:
raise exceptions.HttpClientError("Client Error %s: %s" % (resp.status_code, url), response=resp, content=resp.content)
error_string = "Client Error %s: %s" % (resp.status_code, url)
logger.error("%s, Response from server: %s" % (error_string, resp.content))
raise exceptions.HttpClientError(error_string, response=resp, content=resp.content)
elif 500 <= resp.status_code <= 599:
raise exceptions.HttpServerError("Server Error %s: %s" % (resp.status_code, url), response=resp, content=resp.content)
error_string = "Server Error %s: %s" % (resp.status_code, url)
logger.error("%s, Response from server: %s" % (error_string, resp.content))
raise exceptions.HttpServerError(error_string, response=resp, content=resp.content)

self._ = resp

Expand Down