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

@@ -919,6 +919,9 @@ def clearAllProcessLogs(self):
     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

@@ -180,7 +180,7 @@ def test_continue_request_400_if_method_name_is_empty(self):
         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)
@@ -197,11 +197,38 @@ def test_continue_request_500(self):
         self.assertEqual(len(logdata), expected)
         self.assertEqual(logdata[-2],
                u'XML-RPC method called: supervisor.raiseError()')
-        self.assertTrue(logdata[-1].startswith('Traceback'))
-        self.assertTrue(logdata[-1].endswith('ValueError: error\n'))
+        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])
         self.assertEqual(len(request.producers), 0)
         self.assertEqual(request._error, 500)
 
+    def test_continue_request_500_if_xmlrpc_dumps_raises(self):
+        supervisor = DummySupervisor()
+        subinterfaces = [('supervisor', DummySupervisorRPCNamespace())]
+        handler = self._makeOne(supervisor, subinterfaces)
+        import xmlrpclib
+        data = xmlrpclib.dumps((), 'supervisor.getXmlRpcUnmarshallable')
+        request = DummyRequest('/what/ever', None, None, None)
+        handler.continue_request(data, request)
+        logdata = supervisor.options.logger.data
+        from supervisor.xmlrpc import loads
+        if loads:
+            expected = 3
+        else:
+            expected = 4
+        self.assertEqual(len(logdata), expected)
+        self.assertEqual(logdata[-3],
+               'XML-RPC method called: supervisor.getXmlRpcUnmarshallable()')
+        self.assertEqual(logdata[-2],
+               'XML-RPC method supervisor.getXmlRpcUnmarshallable() '
+               'returned successfully')
+        self.assertTrue("unexpected exception" in logdata[-1])
+        self.assertTrue(repr(data) in logdata[-1])
+        self.assertTrue("Traceback" in logdata[-1])
+        self.assertTrue("TypeError: cannot marshal" in logdata[-1])
+
 class TraverseTests(unittest.TestCase):
     def test_underscore(self):
         from supervisor import xmlrpc

supervisor/xmlrpc.py

@@ -363,7 +363,6 @@ def continue_request (self, data, request):
         logger = self.supervisord.options.logger
 
         try:
-
             params, method = self.loads(data)
 
             # no <methodName> in the request or name is an empty string
@@ -380,14 +379,6 @@ def continue_request (self, data, request):
             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, err:
@@ -415,7 +406,10 @@ def continue_request (self, data, request):
 
         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)