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

diff --git a/quantum/api/v2/resource.py b/quantum/api/v2/resource.py
index 97fc8a8adafa62921b6c6e385240338a78f5f543..bf9bcb132c35ab450aac73e987ab1c1e8f8c0c8e 100644 (file)
--- 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 3ac43b2b6446b5ec2fb5eeb03fed008f9d5ea489..a1bd09e5ac26f2283ec6d886d5e6bdde5d2b7869 100644 (file)
--- 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(