Handle IOErrors and ValueErrors when attempting to register ads.

6 files changed, 57 insertions, 29 deletions

diff --git a/pandora/models/pandora.py b/pandora/models/pandora.py
index 40b368a..6abd3a7 100644
--- a/pandora/models/pandora.py
+++ b/pandora/models/pandora.py
@@ -130,6 +130,8 @@ class PlaylistModel(PandoraModel):
         return cls.get_audio_field(data, "bitrate", preferred_quality)
 
     def get_is_playable(self):
+        if not self.audio_url:
+            return False
         return self._api_client.transport.test_url(self.audio_url)
 
     def prepare_playback(self):
@@ -223,11 +225,26 @@ class AdItem(PlaylistModel):
     def is_ad(self):
         return True
 
-    def register_ad(self, station_id):
-        self._api_client.register_ad(station_id, self.tracking_tokens)
+    def register_ad(self, station_id=None):
+        if not station_id:
+            station_id = self.station_id
+        if self.tracking_tokens:
+            self._api_client.register_ad(station_id, self.tracking_tokens)
+        else:
+            raise ValueError('No ad tracking tokens available for '
+                             'registration.')
 
     def prepare_playback(self):
-        self.register_ad(self.station_id)
+        try:
+            self.register_ad(self.station_id)
+        except ValueError as e:
+            if not self.tracking_tokens:
+                # Ignore registration attempts if no ad tracking tokens are
+                # available
+                pass
+            else:
+                # Unexpected error, re-raise.
+                raise e
         return super(AdItem, self).prepare_playback()
 
 
diff --git a/pandora/transport.py b/pandora/transport.py
index 1a2ee69..62f426a 100644
--- a/pandora/transport.py
+++ b/pandora/transport.py
@@ -9,6 +9,7 @@ exception.
 
 API consumers should use one of the API clients in the pandora.client package.
 """
+import logging
 import random
 import time
 import json
@@ -22,8 +23,10 @@ from .errors import PandoraException
 
 DEFAULT_API_HOST = "tuner.pandora.com/services/json/"
 
+logger = logging.getLogger(__name__)
 
-def retries(max_tries, exceptions=(Exception,)):
+
+def retries(max_tries, exceptions=(IOError,)):
     """Function decorator implementing retrying logic.
 
     exceptions: A tuple of exception classes; default (Exception,)
@@ -46,7 +49,11 @@ def retries(max_tries, exceptions=(Exception,)):
                     retries_left -= 1
                     return func(*args, **kwargs)
 
-                except exceptions:
+                except exceptions as e:
+                    # Don't retry for PandoraExceptions - unlikely that result
+                    # will change for same set of input parameters.
+                    if isinstance(e, PandoraException):
+                        continue
                     if retries_left > 0:
                         time.sleep(delay_exponential(
                             0.5, 2, max_tries - retries_left))
@@ -227,9 +234,13 @@ class APITransport(object):
         self._start_request(method)
 
         url = self._build_url(method)
+        log_data = data
         data = self._build_data(method, data)
         params = self._build_params(method)
+        logger.info('TRANSPORT: \nMETHOD: {}\nPARAMS: {}\nDATA: {}\nURL: {}'
+                    .format(method, params, log_data, url))
         result = self._make_http_request(url, data, params)
+        logger.info('TRANSPORT: \nRESULT: {}\n'.format(result))
 
         return self._parse_response(result)
 
diff --git a/pydora/utils.py b/pydora/utils.py
index 756810a..275c386 100644
--- a/pydora/utils.py
+++ b/pydora/utils.py
@@ -125,15 +125,6 @@ def iterate_forever(func, *args, **kwargs):
             yield playlistItem
         except StopIteration:
             output = func(*args, **kwargs)
-        except errors.ParameterMissing as e:
-            if isinstance(playlistItem, AdItem):
-                if (not playlistItem.tracking_tokens or
-                        len(playlistItem.tracking_tokens) == 0):
-                    # Ad item does not contain any tracking tokens, yield
-                    # without registering
-                    yield playlistItem
-            # Something else went wrong, re-raise
-            raise e
 
 
 class SilentPopen(subprocess.Popen):
diff --git a/tests/test_pandora/test_models.py b/tests/test_pandora/test_models.py
index ecd8cf1..06c988c 100644
--- a/tests/test_pandora/test_models.py
+++ b/tests/test_pandora/test_models.py
@@ -205,19 +205,19 @@ class TestAdItem(TestCase):
     JSON_DATA = {
         'audioUrlMap': {
             'mediumQuality': {
-                'audioUrl': 'mock_med_url', 'bitrate': '64', 'protocol': 'http', 'encoding': 'aacplus'
+                'audioUrl': 'med_url_mock', 'bitrate': '64', 'protocol': 'http', 'encoding': 'aacplus'
             },
             'highQuality': {
-                'audioUrl': 'mock_high_url', 'bitrate': '64', 'protocol': 'http', 'encoding': 'aacplus'
+                'audioUrl': 'high_url_mock', 'bitrate': '64', 'protocol': 'http', 'encoding': 'aacplus'
             },
             'lowQuality': {
-                'audioUrl': 'mock_low_url', 'bitrate': '32', 'protocol': 'http', 'encoding': 'aacplus'}},
-            'clickThroughUrl': 'mock_click_url',
-            'imageUrl': 'mock_img_url',
+                'audioUrl': 'low_url_mock', 'bitrate': '32', 'protocol': 'http', 'encoding': 'aacplus'}},
+            'clickThroughUrl': 'click_url_mock',
+            'imageUrl': 'img_url_mock',
             'companyName': '',
             'title': '',
             'trackGain': '0.0',
-            'adTrackingTokens': ['mock_token_1', 'mock_token_2']
+            'adTrackingTokens': ['token_1_mock', 'token_2_mock']
     }
 
     def setUp(self):
@@ -230,10 +230,19 @@ class TestAdItem(TestCase):
 
     def test_register_ad(self):
         self.result._api_client.register_ad = Mock()
-        self.result.register_ad('id_dummy')
+        self.result.register_ad('id_mock')
 
         assert self.result._api_client.register_ad.called
 
+    def test_register_ad_raises_exception_if_no_tracking_tokens_available(self):
+        with self.assertRaises(ValueError):
+            self.result.tracking_tokens = []
+            self.result._api_client.register_ad = Mock(spec=AdItem)
+
+            self.result.register_ad('id_mock')
+
+            assert self.result._api_client.register_ad.called
+
     def test_prepare_playback(self):
         with patch.object(PlaylistModel, 'prepare_playback') as super_mock:
 
diff --git a/tests/test_pandora/test_transport.py b/tests/test_pandora/test_transport.py
index 59d4cea..bdd549c 100644
--- a/tests/test_pandora/test_transport.py
+++ b/tests/test_pandora/test_transport.py
@@ -6,7 +6,7 @@ from pandora.py2compat import Mock, call
 from tests.test_pandora.test_clientbuilder import TestSettingsDictBuilder
 
 
-class SysCallError(Exception):
+class SysCallError(IOError):
     pass
 
 
diff --git a/tests/test_pydora/test_utils.py b/tests/test_pydora/test_utils.py
index 64cdf74..e5a81b5 100644
--- a/tests/test_pydora/test_utils.py
+++ b/tests/test_pydora/test_utils.py
@@ -18,22 +18,22 @@ class TestIterateForever(TestCase):
         with patch.object(APIClient, 'get_playlist') as get_playlist_mock:
             with patch.object(APIClient, 'register_ad', side_effect=ParameterMissing("ParameterMissing")):
 
-                station = Station.from_json(self.client, {'stationToken': 'dummy_token'})
-                dummy_ad = AdItem.from_json(self.client, {'station_id': 'dummy_id'})
-                get_playlist_mock.return_value=iter([dummy_ad])
+                station = Station.from_json(self.client, {'stationToken': 'mock_token'})
+                ad_mock = AdItem.from_json(self.client, {'station_id': 'mock_id'})
+                get_playlist_mock.return_value=iter([ad_mock])
 
                 station_iter = iterate_forever(station.get_playlist)
 
                 next_track = station_iter.next()
-                self.assertEqual(dummy_ad, next_track)
+                self.assertEqual(ad_mock, next_track)
 
     def test_reraise_missing_params_exception(self):
         with patch.object(APIClient, 'get_playlist', side_effect=ParameterMissing("ParameterMissing")) as get_playlist_mock:
                 with self.assertRaises(ParameterMissing):
 
-                    station = Station.from_json(self.client, {'stationToken': 'dummy_token'})
-                    dummy_track = PlaylistItem.from_json(self.client, {'token': 'dummy_token'})
-                    get_playlist_mock.return_value=iter([dummy_track])
+                    station = Station.from_json(self.client, {'stationToken': 'mock_token'})
+                    mock_track = PlaylistItem.from_json(self.client, {'token': 'mock_token'})
+                    get_playlist_mock.return_value=iter([mock_track])
 
                     station_iter = iterate_forever(station.get_playlist)
                     station_iter.next()