From 6ab8dd80f6c11e1e2def99bdba62b5cf706bf47e Mon Sep 17 00:00:00 2001 From: armando-migliaccio Date: Tue, 2 Jul 2013 17:52:39 -0700 Subject: [PATCH] Do not mask NotImplementedError exceptions With this patch we avoid masking NotImplementedError(s). Previously, a catch-all clause returned 500, and it makes sense to distinguish between genuine 500 errors (i.e. a bug), versus 501 ones. This opens up the issue of keeping the server behavior consistent from one release to another, but one might argue that this was bad design decision in the first place. Fixes bug 1197200 Change-Id: I2d81cba5ee9e5fbe8e865378381790b5b467d255 --- quantum/api/v2/resource.py | 11 +++++++++++ quantum/tests/unit/test_extensions.py | 6 +++--- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/quantum/api/v2/resource.py b/quantum/api/v2/resource.py index 97fc8a8ad..bf9bcb132 100644 --- a/quantum/api/v2/resource.py +++ b/quantum/api/v2/resource.py @@ -94,6 +94,17 @@ def Resource(controller, faults=None, deserializers=None, serializers=None): e.body = serializer.serialize({'QuantumError': e}) e.content_type = content_type raise + except NotImplementedError as e: + # NOTE(armando-migliaccio): from a client standpoint + # it makes sense to receive these errors, because + # extensions may or may not be implemented by + # the underlying plugin. So if something goes south, + # because a plugin does not implement a feature, + # returning 500 is definitely confusing. + body = serializer.serialize( + {'NotImplementedError': e.message}) + kwargs = {'body': body, 'content_type': content_type} + raise webob.exc.HTTPNotImplemented(**kwargs) except Exception as e: # NOTE(jkoelker) Everyting else is 500 LOG.exception(_('%s failed'), action) diff --git a/quantum/tests/unit/test_extensions.py b/quantum/tests/unit/test_extensions.py index 3ac43b2b6..a1bd09e5a 100644 --- a/quantum/tests/unit/test_extensions.py +++ b/quantum/tests/unit/test_extensions.py @@ -76,7 +76,7 @@ class ResourceExtensionTest(base.BaseTestCase): return {'data': {'id': id}} def notimplemented_function(self, request, id): - return webob.exc.HTTPClientError(NotImplementedError()) + return webob.exc.HTTPNotImplemented() def custom_member_action(self, request, id): return {'member_action': 'value'} @@ -114,8 +114,8 @@ class ResourceExtensionTest(base.BaseTestCase): test_app.get("/tweedles/some_id/notimplemented_function") # Shouldn't be reached self.assertTrue(False) - except webtest.AppError: - pass + except webtest.AppError as e: + self.assertTrue('501' in e.message) def test_resource_can_be_added_as_extension(self): res_ext = extensions.ResourceExtension( -- 2.45.2