commit 3c166e576f74326a5d2c2f3cab0ed1672047c1df Author: aagbsn aagbsn@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