From 029a1d8a6523081153cfaa923531534152e6f309 Mon Sep 17 00:00:00 2001 From: "Florent Vennetier (OpenIO)" Date: Thu, 29 Mar 2018 12:03:02 +0200 Subject: [PATCH] obj: improve exceptions handling Still a bit crappy. --- oioswift/proxy/controllers/obj.py | 44 +++++++++--------------------- oioswift/utils.py | 3 +- tests/unit/controllers/test_obj.py | 8 +++--- 3 files changed, 19 insertions(+), 36 deletions(-) diff --git a/oioswift/proxy/controllers/obj.py b/oioswift/proxy/controllers/obj.py index 2f3b5a0b..aed7f8f4 100644 --- a/oioswift/proxy/controllers/obj.py +++ b/oioswift/proxy/controllers/obj.py @@ -39,11 +39,10 @@ from oio.common import exceptions from oio.common.http import ranges_from_http_header -from oio.common.green import SourceReadTimeout -from urllib3.exceptions import ReadTimeoutError -from oioswift.utils import check_if_none_match, handle_service_busy, \ - handle_not_allowed, ServiceBusy +from oio.common.green import SourceReadTimeout +from oioswift.utils import check_if_none_match, \ + handle_not_allowed, handle_oio_timeout, handle_service_busy SLO = 'x-static-large-object' @@ -120,7 +119,6 @@ class ObjectController(BaseObjectController): @public @cors_validation @delay_denial - @handle_service_busy def HEAD(self, req): """Handle HEAD requests.""" return self.GETorHEAD(req) @@ -128,11 +126,12 @@ def HEAD(self, req): @public @cors_validation @delay_denial - @handle_service_busy def GET(self, req): """Handle GET requests.""" return self.GETorHEAD(req) + @handle_oio_timeout + @handle_service_busy @check_if_none_match def GETorHEAD(self, req): """Handle HTTP GET or HEAD requests.""" @@ -243,6 +242,7 @@ def load_object_metadata(self, headers): @cors_validation @delay_denial @handle_not_allowed + @handle_oio_timeout @handle_service_busy @check_if_none_match def POST(self, req): @@ -285,6 +285,7 @@ def _post_object(self, req, headers, stgpol): @cors_validation @delay_denial @handle_not_allowed + @handle_oio_timeout @handle_service_busy @check_if_none_match def PUT(self, req): @@ -408,11 +409,6 @@ def _link_object(self, req): raise HTTPConflict(request=req) except exceptions.PreconditionFailed: raise HTTPPreconditionFailed(request=req) - except SourceReadTimeout as err: - self.app.logger.warning( - _('ERROR Client read timeout (%ss)'), err.seconds) - self.app.logger.increment('client_timeouts') - raise HTTPRequestTimeout(request=req) except exceptions.SourceReadError: req.client_disconnect = True self.app.logger.warning( @@ -421,20 +417,12 @@ def _link_object(self, req): raise HTTPClientDisconnect(request=req) except exceptions.EtagMismatch: return HTTPUnprocessableEntity(request=req) - except exceptions.OioTimeout: - self.app.logger.exception( - _('ERROR Timeout while uploading data or metadata')) - raise ServiceBusy() - except ServiceBusy: + except (exceptions.ServiceBusy, exceptions.OioTimeout): raise - except ReadTimeoutError: - self.app.logger.exception( - _('ERROR Exception causing client disconnect')) - raise ServiceBusy() except exceptions.ClientException as err: # 481 = CODE_POLICY_NOT_SATISFIABLE if err.status == 481: - raise ServiceBusy + raise exceptions.ServiceBusy() self.app.logger.exception( _('ERROR Exception transferring data %s'), {'path': req.path}) @@ -485,6 +473,7 @@ def _store_object(self, req, data_source, headers): raise HTTPConflict(request=req) except exceptions.PreconditionFailed: raise HTTPPreconditionFailed(request=req) + # FIXME(FVE): SourceReadTimeout should never be raised from oio! except SourceReadTimeout as err: self.app.logger.warning( _('ERROR Client read timeout (%ss)'), err.seconds) @@ -498,20 +487,12 @@ def _store_object(self, req, data_source, headers): raise HTTPClientDisconnect(request=req) except exceptions.EtagMismatch: return HTTPUnprocessableEntity(request=req) - except exceptions.OioTimeout: - self.app.logger.exception( - _('ERROR Timeout while uploading data or metadata')) - raise ServiceBusy() - except ServiceBusy: + except (exceptions.ServiceBusy, exceptions.OioTimeout): raise - except ReadTimeoutError: - self.app.logger.exception( - _('ERROR Exception causing client disconnect')) - raise ServiceBusy() except exceptions.ClientException as err: # 481 = CODE_POLICY_NOT_SATISFIABLE if err.status == 481: - raise ServiceBusy + raise exceptions.ServiceBusy() self.app.logger.exception( _('ERROR Exception transferring data %s'), {'path': req.path}) @@ -557,6 +538,7 @@ def _update_x_timestamp(self, req): @cors_validation @delay_denial @handle_not_allowed + @handle_oio_timeout @handle_service_busy def DELETE(self, req): """HTTP DELETE request handler.""" diff --git a/oioswift/utils.py b/oioswift/utils.py index b0aa3d9f..52fbd334 100644 --- a/oioswift/utils.py +++ b/oioswift/utils.py @@ -145,7 +145,8 @@ def _oio_timeout_wrapper(self, req, *args, **kwargs): headers = dict() # TODO(FVE): choose the value according to the timeout headers['Retry-After'] = '1' - return HTTPServiceUnavailable(request=req, body=str(exc)) + return HTTPServiceUnavailable(request=req, headers=headers, + body=str(exc)) return _oio_timeout_wrapper diff --git a/tests/unit/controllers/test_obj.py b/tests/unit/controllers/test_obj.py index 1e88f3e6..decd8241 100644 --- a/tests/unit/controllers/test_obj.py +++ b/tests/unit/controllers/test_obj.py @@ -267,7 +267,7 @@ def read(self, size): with patch('oio.api.replication.io.http_connect', new=fake_http_connect): resp = req.get_response(self.app) - self.assertEqual(resp.status_int, 499) + self.assertEqual(499, resp.status_int) def test_PUT_chunkreadtimeout_during_data_transfer(self): """The gateway times out while reading from the client.""" @@ -286,7 +286,7 @@ def read(self, size): with patch('oio.api.replication.io.http_connect', new=fake_http_connect): resp = req.get_response(self.app) - self.assertEqual(resp.status_int, 408) + self.assertEqual(408, resp.status_int) def test_PUT_timeout_during_data_transfer(self): """The gateway times out while upload data to the server.""" @@ -305,7 +305,7 @@ def read(self, size): with patch('oio.api.replication.io.http_connect', new=fake_http_connect): resp = req.get_response(self.app) - self.assertEqual(resp.status_int, 503) + self.assertEqual(503, resp.status_int) def test_PUT_truncated_input_empty(self): """The gateway does not receive data from the client.""" @@ -324,7 +324,7 @@ def read(self, size): with patch('oio.api.replication.io.http_connect', new=fake_http_connect): resp = req.get_response(self.app) - self.assertEqual(resp.status_int, 499) + self.assertEqual(499, resp.status_int) def test_PUT_truncated_input_almost(self): """The gateway does not receive enough data from the client."""