Catch all errors during change_hook processing.

We shouldn't let errors in change_hooks escape to the user.

master/buildbot/status/web/change_hook.py

@@ -63,6 +63,11 @@ def render_POST(self, request):
         except ValueError, err:
             request.setResponseCode(400, err.args[0])
             return err.args[0]
+        except Exception:
+            log.err(None, "Exception processing web hook.")
+            msg = "Error processing changes."
+            request.setResponseCode(500, msg)
+            return msg
 
         log.msg("Payload: " + str(request.args))
 

master/buildbot/test/unit/test_status_web_change_hook.py

@@ -15,6 +15,7 @@
 
 from buildbot.status.web import change_hook
 from buildbot.util import json
+from buildbot.test.util import compat
 from buildbot.test.fake.web import FakeRequest
 
 from twisted.trial import unittest
@@ -123,3 +124,45 @@ def check_changes(r):
             self.assertEquals(change['files'], ['file1', 'file2'])
         d.addCallback(check_changes)
         return d
+
+    @compat.usesFlushLoggedErrors
+    def testDefaultWithNoChange(self):
+        self.request = FakeRequest()
+        self.request.uri = "/change_hook/"
+        self.request.method = "GET"
+        def namedModuleMock(name):
+            if name == 'buildbot.status.web.hooks.base':
+                class mock_hook_module(object):
+                    def getChanges(self, request, options):
+                        raise AssertionError
+                return mock_hook_module()
+        self.patch(change_hook, "namedModule", namedModuleMock)
+
+        d = self.request.test_render(self.changeHook)
+        def check_changes(r):
+            expected = "Error processing changes."
+            self.assertEquals(len(self.request.addedChanges), 0)
+            self.assertEqual(self.request.written, expected)
+            self.request.setResponseCode.assert_called_with(500, expected)
+            self.assertEqual(len(self.flushLoggedErrors(AssertionError)), 1)
+
+        d.addCallback(check_changes)
+        return d
+
+class TestChangeHookConfiguredBogus(unittest.TestCase):
+    def setUp(self):
+        self.request = FakeRequest()
+        self.changeHook = change_hook.ChangeHookResource(dialects={'garbage' : True})
+
+    @compat.usesFlushLoggedErrors
+    def testBogusDialect(self):
+        self.request.uri = "/change_hook/garbage"
+        self.request.method = "GET"
+        d = self.request.test_render(self.changeHook)
+        def check(ret):
+            expected = "Error processing changes."
+            self.assertEqual(self.request.written, expected)
+            self.request.setResponseCode.assert_called_with(500, expected)
+            self.assertEqual(len(self.flushLoggedErrors()), 1)
+        d.addCallback(check)
+        return d