diff --git a/dummyserver/handlers.py b/dummyserver/handlers.py index fb6f44ff1e..759832962f 100644 --- a/dummyserver/handlers.py +++ b/dummyserver/handlers.py @@ -246,6 +246,11 @@ def nbytes(self, request): data, headers=[('Content-Type', 'application/octet-stream')]) + def status(self, request): + status = request.params.get("status", "200 OK") + + return Response(status=status) + def shutdown(self, request): sys.exit() diff --git a/test/with_dummyserver/test_poolmanager.py b/test/with_dummyserver/test_poolmanager.py index 4065ff8bab..1be2a20cc5 100644 --- a/test/with_dummyserver/test_poolmanager.py +++ b/test/with_dummyserver/test_poolmanager.py @@ -109,6 +109,34 @@ def test_raise_on_redirect(self): self.assertEqual(r.status, 303) + def test_raise_on_status(self): + http = PoolManager() + + try: + # the default is to raise + r = http.request('GET', '%s/status' % self.base_url, + fields={'status': '500 Internal Server Error'}, + retries=Retry(total=1, status_forcelist=range(500, 600))) + self.fail("Failed to raise MaxRetryError exception, returned %r" % r.status) + except MaxRetryError: + pass + + try: + # raise explicitly + r = http.request('GET', '%s/status' % self.base_url, + fields={'status': '500 Internal Server Error'}, + retries=Retry(total=1, status_forcelist=range(500, 600), raise_on_status=True)) + self.fail("Failed to raise MaxRetryError exception, returned %r" % r.status) + except MaxRetryError: + pass + + # don't raise + r = http.request('GET', '%s/status' % self.base_url, + fields={'status': '500 Internal Server Error'}, + retries=Retry(total=1, status_forcelist=range(500, 600), raise_on_status=False)) + + self.assertEqual(r.status, 500) + def test_missing_port(self): # Can a URL that lacks an explicit port like ':80' succeed, or # will all such URLs fail with an error? diff --git a/urllib3/connectionpool.py b/urllib3/connectionpool.py index 01afd61697..37048763b3 100644 --- a/urllib3/connectionpool.py +++ b/urllib3/connectionpool.py @@ -653,7 +653,15 @@ def urlopen(self, method, url, body=None, headers=None, retries=None, # Check if we should retry the HTTP response. if retries.is_forced_retry(method, status_code=response.status): - retries = retries.increment(method, url, response=response, _pool=self) + try: + retries = retries.increment(method, url, response=response, _pool=self) + except MaxRetryError: + if retries.raise_on_status: + # Release the connection for this response, since we're not + # returning it to be released manually. + response.release_conn() + raise + return response retries.sleep() log.info("Forced retry: %s", url) return self.urlopen( diff --git a/urllib3/util/retry.py b/urllib3/util/retry.py index 862174abba..2d3aa20d0a 100644 --- a/urllib3/util/retry.py +++ b/urllib3/util/retry.py @@ -102,6 +102,11 @@ class Retry(object): :param bool raise_on_redirect: Whether, if the number of redirects is exhausted, to raise a MaxRetryError, or to return a response with a response code in the 3xx range. + + :param bool raise_on_status: Similar meaning to ``raise_on_redirect``: + whether we should raise an exception, or return a response, + if status falls in ``status_forcelist`` range and retries have + been exhausted. """ DEFAULT_METHOD_WHITELIST = frozenset([ @@ -112,7 +117,8 @@ class Retry(object): def __init__(self, total=10, connect=None, read=None, redirect=None, method_whitelist=DEFAULT_METHOD_WHITELIST, status_forcelist=None, - backoff_factor=0, raise_on_redirect=True, _observed_errors=0): + backoff_factor=0, raise_on_redirect=True, raise_on_status=True, + _observed_errors=0): self.total = total self.connect = connect @@ -127,6 +133,7 @@ def __init__(self, total=10, connect=None, read=None, redirect=None, self.method_whitelist = method_whitelist self.backoff_factor = backoff_factor self.raise_on_redirect = raise_on_redirect + self.raise_on_status = raise_on_status self._observed_errors = _observed_errors # TODO: use .history instead? def new(self, **kw): @@ -137,6 +144,7 @@ def new(self, **kw): status_forcelist=self.status_forcelist, backoff_factor=self.backoff_factor, raise_on_redirect=self.raise_on_redirect, + raise_on_status=self.raise_on_status, _observed_errors=self._observed_errors, ) params.update(kw)