Properly handle SVN connection/client errors.

This requires my fork of the SVN library, until pull request #43 is merged
upstream: https://github.com/dsoprea/PySvn/pull/43

attract_server/subversion.py

@@ -7,6 +7,7 @@ import re
 import attr
 import blinker
 import svn.remote
+import svn.common
 
 from . import attrs_extra
 
@@ -31,18 +32,28 @@ class CommitLogObserver(object):
 
         self._log.debug('%s: fetch_and_observe()', self)
 
-        svn_log = self.svn_client.log_default(
+        try:
-            revision_from=self.last_seen_revision + 1)
+            svn_log = self.svn_client.log_default(revision_from=self.last_seen_revision + 1)
-
             for log_entry in svn_log:
                 self._log.debug('- %r', log_entry)
 
                 # assumption: revisions are always logged in strictly increasing order.
                 self.last_seen_revision = log_entry.revision
 
+                self._parse_log_entry(log_entry)
+        except svn.common.SvnCommandError:
+            # The SVN library just raises a SvnCommandError when something goes wrong,
+            # without any structured indication of the error. There isn't much else
+            # we can do, except to log the error and return.
+            self._log.exception('Error calling self.svn_client.log_default()')
+            return
+
+    def _parse_log_entry(self, log_entry):
+        """Parses the commit log to see if there are any task markers."""
+
         # Log entries can be None.
         if not log_entry.msg:
-                continue
+            return
 
         # Parse the commit log to see if there are any task markers.
         lines = log_entry.msg.split('\n', 1)
@@ -50,6 +61,5 @@ class CommitLogObserver(object):
         for match in task_marker_re.finditer(first_line):
             task_id = match.group('task_id')
 
-                task_logged.send(self,
+            # Send a Blinker signal for each observed task identifier.
-                                 task_id=task_id,
+            task_logged.send(self, task_id=task_id, log_entry=log_entry)
-                                 log_entry=log_entry)

requirements.txt

@@ -1,7 +1,10 @@
 # Primary requirements:
 pillar
 attrs==16.1.0
-svn==0.3.42
+# Use Sybren's branch, until this pull request is merged:
+# https://github.com/dsoprea/PySvn/pull/43
+-e git+https://github.com/sybrenstuvel/PySvn.git@sybren-svncommanderror#egg=svn
+
 
 # Testing requirements:
 pytest==3.0.1

tests/test_subversion.py

@@ -9,6 +9,7 @@ import unittest
 
 from dateutil.tz import tzutc
 import mock
+import svn.common
 
 import logging_config
 from attract_server import subversion
@@ -122,3 +123,18 @@ class TestCommitLogObserver(unittest.TestCase):
                          blinks[1])
         self.assertEqual({'log_entry': SVN_LOG_BATCH_WITH_TASK_MARKERS[2], 'task_id': 'T4433'},
                          blinks[2])
+
+    def test_svn_error(self):
+        """SVN errors should not crash the observer."""
+
+        self.mock_client.log_default = mock.Mock(name='log_default',
+                                                 side_effect=svn.common.SvnCommandError('unittest'))
+
+        record_blink = mock.Mock(name='record_blink',
+                                 spec={'__name__': 'record_blink'})
+        subversion.task_logged.connect(record_blink)
+
+        self.observer.fetch_and_observe()
+
+        record_blink.assert_not_called()
+        self.mock_client.log_default.assert_called_once()