Merge pull request #545 from tomprince/web-hook-errors

Fix #2379 (and add better error reporting to diagnose it)
buildbot · Oct 5, 2012 · 405606b · 405606b
2 parents df6b0dd + c4201df
commit 405606b
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 7 deletions.
diff --git a/master/buildbot/status/web/change_hook.py b/master/buildbot/status/web/change_hook.py
@@ -63,8 +63,8 @@ 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.")
+        except Exception, e:
+            log.err(e, "processing changes from web hook")
             msg = "Error processing changes."
             request.setResponseCode(500, msg)
             return msg
@@ -78,7 +78,8 @@ def render_POST(self, request):
         def ok(_):
             request.setResponseCode(202)
             request.finish()
-        def err(_):
+        def err(why):
+            log.err(why, "adding changes from web hook")
             request.setResponseCode(500)
             request.finish()
         d.addCallbacks(ok, err)

diff --git a/master/buildbot/status/web/hooks/base.py b/master/buildbot/status/web/hooks/base.py
@@ -56,6 +56,8 @@ def firstOrNothing( value ):
 
         revision = firstOrNothing(args.get('revision'))
         when     = firstOrNothing(args.get('when'))
+        if when is not None:
+            when = float(when)
         author = firstOrNothing(args.get('author'))
         if not author:
             author = firstOrNothing(args.get('who'))

diff --git a/master/buildbot/test/unit/test_status_web_change_hook.py b/master/buildbot/test/unit/test_status_web_change_hook.py
@@ -96,16 +96,16 @@ def testDefaultDialectWithChange(self):
         self.request.args = { "category" : ["mycat"],
                        "files" : [json.dumps(['file1', 'file2'])],
                        "repository" : ["myrepo"],
-                       "when" : [1234],
+                       "when" : ["1234"],
                        "author" : ["Santa Claus"],
-                       "number" : [2],
+                       "number" : ["2"],
                        "comments" : ["a comment"],
                        "project" : ["a project"],
                        "at" : ["sometime"],
                        "branch" : ["a branch"],
                        "revlink" : ["a revlink"],
                        "properties" : [json.dumps( { "prop1" : "val1", "prop2" : "val2" })],
-                       "revision" : [99] }
+                       "revision" : ["99"] }
         d = self.request.test_render(self.changeHook)
         def check_changes(r):
             self.assertEquals(len(self.request.addedChanges), 1)
@@ -115,7 +115,7 @@ def check_changes(r):
             self.assertEquals(change["when"], 1234)
             self.assertEquals(change["author"], "Santa Claus")
             self.assertEquals(change["src"], None)
-            self.assertEquals(change["revision"], 99)
+            self.assertEquals(change["revision"], "99")
             self.assertEquals(change["comments"], "a comment")
             self.assertEquals(change["project"], "a project")
             self.assertEquals(change["branch"], "a branch")