Remove special case and log any request whose response is unmarshallable

The special case assertion only caught one unmarshallable value (``None``)
but others are possible (e.g. ``{'x': None}``).

supervisor/tests/base.py

@@ -942,6 +942,9 @@ class DummySupervisorRPCNamespace:
     def raiseError(self):
         raise ValueError('error')
 
+    def getXmlRpcUnmarshallable(self):
+        return {'stdout_logfile': None}  # None is unmarshallable
+
     def getSupervisorVersion(self):
         return '3000'
 

supervisor/tests/test_xmlrpc.py

@@ -153,12 +153,12 @@ class XMLRPCHandlerTests(unittest.TestCase):
         handler.continue_request(data, request)
         logdata = supervisor.options.logger.data
         self.assertEqual(len(logdata), 1)
-        self.assertEqual(logdata[-1],
+        self.assertEqual(logdata[0],
                'XML-RPC request received with no method name')
         self.assertEqual(len(request.producers), 0)
         self.assertEqual(request._error, 400)
 
-    def test_continue_request_500(self):
+    def test_continue_request_500_if_rpcinterface_method_call_raises(self):
         supervisor = DummySupervisor()
         subinterfaces = [('supervisor', DummySupervisorRPCNamespace())]
         handler = self._makeOne(supervisor, subinterfaces)
@@ -167,12 +167,32 @@ class XMLRPCHandlerTests(unittest.TestCase):
         handler.continue_request(data, request)
         logdata = supervisor.options.logger.data
         self.assertEqual(len(logdata), 2)
-        self.assertEqual(logdata[-2],
+        self.assertEqual(logdata[0],
                'XML-RPC method called: supervisor.raiseError()')
-        self.assertTrue(logdata[-1].startswith('Traceback'))
-        self.assertTrue(logdata[-1].endswith('ValueError: error\n'))
-        self.assertEqual(len(request.producers), 0)
-        self.assertEqual(request._error, 500)
+        self.assertTrue("unexpected exception" in logdata[1])
+        self.assertTrue(repr(data) in logdata[1])
+        self.assertTrue("Traceback" in logdata[1])
+        self.assertTrue("ValueError: error" in logdata[1])
+
+    def test_continue_request_500_if_xmlrpc_dumps_raises(self):
+        supervisor = DummySupervisor()
+        subinterfaces = [('supervisor', DummySupervisorRPCNamespace())]
+        handler = self._makeOne(supervisor, subinterfaces)
+        data = xmlrpclib.dumps((), 'supervisor.getXmlRpcUnmarshallable')
+        request = DummyRequest('/what/ever', None, None, None)
+        handler.continue_request(data, request)
+        logdata = supervisor.options.logger.data
+        self.assertEqual(len(logdata), 3)
+        self.assertEqual(logdata[0],
+               'XML-RPC method called: supervisor.getXmlRpcUnmarshallable()')
+        self.assertEqual(logdata[1],
+               'XML-RPC method supervisor.getXmlRpcUnmarshallable() '
+               'returned successfully')
+        self.assertTrue("unexpected exception" in logdata[2])
+        self.assertTrue(repr(data) in logdata[2])
+        self.assertTrue("Traceback" in logdata[2])
+        self.assertTrue("TypeError: cannot marshal" in logdata[2])
+
 
     def test_continue_request_value_is_function(self):
         class DummyRPCNamespace(object):

supervisor/xmlrpc.py

@@ -381,7 +381,6 @@ class supervisor_xmlrpc_handler(xmlrpc_handler):
         logger = self.supervisord.options.logger
 
         try:
-
             params, method = self.loads(data)
 
             # no <methodName> in the request or name is an empty string
@@ -398,14 +397,6 @@ class supervisor_xmlrpc_handler(xmlrpc_handler):
             try:
                 logger.trace('XML-RPC method called: %s()' % method)
                 value = self.call(method, params)
-                # application-specific: we never want to marshal None (even
-                # though we could by saying allow_none=True in dumps within
-                # xmlrpc_marshall), this is meant as a debugging fixture, see:
-                # http://www.plope.com/software/collector/223
-                assert value is not None, (
-                    'return value from method %r with params %r is None' %
-                    (method, params)
-                    )
                 logger.trace('XML-RPC method %s() returned successfully' %
                              method)
             except RPCError as err:
@@ -433,7 +424,10 @@ class supervisor_xmlrpc_handler(xmlrpc_handler):
 
         except:
             tb = traceback.format_exc()
-            logger.critical(tb)
+            logger.critical(
+                "Handling XML-RPC request with data %r raised an unexpected "
+                "exception: %s" % (data, tb)
+                )
             # internal error, report as HTTP server error
             request.error(500)