Handle exceptions after re-authentication
Currently, no handling is done for the 2nd request (we catch HTTPError, but it won't be raised without raise_for_exceptions). Recurse into the same call instead. Change-Id: I9d593a741344fd7d5d6d59eca48a12222572c1ce
This commit is contained in:
5
releasenotes/notes/re-raise-1fe9f912691e893e.yaml
Normal file
5
releasenotes/notes/re-raise-1fe9f912691e893e.yaml
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
---
|
||||||
|
fixes:
|
||||||
|
- |
|
||||||
|
Correctly handles errors on a request that is re-tried after refreshing
|
||||||
|
an authentication token.
|
@@ -107,7 +107,7 @@ class Connector(object):
|
|||||||
return retry
|
return retry
|
||||||
|
|
||||||
def _op(self, method, path='', data=None, headers=None, blocking=False,
|
def _op(self, method, path='', data=None, headers=None, blocking=False,
|
||||||
timeout=60, server_side_retries_left=None,
|
timeout=60, server_side_retries_left=None, allow_reauth=True,
|
||||||
**extra_session_req_kwargs):
|
**extra_session_req_kwargs):
|
||||||
"""Generic RESTful request handler.
|
"""Generic RESTful request handler.
|
||||||
|
|
||||||
@@ -124,6 +124,8 @@ class Connector(object):
|
|||||||
those calls.
|
those calls.
|
||||||
:param server_side_retries_left: Remaining retries. If not provided
|
:param server_side_retries_left: Remaining retries. If not provided
|
||||||
will use limit provided by instance's server_side_retries
|
will use limit provided by instance's server_side_retries
|
||||||
|
:param allow_reauth: Whether to allow refreshing the authentication
|
||||||
|
token.
|
||||||
:param extra_session_req_kwargs: Optional keyword argument to pass
|
:param extra_session_req_kwargs: Optional keyword argument to pass
|
||||||
requests library arguments which would pass on to requests session
|
requests library arguments which would pass on to requests session
|
||||||
object.
|
object.
|
||||||
@@ -184,6 +186,10 @@ class Connector(object):
|
|||||||
LOG.error('Authentication to the session service failed. '
|
LOG.error('Authentication to the session service failed. '
|
||||||
'Please check credentials and try again.')
|
'Please check credentials and try again.')
|
||||||
raise
|
raise
|
||||||
|
if not allow_reauth:
|
||||||
|
LOG.error("Failure occured while attempting to retry "
|
||||||
|
"request after refreshing the session: %s", e)
|
||||||
|
raise
|
||||||
if self._auth is not None:
|
if self._auth is not None:
|
||||||
# self._session.auth value is only set when basic auth is used
|
# self._session.auth value is only set when basic auth is used
|
||||||
if self._session.auth is not None:
|
if self._session.auth is not None:
|
||||||
@@ -210,18 +216,12 @@ class Connector(object):
|
|||||||
raise
|
raise
|
||||||
LOG.debug("Authentication refreshed successfully, "
|
LOG.debug("Authentication refreshed successfully, "
|
||||||
"retrying the call.")
|
"retrying the call.")
|
||||||
try:
|
return self._op(
|
||||||
response = self._session.request(
|
method, path, data=data, headers=headers,
|
||||||
method, url, json=data,
|
blocking=blocking, timeout=timeout,
|
||||||
headers=headers,
|
server_side_retries_left=server_side_retries_left,
|
||||||
verify=self._verify,
|
allow_reauth=False,
|
||||||
timeout=timeout,
|
**extra_session_req_kwargs)
|
||||||
**extra_session_req_kwargs)
|
|
||||||
except exceptions.HTTPError as retry_exc:
|
|
||||||
LOG.error("Failure occured while attempting to retry "
|
|
||||||
"request after refreshing the session. Error: "
|
|
||||||
"%s", retry_exc.message)
|
|
||||||
raise
|
|
||||||
else:
|
else:
|
||||||
if method == 'GET' and url.endswith('SessionService'):
|
if method == 'GET' and url.endswith('SessionService'):
|
||||||
LOG.debug('HTTP GET of SessionService failed %s, '
|
LOG.debug('HTTP GET of SessionService failed %s, '
|
||||||
|
@@ -335,6 +335,46 @@ class ConnectorOpTestCase(base.TestCase):
|
|||||||
self.auth.authenticate.assert_called_once()
|
self.auth.authenticate.assert_called_once()
|
||||||
self.assertEqual(response.json, third_response.json)
|
self.assertEqual(response.json, third_response.json)
|
||||||
|
|
||||||
|
def test_timed_out_session_fail_after_reestablish(self):
|
||||||
|
self.auth._session_key = 'asdf1234'
|
||||||
|
self.auth.get_session_key.return_value = 'asdf1234'
|
||||||
|
self.conn._auth = self.auth
|
||||||
|
self.session = mock.Mock(spec=requests.Session)
|
||||||
|
self.session.auth = None
|
||||||
|
self.conn._session = self.session
|
||||||
|
self.request = self.session.request
|
||||||
|
first_response = mock.MagicMock()
|
||||||
|
first_response.status_code = http_client.FORBIDDEN
|
||||||
|
second_response = mock.MagicMock()
|
||||||
|
second_response.status_code = http_client.BAD_REQUEST
|
||||||
|
self.request.side_effect = [first_response, second_response]
|
||||||
|
self.assertRaises(exceptions.BadRequestError,
|
||||||
|
self.conn._op,
|
||||||
|
'POST', path='fake/path', data=self.data,
|
||||||
|
headers=self.headers)
|
||||||
|
self.auth.refresh_session.assert_called_with()
|
||||||
|
self.auth.can_refresh_session.assert_called_with()
|
||||||
|
|
||||||
|
def test_timed_out_session_fail_after_reestablish_no_recursion(self):
|
||||||
|
self.auth._session_key = 'asdf1234'
|
||||||
|
self.auth.get_session_key.return_value = 'asdf1234'
|
||||||
|
self.conn._auth = self.auth
|
||||||
|
self.session = mock.Mock(spec=requests.Session)
|
||||||
|
self.session.auth = None
|
||||||
|
self.conn._session = self.session
|
||||||
|
self.request = self.session.request
|
||||||
|
first_response = mock.MagicMock()
|
||||||
|
first_response.status_code = http_client.FORBIDDEN
|
||||||
|
second_response = mock.MagicMock()
|
||||||
|
second_response.status_code = http_client.FORBIDDEN
|
||||||
|
self.request.side_effect = [first_response, second_response]
|
||||||
|
self.assertRaises(exceptions.AccessError,
|
||||||
|
self.conn._op,
|
||||||
|
'POST', path='fake/path', data=self.data,
|
||||||
|
headers=self.headers)
|
||||||
|
self.auth.refresh_session.assert_called_with()
|
||||||
|
self.auth.can_refresh_session.assert_called_with()
|
||||||
|
|
||||||
def test_connection_error(self):
|
def test_connection_error(self):
|
||||||
self.request.side_effect = requests.exceptions.ConnectionError
|
self.request.side_effect = requests.exceptions.ConnectionError
|
||||||
self.assertRaises(exceptions.ConnectionError, self.conn._op, 'GET')
|
self.assertRaises(exceptions.ConnectionError, self.conn._op, 'GET')
|
||||||
|
Reference in New Issue
Block a user