Skip to content

Fixed Unicode coercion errors in logging. #934

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

Closed
wants to merge 1 commit into from
Closed

Fixed Unicode coercion errors in logging. #934

wants to merge 1 commit into from

Conversation

emiller42
Copy link

Some error messages in helpers.py were not correctly wrapping variables
in str() calls before attempting to append them to unicode output. This
caused unicode coercion errors which masked the actual errors triggering
the logging.

This change simply fixes the logging so it is more accurate.

There may be other instances of similar handling. But I wanted to keep
this contained to the specific instances I was encountering in my
install.

Some error messages in helpers.py were not correctly wrapping variables
in str() calls before attempting to append them to unicode output. This
caused unicode coercion errors which masked the actual errors triggering
the logging.

This change simply fixes the logging so it is more accurate.

There may be other instances of similar handling.  But I wanted to keep
this contained to the specific instances I was encountering in my
install.
@emiller42
Copy link
Author

Example log output I'm fixing:

2015-03-22 03:44:03 ERROR    SEARCHQUEUE-RSS-SEARCH :: Error while searching oznzb, skipping: coercing to Unicode: need string or buffer, instance found
2015-03-22 03:44:03 DEBUG    SEARCHQUEUE-RSS-SEARCH :: Traceback (most recent call last):
  File "/Users/Media/Applications/Sick-Beard/sickbeard/search.py", line 161, in searchForNeededEpisodes
    curFoundResults = curProvider.searchRSS()
  File "/Users/Media/Applications/Sick-Beard/sickbeard/providers/generic.py", line 182, in searchRSS
    self.cache.updateCache()
  File "/Users/Media/Applications/Sick-Beard/sickbeard/tvcache.py", line 104, in updateCache
    data = self._getRSSData()
  File "/Users/Media/Applications/Sick-Beard/sickbeard/providers/newznab.py", line 520, in _getRSSData
    data = self.provider.getURL(rss_url)
  File "/Users/Media/Applications/Sick-Beard/sickbeard/providers/generic.py", line 114, in getURL
    response = helpers.getURL(req)
  File "/Users/Media/Applications/Sick-Beard/sickbeard/helpers.py", line 179, in getURL
    obj = getURLFileLike(url, validate, cookies, password_mgr, throw_exc)
  File "/Users/Media/Applications/Sick-Beard/sickbeard/helpers.py", line 234, in getURLFileLike
    logger.log(u"URL error " + str(e.reason) + " while loading URL " + url, logger.WARNING)
TypeError: coercing to Unicode: need string or buffer, instance found

@thezoggy
Copy link
Contributor

should use ek (encoding kludge)

@spidercensus
Copy link

not sure about ek, is that a library?

I've used the str() trick to get around the encoding coercion issue before. Should be fine here. Do you think there would be any value in also wrapping the path variables in the logger.log calls in the make_dirs function (line 482, 488, etc.)?

@CyberTech
Copy link

Did you validate that the url was printed after using the str(url) change?

I tried the same change, and it did NOT print the url -- just an empty string, to the log, and to the console, an instance reference.

Using url.get_full_url() fixed the issue.

Patch @ https://code.google.com/p/sickbeard/issues/detail?id=2585

Fixed (and improved) log message looks like this:

"WARNING SEARCHQUEUE-BACKLOG-281622 :: HTTP error 429:Too Many Requests, while loading URL https://www.nzbfinder.ws/api?... "

@arucard21
Copy link
Contributor

Note that this fix is also provided in #939 and #943 (along with plex fixes).

Those PR's prints the URL correctly instead of just printing a string representation of a Request object (which avoids the problem but doesn't log the URL itself anymore). So it might be better to have one of those PR's merged instead.

@emiller42 emiller42 closed this Dec 7, 2015
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.

5 participants