[tor-commits] [ooni-probe/master] Handle and log errors when OpenVPN fails to start

[art at torproject.org]

commit 4606063140b98d4cd36d1bd2bbe97ffb7ee1e8df
Author: srvetus <srvetus at users.noreply.github.com>
Date:   Thu Nov 19 12:23:38 2015 +0100

    Handle and log errors when OpenVPN fails to start
    
    When OpenVPN encouters a configuration error it will exit the process with
    error code 1 before beginning to bootstrap. This commit monkeypatches
    `processDirector.processExited` to log these pre-bootstrap OpenVPN configuration failures.
    
    This commit also replaces `handleAllFailures()` with `failureToString()`
    for generating the report content. `handleAllFailures()` causes difficult
    to debug errors. It silently raises an exception from `failure.trap()`
    if an unknown exception type is passed to it. Any raised Exception will causes
    the remaining error logging to be skipped making debugging difficult.
    
    Ooni should produce some sort of log message if an Exception is raised in
    `handleAllFailures()`.
---
 ooni/nettests/third_party/openvpn.py | 21 +++++++++++++++++----
 1 file changed, 17 insertions(+), 4 deletions(-)

diff --git a/ooni/nettests/third_party/openvpn.py b/ooni/nettests/third_party/openvpn.py
index b681222..028bf17 100644
--- a/ooni/nettests/third_party/openvpn.py
+++ b/ooni/nettests/third_party/openvpn.py
@@ -3,7 +3,7 @@ from twisted.python import usage
 from twisted.web.client import Agent, readBody
 from ooni.templates.process import ProcessTest
 from ooni.utils import log
-from ooni.errors import handleAllFailures
+from ooni.errors import handleAllFailures, failureToString
 
 import distutils.spawn
 import re
@@ -32,7 +32,7 @@ class OpenVPNTest(ProcessTest):
     name = "OpenVPN Client Test"
     description = "Connects to an OpenVPN server and does a HTTP GET for the specified URL"
     author = "srvetus "
-    version = "0.0.1"
+    version = "0.0.2"
     timeout = 20
     usageOptions = UsageOptions
     requiredOptions = ['url', 'openvpn-config']
@@ -57,6 +57,16 @@ class OpenVPNTest(ProcessTest):
             self.processDirector.transport.signalProcess('TERM')
             self.exited = True
 
+    def processExited(self, reason):
+            """Monkeypatch processExited to log failure if the process ends
+            unexpectedly before OpenVPN bootstraps.
+            """
+            log.debug("Exited %s" % handleAllFailures(reason))
+
+            # Process exited before OpenVPN bootstrapped. Add failure to report
+            if not self.bootstrapped.called:
+                self.bootstrapped.errback(Exception("openvpn_exited_unexpectedly"))
+
     def handleRead(self, stdout=None, stderr=None):
         """handleRead is called with each chunk of data from stdout and stderr
 
@@ -88,8 +98,8 @@ class OpenVPNTest(ProcessTest):
             self.report['success'] = True
 
         def addFailureToReport(failure):
-            log.debug("failed: %s" % handleAllFailures(failure))
-            self.report['failure'] = handleAllFailures(failure)
+            log.debug("Failed: %s" % failureToString(failure))
+            self.report['failure'] = failureToString(failure)
             self.report['success'] = False
 
         def doRequest(noreason):
@@ -106,6 +116,9 @@ class OpenVPNTest(ProcessTest):
         log.debug("Spawning OpenVPN")
         self.d = self.run(self.command)
 
+        # Monkeypatch processExited to log when OpenVPN exits early
+        self.processDirector.processExited = self.processExited
+
         # Try to make a request when the OpenVPN connection successfully bootstraps
         self.bootstrapped.addCallback(doRequest)