Add failing test with bad message

add a cause to different error types

wip

update cause logic

only use cause for non-error related retries

fix python3 warning

revert test

5 files changed, 56 insertions, 15 deletions

diff --git a/test/test_connectionpool.py b/test/test_connectionpool.py
index 28fb89b7..a6dbcf42 100644
--- a/test/test_connectionpool.py
+++ b/test/test_connectionpool.py
@@ -118,7 +118,7 @@ class TestConnectionPool(unittest.TestCase):
             str(MaxRetryError(
                 HTTPConnectionPool(host='localhost'), "Test.", None)),
             "HTTPConnectionPool(host='localhost', port=None): "
-            "Max retries exceeded with url: Test. (Caused by redirect)")
+            "Max retries exceeded with url: Test. (Caused by None)")
 
         err = SocketError("Test")
 
diff --git a/test/test_retry.py b/test/test_retry.py
index 7a3aa400..9da0b8f9 100644
--- a/test/test_retry.py
+++ b/test/test_retry.py
@@ -1,8 +1,10 @@
 import unittest
 
+from urllib3.response import HTTPResponse
 from urllib3.packages.six.moves import xrange
 from urllib3.util.retry import Retry
 from urllib3.exceptions import (
+    ConnectionError,
     ConnectTimeoutError,
     ReadTimeoutError,
     MaxRetryError
@@ -154,3 +156,41 @@ class RetryTest(unittest.TestCase):
     def test_disabled(self):
         self.assertRaises(MaxRetryError, Retry(-1).increment)
         self.assertRaises(MaxRetryError, Retry(0).increment)
+
+    def test_error_message(self):
+        retry = Retry(total=0)
+        try:
+            retry = retry.increment(error=ReadTimeoutError(None, "/", "read timed out"))
+            raise AssertionError("Should have raised a MaxRetryError")
+        except MaxRetryError as e:
+            assert 'Caused by redirect' not in str(e)
+            self.assertEqual(str(e.reason), 'None: read timed out')
+
+        retry = Retry(total=1)
+        try:
+            retry = retry.increment('POST', '/')
+            retry = retry.increment('POST', '/')
+            raise AssertionError("Should have raised a MaxRetryError")
+        except MaxRetryError as e:
+            assert 'Caused by redirect' not in str(e)
+            self.assertEqual(str(e.reason), 'received erroneous response too many times')
+
+        retry = Retry(total=1)
+        try:
+            response = HTTPResponse(status=500)
+            retry = retry.increment('POST', '/', response=response)
+            retry = retry.increment('POST', '/', response=response)
+            raise AssertionError("Should have raised a MaxRetryError")
+        except MaxRetryError as e:
+            assert 'Caused by redirect' not in str(e)
+            self.assertEqual(str(e.reason), 'received 500 server error too many times')
+
+        retry = Retry(connect=1)
+        try:
+            retry = retry.increment(error=ConnectTimeoutError('conntimeout'))
+            retry = retry.increment(error=ConnectTimeoutError('conntimeout'))
+            raise AssertionError("Should have raised a MaxRetryError")
+        except MaxRetryError as e:
+            print e
+            assert 'Caused by redirect' not in str(e)
+            self.assertEqual(str(e.reason), 'conntimeout')
diff --git a/test/with_dummyserver/test_connectionpool.py b/test/with_dummyserver/test_connectionpool.py
index 7d54fbf9..af3a32ce 100644
--- a/test/with_dummyserver/test_connectionpool.py
+++ b/test/with_dummyserver/test_connectionpool.py
@@ -13,8 +13,7 @@ except:
     from urllib import urlencode
 
 from .. import (
-    requires_network,
-    onlyPy3, onlyPy27OrNewer, onlyPy26OrOlder,
+    requires_network, onlyPy3, onlyPy26OrOlder,
     TARPIT_HOST, VALID_SOURCE_ADDRESSES, INVALID_SOURCE_ADDRESSES,
 )
 from ..port_helpers import find_unused_port
diff --git a/urllib3/exceptions.py b/urllib3/exceptions.py
index 7519ba98..6d9c47f1 100644
--- a/urllib3/exceptions.py
+++ b/urllib3/exceptions.py
@@ -72,11 +72,8 @@ class MaxRetryError(RequestError):
     def __init__(self, pool, url, reason=None):
         self.reason = reason
 
-        message = "Max retries exceeded with url: %s" % url
-        if reason:
-            message += " (Caused by %r)" % reason
-        else:
-            message += " (Caused by redirect)"
+        message = "Max retries exceeded with url: %s (Caused by %r)" % (url,
+                                                                        reason)
 
         RequestError.__init__(self, pool, url, message)
 
diff --git a/urllib3/util/retry.py b/urllib3/util/retry.py
index eb560dfc..95a7c5a0 100644
--- a/urllib3/util/retry.py
+++ b/urllib3/util/retry.py
@@ -36,7 +36,6 @@ class Retry(object):
     Errors will be wrapped in :class:`~urllib3.exceptions.MaxRetryError` unless
     retries are disabled, in which case the causing exception will be raised.
 
-
     :param int total:
         Total number of retries to allow. Takes precedence over other counts.
 
@@ -184,8 +183,8 @@ class Retry(object):
         return isinstance(err, ConnectTimeoutError)
 
     def _is_read_error(self, err):
-        """ Errors that occur after the request has been started, so we can't
-        assume that the server did not process any of it.
+        """ Errors that occur after the request has been started, so we should
+        assume that the server began processing it.
         """
         return isinstance(err, (ReadTimeoutError, ProtocolError))
 
@@ -198,8 +197,7 @@ class Retry(object):
         return self.status_forcelist and status_code in self.status_forcelist
 
     def is_exhausted(self):
-        """ Are we out of retries?
-        """
+        """ Are we out of retries? """
         retry_counts = (self.total, self.connect, self.read, self.redirect)
         retry_counts = list(filter(None, retry_counts))
         if not retry_counts:
@@ -230,6 +228,7 @@ class Retry(object):
         connect = self.connect
         read = self.read
         redirect = self.redirect
+        cause = 'unknown'
 
         if error and self._is_connection_error(error):
             # Connect retry?
@@ -251,10 +250,16 @@ class Retry(object):
             # Redirect retry?
             if redirect is not None:
                 redirect -= 1
+            cause = 'too many redirects'
 
         else:
-            # FIXME: Nothing changed, scenario doesn't make sense.
+            # Incrementing because of a server error like a 500 in
+            # status_forcelist and a the given method is in the whitelist
             _observed_errors += 1
+            try:
+                cause = 'received %s server error too many times' % response.status
+            except Exception:
+                cause = 'received erroneous response too many times'
 
         new_retry = self.new(
             total=total,
@@ -262,7 +267,7 @@ class Retry(object):
             _observed_errors=_observed_errors)
 
         if new_retry.is_exhausted():
-            raise MaxRetryError(_pool, url, error)
+            raise MaxRetryError(_pool, url, error or cause)
 
         log.debug("Incremented Retry for (url='%s'): %r" % (url, new_retry))