[tor-commits] [ooni-probe/master] Fix report write failure bug

[art at torproject.org]

commit 3c166e576f74326a5d2c2f3cab0ed1672047c1df
Author: aagbsn <aagbsn at extc.org>
Date:   Thu Aug 22 13:05:17 2013 +0200

    Fix report write failure bug
    
    When a report has opened, and then a reporter has subsequently failed,
    it is now removed from the list of valid reporters.
---
 ooni/reporter.py |   40 ++++++++++++++++++++++++++++++++++++++--
 ooni/tasks.py    |    9 ++++++++-
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/ooni/reporter.py b/ooni/reporter.py
index 67678a6..aee96e5 100644
--- a/ooni/reporter.py
+++ b/ooni/reporter.py
@@ -453,13 +453,48 @@ class Report(object):
                 if report_tracker.finished():
                     all_written.callback(report_tracker)
 
+            def report_failed(failure):
+                log.debug("Report Write Failure")
+                try:
+                    self.failedWritingReport(failure, reporter)
+                except errors.NoMoreReporters, e:
+                    log.err("No More Reporters!")
+                    all_written.errback(defer.fail(e))
+                else:
+                    report_tracker.completed()
+                    if report_tracker.finished():
+                        all_written.callback(report_tracker)
+                return
+
             report_entry_task = ReportEntry(reporter, measurement)
             self.reportEntryManager.schedule(report_entry_task)
 
-            report_entry_task.done.addBoth(report_completed)
+            report_entry_task.done.addCallback(report_completed)
+            report_entry_task.done.addErrback(report_failed)
 
         return all_written
 
+    def failedWritingReport(self, failure, reporter):
+        """
+        This errback gets called every time we fail to write a report.
+        By fail we mean that the number of retries has exceeded.
+        Once a report has failed to be written with a reporter we give up and
+        remove the reporter from the list of reporters to write to.
+        """
+
+        # XXX: may have been removed already by another failure.
+        if reporter in self.reporters:
+            log.err("Failed to write to %s reporter, giving up..." % reporter)
+            self.reporters.remove(reporter)
+        else:
+            log.err("Failed to write to (already) removed reporter %s" % reporter)
+
+        # Don't forward the exception unless there are no more reporters
+        if len(self.reporters) == 0:
+            log.err("Removed last reporter %s" % reporter)
+            raise errors.NoMoreReporters
+        return
+
     def failedOpeningReport(self, failure, reporter):
         """
         This errback get's called every time we fail to create a report.
@@ -470,7 +505,8 @@ class Report(object):
         log.err("Failed to open %s reporter, giving up..." % reporter)
         log.err("Reporter %s failed, removing from report..." % reporter)
         #log.exception(failure)
-        self.reporters.remove(reporter)
+        if reporter in self.reporters:
+            self.reporters.remove(reporter)
         # Don't forward the exception unless there are no more reporters
         if len(self.reporters) == 0:
             log.err("Removed last reporter %s" % reporter)
diff --git a/ooni/tasks.py b/ooni/tasks.py
index cf13da4..ba7ea4e 100644
--- a/ooni/tasks.py
+++ b/ooni/tasks.py
@@ -138,7 +138,14 @@ class ReportTracker(object):
         """
         Returns true if all the tasks are done. False if not.
         """
-        if self.report_completed == len(self.reporters):
+        # If a reporter fails and is removed, the report
+        # is considered completed but failed, but the number
+        # of reporters is now decreased by the number of failed
+        # reporters.
+        # XXX: should we track to see if, for example:
+        # self.report_completed == len(self.reporters) + \
+        #        len(self.failed_reporters)
+        if self.report_completed >= len(self.reporters):
             return True
         return False