commit d52bbea9d3d370e122a4da35a94362ec17523c51 Author: Damian Johnson atagar@torproject.org Date: Sun Aug 4 14:21:21 2019 -0700

Raise DownloadFailed from stem.descriptor.remote

Ok, now that we have decent test coverage time to use this exception type where it matters most: in our remote module.

This will greately simplify exception catchability and provide callers with the underlying stacktrace (which at present is getting obscured). I attempted to provide this back in commit aebf3e4, which got reverted in 8c61fc8. --- stem/__init__.py | 3 ++- stem/descriptor/remote.py | 44 +++++++++++++++++++++++------------------- stem/directory.py | 2 ++ stem/manual.py | 2 ++ stem/util/connection.py | 4 +++- test/unit/descriptor/remote.py | 2 +- 6 files changed, 34 insertions(+), 23 deletions(-)

diff --git a/stem/__init__.py b/stem/__init__.py index dc8ae2d2..293b2b94 100644 --- a/stem/__init__.py +++ b/stem/__init__.py @@ -775,7 +775,8 @@ class DownloadTimeout(DownloadFailed): """

def __init__(self, url, error, stacktrace, timeout): - super(DownloadTimeout, self).__init__('Failed to download from %s: %0.1f second timeout reached' % (url, timeout)) + message = 'Failed to download from %s: %0.1f second timeout reached' % (url, timeout) + super(DownloadTimeout, self).__init__(url, error, stacktrace, message)

Runlevel = stem.util.enum.UppercaseEnum( diff --git a/stem/descriptor/remote.py b/stem/descriptor/remote.py index af24f624..4f29a2e5 100644 --- a/stem/descriptor/remote.py +++ b/stem/descriptor/remote.py @@ -97,6 +97,7 @@ content. For example...

import io import random +import socket import sys import threading import time @@ -525,11 +526,8 @@ class Query(object): **False**...

* **ValueError** if the descriptor contents is malformed - * **socket.timeout** if our request timed out - * **urllib2.URLError** for most request failures - - Note that the urllib2 module may fail with other exception types, in - which case we'll pass it along. + * :class:`~stem.DownloadTimeout` if our request timed out + * :class:`~stem.DownloadFailed` if our request fails """

return list(self._run(suppress)) @@ -951,11 +949,11 @@ class DescriptorDownloader(object): Traceback (most recent call last): File "demo.py", line 3, in detached_sigs = stem.descriptor.remote.get_detached_signatures().run()[0] - File "/home/atagar/Desktop/stem/stem/descriptor/remote.py", line 476, in run + File "/home/atagar/Desktop/stem/stem/descriptor/remote.py", line 533, in run return list(self._run(suppress)) - File "/home/atagar/Desktop/stem/stem/descriptor/remote.py", line 487, in _run + File "/home/atagar/Desktop/stem/stem/descriptor/remote.py", line 544, in _run raise self.error - urllib2.HTTPError: HTTP Error 404: Not found + stem.DownloadFailed: Failed to download from http://154.35.175.225:80/tor/status-vote/next/consensus-signatures (HTTPError): Not found

.. versionadded:: 1.8.0

@@ -1064,20 +1062,26 @@ def _download_from_dirport(url, compression, timeout): :returns: two value tuple of the form (data, reply_headers)

:raises: - * **socket.timeout** if our request timed out - * **urllib2.URLError** for most request failures + * :class:`~stem.DownloadTimeout` if our request timed out + * :class:`~stem.DownloadFailed` if our request fails """

- response = urllib.urlopen( - urllib.Request( - url, - headers = { - 'Accept-Encoding': ', '.join(map(lambda c: c.encoding, compression)), - 'User-Agent': stem.USER_AGENT, - } - ), - timeout = timeout, - ) + try: + response = urllib.urlopen( + urllib.Request( + url, + headers = { + 'Accept-Encoding': ', '.join(map(lambda c: c.encoding, compression)), + 'User-Agent': stem.USER_AGENT, + } + ), + timeout = timeout, + ) + except socket.timeout as exc: + raise stem.DownloadTimeout(url, exc, sys.exc_info()[2], timeout) + except: + exc, stacktrace = sys.exc_info()[1:3] + raise stem.DownloadFailed(url, exc, stacktrace)

return _decompress(response.read(), response.headers.get('Content-Encoding')), response.headers

diff --git a/stem/directory.py b/stem/directory.py index 01eca70b..075235c9 100644 --- a/stem/directory.py +++ b/stem/directory.py @@ -215,6 +215,8 @@ class Directory(object): :raises: **IOError** if unable to retrieve the fallback directories """

+ # TODO: change IOError to DownloadFailed in stem 2.x + raise NotImplementedError('Unsupported Operation: this should be implemented by the Directory subclass')

def __hash__(self): diff --git a/stem/manual.py b/stem/manual.py index 0aef8a48..729fb204 100644 --- a/stem/manual.py +++ b/stem/manual.py @@ -296,6 +296,8 @@ def download_man_page(path = None, file_handle = None, url = GITWEB_MANUAL_URL, :raises: **IOError** if unable to retrieve the manual """

+ # TODO: change IOError to DownloadFailed in stem 2.x + if not path and not file_handle: raise ValueError("Either the path or file_handle we're saving to must be provided") elif not stem.util.system.is_available('a2x'): diff --git a/stem/util/connection.py b/stem/util/connection.py index a1f93625..421eb9a3 100644 --- a/stem/util/connection.py +++ b/stem/util/connection.py @@ -186,7 +186,9 @@ def download(url, timeout = None, retries = None):

:returns: **bytes** content of the given url

- :raises: :class:`~stem.DownloadFailed` if the download fails + :raises: + * :class:`~stem.DownloadTimeout` if our request timed out + * :class:`~stem.DownloadFailed` if our request fails """

if retries is None: diff --git a/test/unit/descriptor/remote.py b/test/unit/descriptor/remote.py index 6dbaf43e..8a35216d 100644 --- a/test/unit/descriptor/remote.py +++ b/test/unit/descriptor/remote.py @@ -383,7 +383,7 @@ class TestDescriptorDownloader(unittest.TestCase): # After two requests we'll have reached our total permissable timeout. # Check that we don't make a third.

- self.assertRaises(socket.timeout, query.run) + self.assertRaises(stem.DownloadTimeout, query.run) self.assertEqual(2, dirport_mock.call_count)

def test_query_with_invalid_endpoints(self):